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 341524 - [decodebin] can't handle decoders with always src pads with ANY caps
[decodebin] can't handle decoders with always src pads with ANY caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal normal
: 0.10.12
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 346563 (view as bug list)
Depends on:
Blocks: 341752
 
 
Reported: 2006-05-12 06:38 UTC by Lutz Mueller
Modified: 2007-01-24 09:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for gstdecodebin (1.69 KB, patch)
2006-05-12 06:41 UTC, Lutz Mueller
none Details | Review
Patch removing typefind code from bz2 plugin (5.12 KB, patch)
2006-05-12 06:45 UTC, Lutz Mueller
reviewed Details | Review
Tiny patch for gstelement (644 bytes, patch)
2006-05-12 06:48 UTC, Lutz Mueller
rejected Details | Review
Patch to remove typefind code from bz2 plugin (6.31 KB, patch)
2006-05-12 19:14 UTC, Lutz Mueller
committed Details | Review
Patch to fix failure of bz2dec if not linked (1.78 KB, patch)
2006-06-06 19:09 UTC, Lutz Mueller
none Details | Review
Patch to fix decodebin in case of GST_CAPS_ANY (7.42 KB, patch)
2006-06-06 19:12 UTC, Lutz Mueller
none Details | Review
Patch to fix decodebin in case of GST_CAPS_ANY (7.75 KB, patch)
2006-07-05 06:39 UTC, Lutz Mueller
none Details | Review

Description Lutz Mueller 2006-05-12 06:38:37 UTC
Please describe the problem:
If elements do not do typefind on their own, decodebin cannot continue to
construct a pipeline.

Steps to reproduce:
Remove typefind code in bz2dec and run something like filesrc ! decodebin ! fakesink


Actual results:
Pipeline won't play because decodebin cannot link to the final sink.

Expected results:


Does this happen every time?


Other information:
Comment 1 Lutz Mueller 2006-05-12 06:41:29 UTC
Created attachment 65294 [details] [review]
Patch for gstdecodebin

Patch to let decodebin create another decodebin if it encounters gst_caps_any while trying to decode a stream.

I am aware of the fact that there may be a problem of recursive creation of decodebins, but I'll fix that once you agree with the general idea.
Comment 2 Lutz Mueller 2006-05-12 06:45:29 UTC
Created attachment 65295 [details] [review]
Patch removing typefind code from bz2 plugin

The bz2 plugin resides in gst-plugins-bad. This is just to demonstrate how much code we can remove from individual plugins if we modify the decodebin as proposed above.

The patch also fixes one bug (reference counting), increases the rank of the plugin and implements the state_change function to correctly reset the decoder.
Comment 3 Lutz Mueller 2006-05-12 06:48:33 UTC
Created attachment 65296 [details] [review]
Tiny patch for gstelement

Unfortunately I cannot tell you why this patch is needed to make decodebin work after above patch to decodebin is applied. It seems some state changes in the decodebin within the decodebin don't get propagated/finished so that gst-launch will never receive the state-change-message and start playing...
Comment 4 Lutz Mueller 2006-05-12 06:52:04 UTC
And finally the pipeline that proved it all on my system: 

gst-launch filesrc location=test.ogg.bz2 ! decodebin ! audioconvert ! alsasink

This pipeline will create a decodebin first, detect that the stream is application/x-bzip, create the bz2dec element, detect that the stream can be anything, create another decodebin element which will detect application/ogg and construct the remaining elements.
Comment 5 Wim Taymans 2006-05-12 09:34:06 UTC
that gstelement patch looks bogus. when an element was busy doing a state change, the return value of any get_state method should be ASYNC and not suddenly SUCCESS.

Comment 6 Tim-Philipp Müller 2006-05-12 11:26:10 UTC
Why not just require (non-source) elements that may be autoplugged to do typefinding themselves [*] in such cases and throw a 'buggy element' error in decodebin if it comes across ANY caps? IMHO that's preferable to adding more complexity to decodebin.


In state_change functions (like the one you're adding in the bz2dec-remove-typefinding patch) you should handle upwards state changes first, then chain up to the parent class, and then handle downwards state changes. So in your patch you'd need to chain up first.


[*] after all it's dead easy using gst_type_find_helper_*() (bz2dec should probably use gst_type_find_helper_from_buf() as well btw)

Comment 7 Lutz Mueller 2006-05-12 16:04:48 UTC
When I wrote bz2dec, gst_type_find_helper_from_buf did not yet exist...

My intention was to move as much complexity out of the individual plugins into the core. But I agree that gst_type_find_helper_from_buf makes things very easy. I'll submit a new patch for the bz2dec.
Comment 8 Lutz Mueller 2006-05-12 19:14:25 UTC
Created attachment 65346 [details] [review]
Patch to remove typefind code from bz2 plugin

So, let's forget about the decodebin patch and just fix the bz2 plugin:

* link to gst-base-libs
* increase rank of bz2 plugin to PRIMARY
* use gst_type_find_helper_for_buffer instead of reinventing the wheel
* set the correct caps on outgoing buffers
* fix refcounting error (no unref if pushing of buffer failed)
* use fixed caps
* implement state change function
Comment 9 Jan Schmidt 2006-05-26 10:17:13 UTC
This looks fine to me.
Comment 10 Tim-Philipp Müller 2006-05-26 11:30:31 UTC
The patch to bz2 is fine, but last time I checked it still didn't work right and IIRC the problem is indeed decodebin doing something stupid or nothing when a pad that ALWAYS exists has ANY caps.

So we do need to fix this in decodebin, or make bz2dec use a SOMETIMES src pad that only gets added when the type is known (like id3demux).
Comment 11 Jan Schmidt 2006-05-26 11:37:11 UTC
As I remember it, there's some code in decodebin that is supposed to defer linking on ANY pads until the caps become more fixed. Possibly that code is just broken, or I'm misremembering.
Comment 12 Lutz Mueller 2006-05-26 20:33:39 UTC
I don't have write access to CVS. Could you check it in for me? Thanks!

Tim-Philipp: You are right. gst_pad_alloc_buffer fails in gst_bz2dec_chain because the pad is not linked. Just using gst_buffer_new_and_alloc won't help either because gst_pad_push will then fail (because the pad is still not linked although the caps have been set in the meanwhile). This needs to be fixed in decodebin.
Comment 13 Tim-Philipp Müller 2006-05-28 17:10:16 UTC
Committed:

 2006-05-28  Tim-Philipp Müller  <tim at centricular dot net>

        Patch by: Lutz Müller  <lutz at topfrose de>

        * ext/bz2/Makefile.am:
        * ext/bz2/gstbz2dec.c: (gst_bz2dec_chain), (gst_bz2dec_init),
        (gst_bz2dec_change_state), (gst_bz2dec_class_init):
          Use gst_type_find_helper_* functions for typefinding; use
          correct caps with gst_pad_alloc_buffer(); add state change
          function and reset decoder in it; don't unref buffer if
          pad_push fails; use fixed caps on source pad. (#341524).


Kept rank at NONE until the issue in decodebin is sorted out.
Comment 14 Lutz Mueller 2006-06-06 19:09:29 UTC
Created attachment 66844 [details] [review]
Patch to fix failure of bz2dec if not linked

This patch moves the rank to PRIMARY and avoids failure of the bz2dec plugin if the source pad is not linked yet.
Comment 15 Lutz Mueller 2006-06-06 19:12:32 UTC
Created attachment 66847 [details] [review]
Patch to fix decodebin in case of GST_CAPS_ANY

This patch simplifies the syntax of the function close_pad_link. In addition, if a pad with GST_CAPS_ANY is encountered, linking is postponed until the pad changes its caps to more specific ones.

This works with the (patched) bz2dec plugin.
Comment 16 Lutz Mueller 2006-07-05 06:39:10 UTC
Created attachment 68382 [details] [review]
Patch to fix decodebin in case of GST_CAPS_ANY

According to [1], typefind should be done in decodebin. But currently, decodebin does only create a typefind plugin once. This patch lets decodebin create a typefind element each time it encounters GST_CAPS_ANY. This fixes the case where decodebin dynamically creates an element like gnomevfssrc or neonhttpsrc that do not do typefind on their own. If this patch gets applied, bug [1] can be closed.

[1] http://bugzilla.gnome.org/show_bug.cgi?id=346563#c2
Comment 17 Lutz Mueller 2006-07-05 06:42:26 UTC
*** Bug 346563 has been marked as a duplicate of this bug. ***
Comment 18 Wim Taymans 2006-10-10 15:39:34 UTC
Decodebin can now handle delaying the plugging of pads until they have fixed caps set on them (using the "notify::caps" on pads).

Not quite the same but needed for some RTP depayloaders.

As a general note, I prefer elements to use the typefind helpers as much as possible instead of plugging typefind elements (or worse decodebins). The reason being that when a pad has non caps, it's unclear that the type is not known to gstreamer, not yet known to the element, or never known to the element and thus external typefinding is needed (which might lead again to the above three situations).
Comment 19 Wim Taymans 2007-01-22 17:01:58 UTC
can this be closed? I think it's solved unless I'm missing something.
Comment 20 Lutz Mueller 2007-01-23 07:38:30 UTC
I still don't like the fact that decodebin does typefinding only at the beginning. That way, the logic of detecting the mime/type needs to be duplicated in elements like for example bz2dec instead of implementing once in decodebin.

But if this is the way to go, this bug can be closed.