GNOME Bugzilla – Bug 682110
qtdemux: discont flag set on multiple buffers in push mode
Last modified: 2013-07-17 17:41:05 UTC
When playing an mp4 file in push mode, I receive multiple successive buffers with the discont flag set on stream start, on each stream. Only the first buffer should have the discont flag set for each stream. The problem is that gst_adapter_take_buffer might return a region copy of the original buffer pushed in the adapter, and the copy includes the buffer flags. So if an input buffer has the discont flag and it is cut in two output buffers, both buffers will have the discont flag set. The bug can be fixed by either patching the gst_adapter_take_buffer function, or clearing the buffer flags in gst_qtdemux_decorate_and_push_buffer.
Created attachment 221645 [details] [review] clear buffer timestamps, metas and flags in GstAdapter
It removes an important optimization from the adapter, won't apply.
A compromise would be to remove flags in the push() function, which is not a costly operation. We can't remove timestamps there since they are needed to update adapter pts and dts on flush(). I'm not sure about metas.
IMO, demuxers should unset the discont flag before pushing, just how they update other flags.
Comment on attachment 221645 [details] [review] clear buffer timestamps, metas and flags in GstAdapter Rejected as per comment #2. I did add note to the adapter docs though to say that the caller needs to clear/set buffer flags as needed, if needed.
I suspect this fixes it, if not please re-open: commit 725faab590353610caf372e2bbe51d27d0a0275f Author: Thiago Santos <thiago.sousa.santos@collabora.com> Date: Wed Apr 17 16:54:22 2013 -0300 qtdemux: do not push discont buffers if they aren't discont qtdemux takes its buffers from a GstAdapter. Those buffers are created from the larger buffer that it obtained from upstream and they carry the same flags, including DISCONT if it is set. In these cases, all buffers that qtdemux is going to push would be marked as DISCONT. This scenario can make parsers/decoders flush on every buffer leading to no decoding at all hapenning. This patch prevents this by unsetting the flag if it shouldn't be set.
Created attachment 244305 [details] [review] avidemux: do not push discont buffers if they aren't discont The qtdemux patch does work, avidemux has the same issue so here is the patch to fix it.
Thanks! commit 8ed611cdbcea0aaf820d157e5ac0c88570d9b77e Author: Arnaud Vrac <avrac@freebox.fr> Date: Wed May 15 11:13:12 2013 +0200 avidemux: do not push discont buffers if they aren't discont https://bugzilla.gnome.org/show_bug.cgi?id=682110
Created attachment 249414 [details] [review] matroskademux: clear discont flag on input buffers
Created attachment 249415 [details] [review] mpegdemux: clear discont flag on input buffers Same for mpegdemux
Comment on attachment 249414 [details] [review] matroskademux: clear discont flag on input buffers commit 6e26f1d0678171ca0a221d43113f163f342924c1 Author: Arnaud Vrac <avrac@freebox.fr> Date: Wed Jul 17 17:11:44 2013 +0200 mastrokademux: do not push discont buffers if they aren't discont Unset the discont flag instead of posssibly pushing a buffer with a flag that's still set. https://bugzilla.gnome.org/show_bug.cgi?id=682110
Comment on attachment 249415 [details] [review] mpegdemux: clear discont flag on input buffers commit 506abb06e28acf2e036c2007b7f0571b3d816b75 Author: Arnaud Vrac <avrac@freebox.fr> Date: Wed Jul 17 17:12:59 2013 +0200 mpegdemux: do not push discont buffers if they aren't discont Explicitly unset discont flag when it shouldn't be set. https://bugzilla.gnome.org/show_bug.cgi?id=682110