GNOME Bugzilla – Bug 703267
funnel: Needs to be usable in playbin
Last modified: 2013-07-10 15:24:06 UTC
For WebKit, we need an element that can be used as the text-stream-combiner in playbin. It needs to: * Combine multiple text streams into one. * The pads need to: * Allow us to activate and deactivate streams. * Expose tags. Adder is similar to what we need, but it waits until it gets buffers on all of the pads, and that could cause problems in text streams, since we may not have buffers on every pad. We also don't want to merge the buffers, we just want to forward all of them. I'm starting on an element I'm calling "textmixer" for now (I'm open to better names, maybe "bufferforwarder" or "rawstreamcombiner" or something), but I wanted to make sure I'm not missing an obvious easier way to handle this.
> I'm starting on an element I'm calling "textmixer" for now (I'm open to better > names, maybe "bufferforwarder" or "rawstreamcombiner" or something), but I > wanted to make sure I'm not missing an obvious easier way to handle this. That sounds like 'funnel' ?
(In reply to comment #1) > That sounds like 'funnel' ? That looks right. I may need to add some things to it though (tags, ability to enable and disable, a way to tell what pad a buffer is from), but that will be much easier to start with.
funnel doesn't seem to work correctly in playbin: gst-git gst-launch-1.0 playbin uri=http://ftp.nluug.nl/ftp/graphics/blender/apricot/trailer/Sintel_Trailer1.480p.DivX_Plus_HD.mkv text-stream-combiner="funnel" Setting pipeline to PAUSED ... Pipeline is PREROLLING ... Redistribute latency... buffering... 100% # hang
To enable/disable a stream, can't you just put a valve before each branch of the funnel? For the tags, I think we should keep track of the pad that is pushing a buffer, and if it's not the same as the last one, push all of the sticky events (and not push them otherwise). While we're at it, I'm not sure if the segment handling in funnel is still required ?
And the information about which pad a buffer comes from could be added as a new GstMeta on the buffers. segment handling in funnel should not be needed anymore, however it would need to forward all sticky events of the current input pad before each buffer after a input pad switch.
Created attachment 248170 [details] Python script that runs playbin with a funnel as the text-stream-combiner The problems with playbin are because the stream-combiners can't be bins: https://bugzilla.gnome.org/show_bug.cgi?id=703405 I'm using a Python script to test this now (attached). It doesn't stall the pipeline, but I also don't see any subtitles. Run like: gst-git ./playbin-funnel.py http://ftp.nluug.nl/ftp/graphics/blender/apricot/trailer/Sintel_Trailer1.480p.DivX_Plus_HD.mkv --use-funnel
It looks like the funnel (thread 9) can't push its buffers because the stream lock is held by something in thread 10, which is waiting on stream synchronizer to get some event related to the segment.
+ Trace 232175
Thread 16 (Thread 0x7fffbe7fc700 (LWP 12403))
Thread 15 (Thread 0x7fffbeffd700 (LWP 12402))
Thread 14 (Thread 0x7fffbf7fe700 (LWP 12401))
Thread 13 (Thread 0x7fffbffff700 (LWP 12400))
Thread 12 (Thread 0x7fffe0abc700 (LWP 12399))
Thread 11 (Thread 0x7fffe12bd700 (LWP 12398))
Thread 9 (Thread 0x7fffe22bf700 (LWP 12396))
(In reply to comment #7) The lockup seems to be because of this code in `gst_stream_synchronizer_sink_event`: if (stream) { if (stream->wait) { GST_DEBUG_OBJECT (pad, "Stream %d is waiting", stream->stream_number); g_cond_wait (&stream->stream_finish_cond, &self->lock); stream = gst_pad_get_element_private (pad); if (stream) stream->wait = FALSE; } } If I comment it out, the funnel works. I'm not sure why the stream is waiting though. It looks like if the stream changes, it waits, but for some reason it doesn't stop waiting. If I change a couple relevant log messages to INFO, I get: 0:00:00.174538788 4031 0x7f9a58013990 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_0> Stream 0 changed 0:00:00.174584992 4031 0x7f9a5802e2d0 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.174619845 4031 0x7f9a600b4140 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_1> Stream 1 changed 0:00:00.174649543 4031 0x7f9a600b4140 INFO streamsynchronizer gststreamsynchronizer.c:291:gst_stream_synchronizer_sink_event:<streamsynchronizer0> All streams have changed -- unblocking 0:00:00.174710411 4031 0x7f9a5802e2d0 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.174780529 4031 0x7f9a58039140 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.174823157 4031 0x7f9a5802e630 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.175018287 4031 0x7f9a5802e1e0 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.175082229 4031 0x7f9a58039370 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.175137989 4031 0x7f9a5802e0a0 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.175322636 4031 0x7f9a580390a0 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.175366048 4031 0x7f9a58039320 INFO streamsynchronizer gststreamsynchronizer.c:276:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 changed 0:00:00.206073080 4031 0x7f9a5802e2d0 INFO streamsynchronizer gststreamsynchronizer.c:341:gst_stream_synchronizer_sink_event:<streamsynchronizer0:sink_2> Stream 2 is waiting
I guess the problem is that the funnel is forwarding all of the stream events instead of creating a new stream?
Created attachment 248188 [details] [review] Only send one stream-start event in funnel I'm not sure if we want to change funnel to only send one stream-start event, or change stream synchronizer to not wait forever if we get multiple streams on one pad. This patch does the funnel change.
(In reply to comment #10) > Created an attachment (id=248188) [details] [review] > Only send one stream-start event in funnel Sometimes this causes "Sticky event misordering, got 'caps' before 'stream-start'" :\ I guess I could use a lock/condition on caps..
(In reply to comment #11) > Sometimes this causes "Sticky event misordering, got 'caps' before > 'stream-start'" :\ I guess I could use a lock/condition on caps.. And if I add a lock/condition so caps get sent after the stream start event, I get warnings about "Got data flow before segment event". I'm guessing we don't want a bunch of locks to keep data in order. I guess we could probably gobble all of the sticky events, and only send them right before data.
I'm looking at the GstMeta now, but I'm not exactly sure how it's supposed to be used. Do I define a GstMeta subclass in gstfunnel.c, or does it go somewhere else so other elements can use this? Are there examples somewhere of this being used?
Finally really ported funnel to 1.x, about a year late. I removed the event specific code for all events except EOS (which have to be accumulated). As for the multiple stream-start problem, I'm blaming streamsynchronizer. commit 176e16aa5ffaa8a357873970aa420f56fff72be0 Author: Olivier Crête <olivier.crete@collabora.com> Date: Mon Jul 1 20:21:10 2013 -0400 funnel: Use default pad function for upstream event/queries The default functions in 1.x already do the right thing commit bbb26f875692a6cd84050c545ba85a7d2129cf5d Author: Olivier Crête <olivier.crete@collabora.com> Date: Mon Jul 1 20:35:21 2013 -0400 funnel: Re-push all sticky events when buffers come from a different pad Don't special case segment/caps, just push all sticky events when they are received on the currently active pad or when the active pad changes.
I guess to make stream-synchronizer happy, the demuxers should put the same seqnum on all stream-starts event it sends for a specific file?
(In reply to comment #15) > I guess to make stream-synchronizer happy, the demuxers should put the same > seqnum on all stream-starts event it sends for a specific file? Well, not necessarily? streamsynchronizer only requires the same seqnum for a single stream, e.g. an audio and video stream of the same container file can have different seqnums... and also different stream-ids. However I see the problem here with funnel, it will forward the different stream-start events of all input streams and confuse streamsynchronizer with that. Maybe funnel should only create a single stream-start event, and in there have a stream-id that is a combination of all input stream-ids? Or maybe the logic in streamsynchronizer needs to be changed, it's the cause of problems since quite some time. Maybe all this has to be implemented differently
(In reply to comment #13) > I'm looking at the GstMeta now, but I'm not exactly sure how it's supposed to > be used. Do I define a GstMeta subclass in gstfunnel.c, or does it go somewhere > else so other elements can use this? Are there examples somewhere of this being > used? You would have to put the GstMeta in a public library somewhere, otherwise the definiton of the meta wouldn't be available to code outside gstfunnel unfortunately. If that wouldn't be a problem you could as well define it inside gstfunnel.c. But as this meta is very funnel specific I wouldn't know in which library to put it. Maybe instead of doing that you could do something with the stream-start events that are now sent between each switch of inputs? Or funnel could introduce a new, custom sticky event for that?
(In reply to comment #17) > Maybe instead of doing that you could do something with the > stream-start events that are now sent between each switch of inputs? Or funnel > could introduce a new, custom sticky event for that? The stream start events seem to be working, but I'm a little concerned that if I get a bunch of buffers at once, the stream start event will only be for the last one. If I handle the new samples in the callback for "new-sample", am I guaranteed to get to them before the next events are handled?
I guess I forgot to mention: For some reason funnel works with stream synchronizer now. I guess something in Olivier's changes fixed it for some reason.
(In reply to comment #18) > (In reply to comment #17) > > Maybe instead of doing that you could do something with the > > stream-start events that are now sent between each switch of inputs? Or funnel > > could introduce a new, custom sticky event for that? > > The stream start events seem to be working, but I'm a little concerned that if > I get a bunch of buffers at once, the stream start event will only be for the > last one. If I handle the new samples in the callback for "new-sample", am I > guaranteed to get to them before the next events are handled? A stream-start event will always arrive in front of the buffers to which it applies, so there should be no problem there. (In reply to comment #19) > I guess I forgot to mention: For some reason funnel works with stream > synchronizer now. I guess something in Olivier's changes fixed it for some > reason. Means we can close this bug now? :)
(In reply to comment #20) > A stream-start event will always arrive in front of the buffers to which it > applies, so there should be no problem there. I mean, if I have two samples, and I use gst_element_get_sticky_event(), I'll get the stream start event for the second sample twice. That's what was happening in WebKit when I waited for the g_timeout to finish, but now that I changed it to immediately handle the new sample in the "new-sample" callback, it seems to work. What I'm wondering is if I'm just getting lucky (always handling it fast enough), or if it's guaranteed that I won't get another sample while I'm in that callback?
Maybe sticky events should be attached to the GstSample structures, getting them from the element is clearly racy ?
(In reply to comment #22) > Maybe sticky events should be attached to the GstSample structures, getting > them from the element is clearly racy ? I'm not sure if it would help, since the sample is created in `gst_app_sink_pull_sample`..
AppSink has its own special callbacks that seem to run immediately: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-appsink.html#gst-app-sink-set-callbacks I'm just wonder if emitting an event does the same thing?
(In reply to comment #22) > Maybe sticky events should be attached to the GstSample structures, getting > them from the element is clearly racy ? Makes sense, for the segment and caps it already does that... so why not for all the other "context". Obviously getting the events from outside the stream is racy, you can only do the buffer-events relation from inside the stream. (In reply to comment #24) > AppSink has its own special callbacks that seem to run immediately: > > http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-appsink.html#gst-app-sink-set-callbacks > > I'm just wonder if emitting an event does the same thing? The callbacks are equivalent to the signals, just cause less overhead.
Olivier's commits fixed this, I'll open a new bug if the stream start events don't work.