GNOME Bugzilla – Bug 712744
matroskamux: Fix handling of negative timestamps
Last modified: 2013-11-26 11:33:39 UTC
Currently matroskamux can output buffers with timestamps jumping to crazy values close to G_MAXUINT64. This was broken in commit 3d876d2b16a31a18bd052cf15a50a37e245272eb. For reasons unknown to me this code path is taken fairly often at least in 1.2.x, but rarely (never?) in 0.10, so that's why it could have been left unnoticed for a very long time. Example way to reproduce it is to mux at least two different rtsp (h264) streams.
Created attachment 260312 [details] [review] Patch
So, to make the case more convincing, party^Wgst-launch time: Producer: gst-launch-1.0 -e \ matroskamux name=mux \ videotestsrc ! queue ! x264enc key-int-max=20 ! mux.video_0 \ videotestsrc ! queue ! x264enc ! mux.video_1 \ mux. ! filesink location=broken.mkv Consumer: gst-launch-1.0 -v filesrc location=broken.mkv ! matroskademux name=d d.video_1 ! identity silent=0 ! fakesink | awk '{print $11}' | less ... pts:0:00:10.567000000, pts:0:00:00.000000000, <- bug pts:0:00:10.799000000, ... pts:0:00:21.900000000, pts:0:00:03.003000000, <- once again pts:0:00:22.133000000, Let's see what `mkvinfo -t broken.mkv` (mkvtoolnix package) has to say about this: + Cluster timecode: 10.666s + Cluster previous size: 244936 + SimpleBlock (key, track number 1, 1 frame(s), timecode 10.666s = 00:00:10.666) + Frame with size 7237 + SimpleBlock (track number 2, 1 frame(s), timecode 18446744027.136s = -00:00:08.331) ^^^^^^^^^^^^^^^^ That's what we get from using gst_util_*uint64*_scale with gint64 (relative_timestamp64) and putting it in gint16 (relative_timestamp). I didn't make it clear, but the patch is just a partial revert back to the original/correct code (Bug 339678).
My first comment about code path being taken (or not) is misleading. What I meant was that relative_timestamp64 can (or cannot) be negative in practice. matroskamux in 0.10 can also overflow there - it just seems to be a much, much more rare situation, but still reproducible with the pipeline above.
Thanks for the bug report. It probably occurs (much) more in 1.x due to overall changes in (dts/pts) timestamp handling versus 0.10. It's not quite clear why the referenced commit is introducing lots of _scale stuff rather than simply integer arithmetic, since there is no danger of overflows or so (afaics). So it is either mostly pointless (?) or I am missing something else (such as avoiding int64 arithmetic on some platform or ... ?) As such, have patched this slightly differently while keeping the current _scale usage. commit 643e6fdc363b48af7b62536cd084215e66178393 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Nov 23 12:15:40 2013 +0100 matroskamux: correctly handle negative relative timestamps ... rather than scaling these as unsigned. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=712744 Based on patch by Krzysztof Kotlenga <pocek@users.sf.net>