After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682110 - qtdemux: discont flag set on multiple buffers in push mode
qtdemux: discont flag set on multiple buffers in push mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.11.x
Other Linux
: Normal normal
: 1.0.8
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-08-17 16:55 UTC by Arnaud Vrac
Modified: 2013-07-17 17:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
clear buffer timestamps, metas and flags in GstAdapter (1.96 KB, patch)
2012-08-17 17:26 UTC, Arnaud Vrac
rejected Details | Review
avidemux: do not push discont buffers if they aren't discont (1.25 KB, patch)
2013-05-15 11:52 UTC, Arnaud Vrac
committed Details | Review
matroskademux: clear discont flag on input buffers (1.22 KB, patch)
2013-07-17 16:25 UTC, Arnaud Vrac
committed Details | Review
mpegdemux: clear discont flag on input buffers (824 bytes, patch)
2013-07-17 16:26 UTC, Arnaud Vrac
committed Details | Review

Description Arnaud Vrac 2012-08-17 16:55:19 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.
Comment 1 Arnaud Vrac 2012-08-17 17:26:32 UTC
Created attachment 221645 [details] [review]
clear buffer timestamps, metas and flags in GstAdapter
Comment 2 Wim Taymans 2012-08-22 08:46:29 UTC
It removes an important optimization from the adapter, won't apply.
Comment 3 Arnaud Vrac 2012-08-22 08:58:17 UTC
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.
Comment 4 Wim Taymans 2012-08-22 09:04:03 UTC
IMO, demuxers should unset the discont flag before pushing, just how they update other flags.
Comment 5 Tim-Philipp Müller 2013-01-16 20:36:04 UTC
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.
Comment 6 Tim-Philipp Müller 2013-05-09 07:56:42 UTC
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.
Comment 7 Arnaud Vrac 2013-05-15 11:52:05 UTC
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.
Comment 8 Tim-Philipp Müller 2013-05-15 12:16:51 UTC
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
Comment 9 Arnaud Vrac 2013-07-17 16:25:31 UTC
Created attachment 249414 [details] [review]
matroskademux: clear discont flag on input buffers
Comment 10 Arnaud Vrac 2013-07-17 16:26:29 UTC
Created attachment 249415 [details] [review]
mpegdemux: clear discont flag on input buffers

Same for mpegdemux
Comment 11 Tim-Philipp Müller 2013-07-17 17:37:33 UTC
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 12 Tim-Philipp Müller 2013-07-17 17:41:05 UTC
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