GNOME Bugzilla – Bug 776458
urisourcebin: Always configure typefind
Last modified: 2017-01-10 13:28:47 UTC
When "file://" uri used, adaptivedemux is not configured in urisourcebin. Is it intended behaviour? If not, we may need to configure typefind element in urisourcebin always.
Created attachment 342441 [details] [review] urisourcebin: Allow configure typefind element for filesrc To ensure configuring adaptivedemux if needed, setup typefind element even if uri is not matched to streaming protocol.
Comment on attachment 342441 [details] [review] urisourcebin: Allow configure typefind element for filesrc Makes sense, yes. Having a typefind there should be done in any case to support cases like your's
Review of attachment 342441 [details] [review]: ::: gst/playback/gsturisourcebin.c @@ +1902,3 @@ + GST_URI_SOURCE_BIN_UNLOCK (urisrc); + + gst_element_no_more_pads (GST_ELEMENT_CAST (urisrc)); no_more_pads() here looks wrong, it's not done in any of the other cases but instead done "outside" @@ +2185,3 @@ G_CALLBACK (source_new_pad), urisrc); } else { + if (urisrc->is_stream || num_srcpads == 1) { Why special case 1 srcpad here? The other case should handle 1 srcpad just fine
(In reply to Sebastian Dröge (slomo) from comment #3) > Review of attachment 342441 [details] [review] [review]: > ::: gst/playback/gsturisourcebin.c > @@ +1902,3 @@ > + GST_URI_SOURCE_BIN_UNLOCK (urisrc); > + > + gst_element_no_more_pads (GST_ELEMENT_CAST (urisrc)); > > no_more_pads() here looks wrong, it's not done in any of the other cases but > instead done "outside" It seems that we can remove it, because playbin3 does not connect no-more-pads signal of urisourcebin. Also, no-more-pads does not posted if a source element have only sometimes pads. Do we need no-more-pads signal in playbin3 usecase? Note that decodebin3 also never post it. > @@ +2185,3 @@ > G_CALLBACK (source_new_pad), urisrc); > } else { > + if (urisrc->is_stream || num_srcpads == 1) { > > Why special case 1 srcpad here? The other case should handle 1 srcpad just > fine I'm not sure whether there is an source element which has two static source pad. (I've never seen it in my usecase). But, for safe reason, I restrict it to one. Anyway, I can change it :)
Created attachment 342445 [details] [review] urisourcebin: Remove unused signal handler variable Remove never used handler id
Created attachment 342446 [details] [review] urisourcebin: Use GList for typefind elements We need typefind elements per source element's srcpad
Created attachment 342447 [details] [review] urisourcebin: Always configure typefind element
Review of attachment 342445 [details] [review]: OK
Review of attachment 342446 [details] [review]: OK
Review of attachment 342447 [details] [review]: ::: gst/playback/gsturisourcebin.c @@ +2114,3 @@ GST_DEBUG_PAD_NAME (pad), GST_ELEMENT_NAME (element)); + + /* FIXME: What should we do if fail to setup typefind here ? */ There's no need to do extra - if setup_typefind fails, it will already have posted an error on the bus. @@ +2115,3 @@ + + /* FIXME: What should we do if fail to setup typefind here ? */ + setup_typefind (urisrc, pad); I think this code should be: caps = gst_pad_get_current_caps (pad); if (caps == NULL) setup_typefind (urisrc, pad); else { handle_new_pad (urisrc, pad, caps); gst_caps_unref (caps); } That way, if the pad does already have caps, we can skip the typefind element entirely (which will just get the same caps and then fire the have-type signal anyway)
Created attachment 343240 [details] [review] urisourcebin: Configure typefind element for non-streaming uri To ensure configuring adaptivedemux if needed, setup typefind element even if uri is not matched to streaming protocol.
Review of attachment 343240 [details] [review]: Looks great, thanks!
Thanks, pushed: commit 7bd7b2209ad124d65ce33c804c15edc4cc193cb4 Author: Seungha Yang <sh.yang@lge.com> Date: Tue Jan 10 21:52:34 2017 +0900 urisourcebin: Configure typefind element for non-streaming uri To ensure configuring adaptivedemux if needed, setup typefind element even if uri is not matched to streaming protocol. https://bugzilla.gnome.org/show_bug.cgi?id=776458 commit 5d0628ff52ca3194a870fdf6af771fce61924984 Author: Seungha Yang <sh.yang@lge.com> Date: Sat Dec 24 16:44:26 2016 +0900 urisourcebin: Use GList for typefind elements We need typefind elements per source element's srcpad https://bugzilla.gnome.org/show_bug.cgi?id=776458 commit 46c6e92abdf4d20bdd12e6f59def1c335ce3b0b7 Author: Seungha Yang <sh.yang@lge.com> Date: Sat Dec 24 16:15:45 2016 +0900 urisourcebin: Remove unused signal handler variable Remove never used handler id https://bugzilla.gnome.org/show_bug.cgi?id=776458