After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 680614 - [0.11] audio base classes: _set_format called too soon
[0.11] audio base classes: _set_format called too soon
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-07-25 18:54 UTC by Anton Belka
Modified: 2014-07-08 01:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
log (566 bytes, text/x-log)
2012-07-25 18:54 UTC, Anton Belka
  Details
[PATCH] flacenc: fix writing metadata (4.46 KB, patch)
2012-07-25 18:56 UTC, Anton Belka
none Details | Review

Description Anton Belka 2012-07-25 18:54:02 UTC
Created attachment 219648 [details]
log

Implementing TOC support in flacenc I found strange bug: metadata don't write in flac file. With slomo we found reason: early call gst_flac_enc_set_metadata (). It calls after caps received. I write some g_print () in flacenc to show how it's looks. Please see log for this.
Comment 1 Anton Belka 2012-07-25 18:56:08 UTC
Created attachment 219649 [details] [review]
[PATCH] flacenc: fix writing metadata

Now I moved call gst_flac_enc_set_metadata () from  gst_flac_enc_set_format () in gst_flac_enc_handle_frame (). Maybe this don't look good, but all work prefect. I hope, that patch will be commited soon. And please fix commit message, because I'm not well at English. Many thanks.
Comment 2 Mark Nauwelaerts 2012-07-26 09:25:56 UTC
This is actually an effect of changed behaviour in 0.11, and could effect all sorts of encoders in similar manner.

In both cases, _set_format is called when caps are set.  But in 0.10 this typically happened when a buffer with data arrived (and so just before something really had to be done), but in 0.11 this can happen way sooner (due to caps event) where some tags or so still have to be received.

In short, changing this bug to reflect the underlying problem.
IMO, the solution to it is to change all codec base classes (where possible) to only invoke _set_format when actual data is received, and at which time something really has to be done.  Delaying it allows waiting for tags and also has the benefit to handle "partial caps" in case it takes upstream a bit longer to figure out all the parameters (codec-data or whatever) and may only send complete caps after sending partial caps.

It is also still in line with the documentation saying (e.g.)
----
"GstAudioDecoder calls @set_format to inform subclass of the format
 of input audio data that it is about to receive."
----
and in current 0.11 the 'about' can actually be way sooner now, whereas the suggestion restores it to the 'about' it used to be.
Comment 3 Mark Nauwelaerts 2012-07-26 12:54:16 UTC
Following commits implement approach outlined above, and should as such also resolve this particular flacenc issue (without having to fiddle with subclass):

commit 7b135e8810ab08401d86ea4ba184ecfeefb8c4ce
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu Jul 26 14:28:26 2012 +0200

    video{de,en}coder: delay input caps processing until processing data
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=680614

commit c91615bd82542c0f53046ec6c6d77b5344133ced
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Thu Jul 26 14:27:38 2012 +0200

    audio{de,en}coder: delay input caps processing until processing data
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=680614
Comment 4 Anton Belka 2012-07-26 13:05:50 UTC
Many thanks!
Comment 5 Nicolas Dufresne (ndufresne) 2014-07-08 01:49:29 UTC
There is an annoying side effect to that, the propose_allocation() virtual now get called after the set_format(). This is all counter intuitive, as caps negotiation should be before the allocation. I would suggest we revisit this for 2.0.