GNOME Bugzilla – Bug 744572
flowcombiner regressions
Last modified: 2015-03-29 13:37:59 UTC
With the introduction of GstFlowCombiner as a helper and so used in lots of demuxers, quite some of them had a last_flow removed in pad/stream struct data. However, so it seems, without replacing all steps and the whole logic that was involved on these stored GstFlowReturn. The following has been specifically seen with avidemux, but other demuxers are potentially affected as well. The problem is that segment seek is pretty broken as follows: * segment seek occurs * data loop advances streams * if a stream reaches past the end-of-segment (minding keyframe), then this is internally tracked by returning a GST_FLOW_EOS (but no data is pushed downstream). So this EOS is passed to GstFlowCombiner but not stored in the pad ABI last_flowret (which still registers OK). So the combined GstFlowReturn determined by GstFlowCombiner never turns EOS (and loop eventually stops at end way past segment or a downstream decoder has seen enough and returns EOS). * so we finally made it past this segment somehow, then a next segment-seek happens * last_flow used to be reset at seek time, but the pad ABI last_flowret is never reset. So some streams may still assume EOS is reached, and internal book-keeping is confused (in this case, no data sent for such stream and things go wrong).
Created attachment 296890 [details] [review] flowcombiner: add a gst_flow_combiner_reset method This patch adds a gst_flow_combiner_reset() method, which will probably have to be used in some places where there used to be stream->last_flowret = GST_FLOW_OK (e.g. at seek time).
Created attachment 296891 [details] [review] avidemux: resurrect some flow return handling ... and this makes avidemux use gst_flow_combiner_reset(). It also really stores the flow return, as it used to. As it stands this is possibly not that clean, and the storing might be best left up to GstFlowCombiner, but that would need some version of _update that is also passed the pad in question. Something similar may also be needed in other demuxers, but let's figure out where/how to go first ... ?
gstflowcombiner uses the value stored in the pad itself to compute the flow combination. The GstPad's value is stored for information only (it is not read anywhere in GstPad code atm). I'd be in favor of adding API to gstflowcombiner and/or gstpad to update this value when the elements wants the pad to explicitly have a flow return, like the EOS case you pointed. Something like gst_flow_combiner_update_pad_flow() or gst_pad_set_last_flow_return(). Doing it through the gstflowcombiner makes it more uniform and could make future changes easier.
Created attachment 297523 [details] [review] flowcombiner: add a gst_flow_combiner_update_pad_flow method As suggested, adds a gst_flow_combiner_update_pad_flow method.
Created attachment 297524 [details] [review] avidemux: resurrect some flow return handling ... and this makes avidemux also use the new method.
Created attachment 297525 [details] [review] flowcombiner: add a gst_flow_combiner_update_pad_flow method ... sigh ... this really being the proper approach.
No objections, so lets assume this is the way to go ... Not resolving for now; needs some more checking whether other demuxers are affected as well. So in core: commit 4aee7ca58ac1143be89d75351f0e7f414dd0a85c Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sun Feb 22 10:12:01 2015 +0100 win32: update exports commit b9411dab75ecaa306a25d5ec9dfc6dff3ee7df79 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Feb 21 20:13:04 2015 +0100 flowcombiner: add a gst_flow_combiner_update_pad_flow() method https://bugzilla.gnome.org/show_bug.cgi?id=744572 API: gst_flow_combiner_update_pad_flow() commit 8ec7272d99e9b7cafcfff178f45e1203d73d38dc Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sun Feb 15 20:52:10 2015 +0100 flowcombiner: add a gst_flow_combiner_reset() method https://bugzilla.gnome.org/show_bug.cgi?id=744572 API: gst_flow_combiner_reset() ... and in -good: commit d0587467fc383c4840a6fda3a06829fee507d499 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Feb 21 20:05:24 2015 +0100 avidemux: resurrect some flow return handling
And some more commits along these lines: in -base: commit d1f91723bed402371fe0829be2f9c8c93ba9d207 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 28 16:59:23 2015 +0100 oggdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 in -ugly: commit 573ce40fad3948981ebf54d876cd1b73b2e0c08e Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 28 16:58:26 2015 +0100 rmdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 commit 53642b1073685346128f524f70ff3f372193a624 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 28 16:57:06 2015 +0100 asfdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 in -good: commit 71b0b8d943bacecdd66fe724bc3a67b145abe215 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Mon Mar 23 20:58:25 2015 +0100 qtdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 commit 33cc1b4854d58fcb06131c88ffbfb38b991667bf Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Mon Mar 23 20:57:56 2015 +0100 flvdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 commit 593cfa086c4e7f8eb29328f493883e2495055585 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Mon Mar 23 20:56:41 2015 +0100 matroskademux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 in -bad: commit 32e87b1024e8019e471273e7f9553e0487a09fc4 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 28 17:28:34 2015 +0100 mxfdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 commit b2d109242fb53592dfd4d648cb0bf430c33cf355 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 28 17:28:26 2015 +0100 mpegdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 in -ffmpeg: commit 2270026d829c58ab73d13855019a34a889af07ab Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sun Mar 29 14:01:50 2015 +0200 avdemux: resurrect some flow return handling https://bugzilla.gnome.org/show_bug.cgi?id=744572 That's all folks (afaik) ...