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 792775 - matroskamux wrong timestamps
matroskamux wrong timestamps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.x
Other Linux
: Normal blocker
: 1.12.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-01-22 10:20 UTC by Nicola
Modified: 2018-03-13 14:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (3.81 KB, patch)
2018-01-22 12:50 UTC, Nicola
needs-work Details | Review
fix for streaming (1.13 KB, patch)
2018-01-22 18:36 UTC, Nicola
needs-work Details | Review

Description Nicola 2018-01-22 10:20:00 UTC
I'm having some issue receiving video from an axis camera if zipstream technology is enabled.

The problem can be reproduced with the following file:

http://94.177.162.225/temp/sample.mkv

using the following pipeline:

filesrc location=/tmp/sample.mkv ! matroskademux name=d d.video_0 ! queue max-size-bytes=0 max-size-time=0 max-size-buffers=0 ! h264parse ! matroskamux streamable=true ! filesink location=/tmp/test.mkv d.audio_0 ! queue max-size-bytes=0 max-size-time=0 max-size-buffers=0 ! aacparse ! m.

if you look at the produced file with a pipeline like this:

filesrc location=/tmp/test.mkv ! matroskademux ! fakesink silent=false

you can see:

(fakesink0:sink) (121 bytes, dts: none, pts: 0:00:31.901000000, duration: 0:00:00.033333333, offset: -1, offset_end: -1, flags: 00006000 delta-unit tag-memory , meta: none) 0x7f682c014b00
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = event   ******* (fakesink0:sink) E (type: gap (40966), GstEventGap, timestamp=(guint64)0, duration=(guint64)28901000000;) 0x7f682c017740
/GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain   ******* (fakesink0:sink) (113 bytes, dts: none, pts: 0:00:00.000000000, duration: 0:00:00.033333333, offset: -1, offset_end: -1, flags: 00006000 delta-unit tag-memory , meta: none) 0x7f682c014c10


but in the original file the buffer with size 113 bytes following the one with size 121 and pts 0:00:31.901000000 has a pts setted to 0:00:32.867000000 and not to 0
Comment 1 Nicola 2018-01-22 10:24:54 UTC
the problem does not happen using 1.10, while happen with 1.12 and master, I'm now searching the commit to blame
Comment 2 Nicola 2018-01-22 10:41:50 UTC
I'm myself the one to blame :) the regression is introduced in this commit

https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?h=1.12&id=c90b99b7bea9453c100be40bea49c8dee204de21
Comment 3 Nicola 2018-01-22 11:34:23 UTC
the problem here is that before this commit max_cluster duration was:

G_MAXINT16 * mux->time_scale =  32767000000

now it is configurable and its default was inadvertently changed to 65535000000

settings max-cluster-duration to 32767000000 makes the posted pipeline to work as expected.

Maybe make max-cluster-duration configurable is not a good idea and we should hardcode to the previous value, what do you think about?
Comment 4 Nicola 2018-01-22 12:50:02 UTC
Created attachment 367209 [details] [review]
proposed fix
Comment 5 Nicola 2018-01-22 13:36:25 UTC
this seems relevant here:


https://matroska.org/technical/specs/notes.html

"""
The Block's timecode is represented by a 16bit signed interger (sint16). This means that the Blocks timecode has a range of -32768 to +32767 units.. When using the default value of TimecodeScale, each integer represents 1ms. So, the maximum time span of Blocks in a Cluster using the default TimecodeScale of 1ms is 65536ms.

The quick eye will notice that if a Cluster's Timecode is set to zero, it is possible to have Blocks with a negative Raw Timecode. Blocks with a negative Raw Timecode are not valid.
"""
Comment 6 Nicola 2018-01-22 18:25:10 UTC
the sample file has the second keyframe after about 9 minutes and 40 seconds,

obviously a such file will cause problems in pipelines such as this one

gst-launch-1.0 -v filesrc location=/tmp/sample.mkv ! matroskademux ! matroskamux streamable=true ! tcpserversink host=127.0.0.1 port=3000 sync=true sync-method=2

if you connect with tcpclientsrc after 32 seconds that the tcpserversink pipeline is running you will not be able to see the video: a new matroska cluster is started because the max duration was reached and it does not start with a video keyframe, maybe we should set the discont flag in this case. I guess this will cause seeking issues too
Comment 7 Nicola 2018-01-22 18:25:58 UTC
the sample file has the second keyframe after about 9 minutes and 40 seconds,

obviously a such file will cause problems in pipelines such as this one

gst-launch-1.0 -v filesrc location=/tmp/sample.mkv ! matroskademux ! matroskamux streamable=true ! tcpserversink host=127.0.0.1 port=3000 sync=true sync-method=2

if you connect with tcpclientsrc after 32 seconds that the tcpserversink pipeline is running you will not be able to see the video: a new matroska cluster is started because the max duration was reached and it does not start with a video keyframe, maybe we should set the discont flag in this case. I guess this will cause seeking issues too
Comment 8 Nicola 2018-01-22 18:36:50 UTC
Created attachment 367235 [details] [review]
fix for streaming

set delta unit on the matroska cluster if it is started for max cluster duration exceeded
Comment 9 Sebastian Dröge (slomo) 2018-02-28 17:09:29 UTC
Comment on attachment 367209 [details] [review]
proposed fix

We can't just remove a property that was in a stable release. I would suggest that we keep the property but clip it to the maximum possible value. So it can be set smaller than that, but not bigger.

I'll make a patch like that.
Comment 10 Sebastian Dröge (slomo) 2018-02-28 17:11:46 UTC
Review of attachment 367235 [details] [review]:

::: gst/matroska/matroska-mux.c
@@ +3700,3 @@
           gst_util_uint64_scale (buffer_timestamp, 1, mux->time_scale));
+      gst_ebml_write_flush_cache (ebml, !is_max_duration_exceeded,
+          buffer_timestamp);

This wrongly marks it as DELTA_UNIT if max_duration_exceeded happens at the same time as this is a keyframe.
Comment 11 Sebastian Dröge (slomo) 2018-02-28 17:26:03 UTC
commit 80d5c43a8108a3769c27e35117f51dde5135cd1d (HEAD -> master)
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Feb 28 19:21:53 2018 +0200

    matroskamux: Only mark new clusters as keyframe if they start on a keyframe or we're muxing only audio
    
    Based on a patch by Nicola Murino <nicola.murino@gmail.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=792775

commit 15ae79838caa6ee9494bece791b5221b9696c41a
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Feb 28 19:19:10 2018 +0200

    matroskamux: Clip maximum cluster duration to the maximum possible value
    
    Only up to timescale * G_MAXINT16 is possible as cluster duration, which
    is already higher than our default value. Using higher values would
    cause overflows and broken files.
    
    Based on the investigation by Nicola Murino <nicola.murino@gmail.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=792775
Comment 12 Nicola 2018-02-28 17:45:00 UTC
Hi Sebastian,

I'm on deadlines and I have no way to provide a proper patch these days, anyway seems strange to have a property (max-clutser-duration) with range "0 - 9223372036854775807", default "65535000000" and then clip the maximum to a value lesser than the default. We cannot simply change the property range to "0-32767000000" and use 32767000000 as default value?
Comment 13 Nicola 2018-02-28 17:49:00 UTC
nevermind it is related to timecodescale it is 32767000000 with the default timecodescale