GNOME Bugzilla – Bug 785951
urisourcebin/decodebin3: Don't use custom EOS events
Last modified: 2017-08-17 10:41:56 UTC
(Note: reproducible with seek_with_stop validate scenarios and playbin3) urisourcebin and decodebin3 make usage of GST_EVENT_CUSTOM_DOWNSTREAM events replacing the actual GST_EVENT_EOS to decide later on whether the EOS event should be forwarded or not. The problem with this is that queue/multiqueue/queue2 will drop any non-{EOS|SEGMENT} events if the downstream flow return is GST_FLOW_EOS. This results in hangs because: 1) upstream (say adaptivedemux) pushes GST_EVENT_EOS once it's done pushing buffers (of which there is slightly too much, causing a downstream decoder/sink to return GST_FLOW_EOS). 2) urisourcebin/decodebin3 converts that EOS to a GST_EVENT_CUSTOM_DOWNSTREAM and pushes it through queue2 and multiqueue elements 3) => Those custom events get dropped because GST_FLOW_EOS was previously returned 4) => Actual EOS events never reaches sinks 5) => hang I am failing to remember the reason why we used custom downstream events instead of actual EOS. The ideal way forward would be to just use regular EOS events to which we add a custom field in the event gststructure (gst_structure_set(eos_event_structure, "urisourcebin", G_TYPE_BOOLEAN, TRUE, NULL).
The reason we don't use a regular EOS event but instead a custom one was introduced partly by the following commit: commit c10e7c5011e5149712bf3c7c9d9bb8ee765c4ac6 Author: Seungha Yang <sh.yang@lge.com> Date: Sun Jan 8 21:36:04 2017 +0900 urisourcebin: Never push actual EOS event to slot Due to the special nature of adaptivedemux, reconfigure happens frequently with seek/track-change. In very exceptional cases, the following sequence is possible: * EOS event is pushed to queue element and still buffers are queued * During draining remaining buffers, reconfiguration downstream happens due to track switch. * The queue gets a not-linked flow return from downstream * Because the sinkpad is EOS, the queue registers an error on the bus, causing the pipeline to fail. Avoid the sinkpad getting marked EOS in the first place, by using a custom event in place of EOS. https://bugzilla.gnome.org/show_bug.cgi?id=777009 I did some testing to try to replace it with actual EOS, but then the problem becomes dealing with EOS propagation being delayed (it gets very messy). The bottom line question is: * Why do we need custom EOS events in the first place and ss there any reason we would want to delay EOS propagation ?
Created attachment 357276 [details] [review] decodebin3/urisourcebin: Switch to actual EOS events internally Use the intended sequence for re-using elements: * EOS * STREAM_START if element is to be re-used This avoids having elements (such as queue/multiqueue/queue2) not properly resetting themselves. When delaying EOS propagation (because we want to wait until all streams of a group are done for example), we re-trigger them by first sending the cached STREAM_START and then EOS (which will cause elements to re-set themselves if needed and accept new buffers/events).
Review of attachment 357276 [details] [review]: Looks good to me except for two minor problems ::: gst/playback/gstdecodebin3-parse.c @@ +272,3 @@ + s = gst_event_writable_structure (event); + gst_structure_set (s, "decodebin3-custom-eos", G_TYPE_BOOLEAN, TRUE, + NULL); You probably want to take over the seqnum of the input event ::: gst/playback/gsturisourcebin.c @@ +1444,3 @@ + s = gst_event_writable_structure (event); + gst_structure_set (s, "urisourcebin-custom-eos", G_TYPE_BOOLEAN, TRUE, + NULL); And here
Review of attachment 357276 [details] [review]: Fixed up and merged, thanks! ::: gst/playback/gsturisourcebin.c @@ +1444,3 @@ + s = gst_event_writable_structure (event); + gst_structure_set (s, "urisourcebin-custom-eos", G_TYPE_BOOLEAN, TRUE, + NULL); Nevermind for this one