GNOME Bugzilla – Bug 703800
playbin: We always request stream combiner pad sink_%u, even if another pad would be better
Last modified: 2013-07-16 06:54:06 UTC
Created attachment 248622 [details] [review] Patch to use `gst_element_get_compatible_pad` instead of `gst_element_get_request_pad` I have a situation where my stream combiner element has multiple kinds of request pads, but playbin only uses the ones named "sink_%u", even if the caps aren't compatible. This patch makes playbin request a compatible pad using the caps. I'm not sure if my reason for doing this is actually a good idea, but I think this makes playbin better anyway. Backwards compatibility: Since input-selector only has one kind of request pad, with ANY caps, this doesn't change behaviour in the default case.
This will also use unlinked existing pads. I think it's better to iterate over all request sink pad templates of the combiner and then explicitly request a pad for the first template that is matching. Otherwise you might get a non-request pad and try to release that later (which will fail). Out of curiosity, what exactly is your custom stream combiner doing? Why does it have different request pad templates?
(In reply to comment #1) > Out of curiosity, what exactly is your custom stream combiner doing? Why does > it have different request pad templates? My stream combiner is a bin with a funnel. If you request a text/vtt pad, it just connects to the funnel. If you request a text/x-raw pad, it creates a webvttenc, then connects that to the funnel. I could probably do it with caps events, but I think it would be more complicated.
If you want to do it with the caps event, you can just put a autoconvert with identity & webvttenc.
That sounds like something that should indeed be done with the CAPS event like everywhere else :)
(In reply to comment #3) > If you want to do it with the caps event, you can just put a autoconvert with > identity & webvttenc. Ah, sounds like another case of me not knowing about standard elements.. I feel like I need to order a GStreamer dictionary.
Created attachment 248747 [details] [review] Use gst_element_request_pads Here's this if you want it. I still think it's a good idea for playbin to not care what the pads are named, even though I won't actually be using this.
(In reply to comment #3) > If you want to do it with the caps event, you can just put a autoconvert with > identity & webvttenc. I'm trying to use this, and autoconvert hooks up the webvttenc at first (because it matches caps ANY), and then the link fails with no attempt to try the identity element.
(In reply to comment #7) > (In reply to comment #3) > > If you want to do it with the caps event, you can just put a autoconvert with > > identity & webvttenc. > > I'm trying to use this, and autoconvert hooks up the webvttenc at first > (because it matches caps ANY), and then the link fails with no attempt to try > the identity element. Can you create a separate bug for that? Also it might be an option to just not use autoconvert for this simple scenario and just do the linking yourself based on the CAPS event.
(In reply to comment #6) > Created an attachment (id=248747) [details] [review] > Use gst_element_request_pads > > Here's this if you want it. I still think it's a good idea for playbin to not > care what the pads are named, even though I won't actually be using this. I'm not so sure about it, it might be better to enforce a stricter interface to the stream combiners to make sure people try to understand the expected behaviour instead of just putting random elements there. We have the same for sinks for example, where a pad with the name "sink" is required.
(In reply to comment #9) > I'm not so sure about it, it might be better to enforce a stricter interface to > the stream combiners to make sure people try to understand the expected > behaviour instead of just putting random elements there. > > We have the same for sinks for example, where a pad with the name "sink" is > required. It seems useful to be able to drop-in elements that weren't specifically designed for this. It's not difficult to throw an element in a bin and setup some ghostpads, but it seems worthwhile to simplify it in playbin. If you're not interested, feel free to close this as invalid though. I don't really have a strong opinion about it anymore.
Ok, let's close it then.