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 749098 - matroskamux: drop streamheader buffers only if they really are headers
matroskamux: drop streamheader buffers only if they really are headers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-08 10:40 UTC by Nicola
Modified: 2016-12-21 17:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (1.22 KB, patch)
2015-05-08 10:40 UTC, Nicola
none Details | Review
proposed fix (1.21 KB, patch)
2015-05-08 10:46 UTC, Nicola
committed Details | Review

Description Nicola 2015-05-08 10:40:24 UTC
Created attachment 303066 [details] [review]
proposed fix

in my app I use matroskademux->appsink/appsrc and I already drop streamheader or buffer with invalid timestamp before sending to appsrc, the additional drop in matroskamux will cause the loss of the first few frames

additionally matroskamux will however drop buffers with invalid timestamp (and streamheader generally have invalid timestamp, at least with my files).

The proposed fix does not break the typical usage with a single pipeline and make the life easier for other use cases too, please accept
Comment 1 Nicola 2015-05-08 10:46:35 UTC
Created attachment 303067 [details] [review]
proposed fix

.
Comment 2 Tim-Philipp Müller 2015-05-08 11:00:57 UTC
I think this makes sense, but it would be good to confirm that at least the affected encoders, and parsers flag the stream header buffers correctly before merging this.
Comment 3 Nicola 2015-05-08 11:24:07 UTC
some tests to verify that the header flag is correctly set 

gst-launch-1.0 -v audiotestsrc num-buffers=1000 ! opusenc ! matroskamux ! filesink location=/tmp/test.mkv

gst-launch-1.0 -v filesrc location=/tmp/test.mkv ! matroskademux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (19 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000440 discont header ) 0x7f0428013280
New clock: GstSystemClock
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (160 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.020000000, offset: -1, offset_end: -1, flags: 00004000 tag-memory ) 0x7f04280134a0

gst-launch-1.0 -v audiotestsrc num-buffers=1000 ! opusenc ! oggmux ! filesink location=/tmp/test.ogg

gst-launch-1.0 -v filesrc location=/tmp/test.ogg ! oggdemux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (19 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7ff16c007390
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (77 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7ff16c007280
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (160 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.020000000, offset: 20000000, offset_end: 960, flags: 00000040 discont ) 0x7ff16c007170


gst-launch-1.0 -v audiotestsrc num-buffers=100 ! vorbisenc ! oggmux ! filesink location=/tmp/test.ogg

gst-launch-1.0 -v filesrc location=/tmp/test.ogg ! oggdemux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (30 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7f81e4007280
New clock: GstSystemClock
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (90 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7f81e4007170
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (3189 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00000440 discont header ) 0x7f81e4007060
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (43 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.000000000, offset: 0, offset_end: 0, flags: 00000040 discont ) 0x7f81e4007b00

gst-launch-1.0 -v audiotestsrc num-buffers=100 ! vorbisenc ! matroskamux ! filesink location=/tmp/test.mkv

gst-launch-1.0 -v filesrc location=/tmp/test.mkv ! matroskademux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (30 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000440 discont header ) 0x7f1bf00066c0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (90 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000400 header ) 0x7f1bf00067d0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (3189 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000400 header ) 0x7f1bf00068e0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (43 bytes, dts: none, pts: 0:00:00.000000000, duration: none, offset: -1, offset_end: -1, flags: 00004000 tag-memory ) 0x7f1bf00069f0

gst-launch-1.0 -v videotestsrc num-buffers=100 ! theoraenc ! matroskamux ! filesink location=/tmp/test.mkv

gst-launch-1.0 -v filesrc location=/tmp/test.mkv ! matroskademux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (42 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000440 discont header ) 0x7fc844013690
New clock: GstSystemClock
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (58 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000400 header ) 0x7fc8440137a0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (2613 bytes, dts: none, pts: none, duration: none, offset: -1, offset_end: -1, flags: 00000400 header ) 0x7fc8440138b0
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (8443 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.033333333, offset: -1, offset_end: -1, flags: 00004000 tag-memory ) 0x7fc8440139c0

gst-launch-1.0 -v videotestsrc num-buffers=100 ! theoraenc ! oggmux ! filesink location=/tmp/test.ogg

gst-launch-1.0 -v filesrc location=/tmp/test.ogg ! oggdemux ! queue ! fakesink silent=false

/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (42 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00002440 discont header delta-unit ) 0x7fd94c007280
New clock: GstSystemClock
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (58 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00002440 discont header delta-unit ) 0x7fd94c007170
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (2613 bytes, dts: none, pts: none, duration: none, offset: 0, offset_end: -1, flags: 00002440 discont header delta-unit ) 0x7fd94c007060
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (8443 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.033333333, offset: 33333333, offset_end: 64, flags: 00000040 discont ) 0x7fd94c007c10
Comment 4 Nicola 2016-02-15 12:25:36 UTC
ping
Comment 5 Nicola 2016-12-19 16:25:49 UTC
this patch works for me with no issue, is anything missing for upstream inclusion?
Comment 6 Tim-Philipp Müller 2016-12-21 17:09:00 UTC
Thanks, pushed:

commit c9506728095b605716ee971c8b13f543804bea6c
Author: Nicola Murino <nicola.murino@gmail.com>
Date:   Fri May 8 12:44:01 2015 +0200

    matroskamux: only drop actual streamheader buffers with xiph codecs
    
    With Xiph codecs the stream header buffers are both in the caps and are
    usually also at the beginning of each input stream, but it's perfectly
    possible that the input stream does not have the stream header buffers
    inline in the data. Matroskamux would drop the first N buffers assuming
    they're stream headers, but this meant it would drop actual payload data
    when the stream didn't contain the stream headers inline. Fix this by
    only dropping leading buffers if they're flagged as stream headers. This
    fixes issues with streams that are being tapped into after streaming
    has started.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=749098