From 5fcb7f715ace94b5781be8fbc31aa4fae2e3b07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Thu, 1 Mar 2018 17:25:07 +0100 Subject: [PATCH] qtmux: round to nearest when computing mehd and tkhd duration This fixes a bug where in some files mehd.fragment_duration is one unit less than the actual duration of the fragmented movie, as explained below: mehd.fragment_duration is computed by scaling the end timestamp of the last frame of the movie in (in nanoseconds) by the movie timescale. In some situations, the end timestamp is innacurate due to lossy conversion to fixed point required by GstBuffer upstream. Take for instance a movie with 3 frames at exactly 3 fps. $ gst-launch-1.0 -v videotestsrc num-buffers=3 \ ! video/x-raw, framerate="(fraction)3/1" \ ! x264enc \ ! fakesink silent=false dts: 999:59:59.333333334, pts: 1000:00:00.000000000, duration: 0:00:00.333333333 dts: 999:59:59.666666667, pts: 1000:00:00.666666666, duration: 0:00:00.333333334 dts: 1000:00:00.000000000, pts: 1000:00:00.333333333, duration: 0:00:00.333333333 The end timestamp is calculated by qtmux in this way: end timestamp = last frame DTS + last frame DUR - first frame DTS = = 1000:00:00.000000000 + 0:00:00.333333333 - 999:59:59.333333334 = = 0:00:00.999999999 qtmux needs to round this timestamp to the declared movie timescale, which can ameliorate this distortion, but it's important that round-neareast is used; otherwise it would backfire badly. Take for example a movie with a timescale of 30 units/s. 0.999999999 s * 30 units/s = 29.999999970 units A round-floor (as it was done before this patch) would set fragment_duration to 29 units, amplifying the original distorsion from 1 nanosecond up to 33 milliseconds less than the correct value. The greatest distortion would occur in the case where timescale = framerate, where an entire frame duration would be subtracted. Also, rounding is added to tkhd duration computation too, which potentially has the same problem. https://bugzilla.gnome.org/show_bug.cgi?id=793959 --- gst/isomp4/atoms.c | 2 +- gst/isomp4/gstqtmux.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/gst/isomp4/atoms.c b/gst/isomp4/atoms.c index 360e411ce7..1d0735ce70 100644 --- a/gst/isomp4/atoms.c +++ b/gst/isomp4/atoms.c @@ -3281,7 +3281,7 @@ atom_trak_update_duration (AtomTRAK * trak, guint64 moov_timescale) atom_stts_get_total_duration (&trak->mdia.minf.stbl.stts); if (trak->mdia.mdhd.time_info.timescale != 0) { trak->tkhd.duration = - gst_util_uint64_scale (trak->mdia.mdhd.time_info.duration, + gst_util_uint64_scale_round (trak->mdia.mdhd.time_info.duration, moov_timescale, trak->mdia.mdhd.time_info.timescale); } else { trak->tkhd.duration = 0; diff --git a/gst/isomp4/gstqtmux.c b/gst/isomp4/gstqtmux.c index c5fd2ca203..508379b232 100644 --- a/gst/isomp4/gstqtmux.c +++ b/gst/isomp4/gstqtmux.c @@ -3648,9 +3648,11 @@ gst_qt_mux_stop_file (GstQTMux * qtmux) * mvhd should be consistent with empty moov * (but TODO maybe some clients do not handle that well ?) */ qtmux->moov->mvex.mehd.fragment_duration = - gst_util_uint64_scale (qtmux->last_dts, qtmux->timescale, GST_SECOND); - GST_DEBUG_OBJECT (qtmux, "rewriting moov with mvex duration %" - GST_TIME_FORMAT, GST_TIME_ARGS (qtmux->last_dts)); + gst_util_uint64_scale_round (qtmux->last_dts, qtmux->timescale, + GST_SECOND); + GST_DEBUG_OBJECT (qtmux, + "rewriting moov with mvex duration %" GST_TIME_FORMAT, + GST_TIME_ARGS (qtmux->last_dts)); /* seek and rewrite the header */ gst_segment_init (&segment, GST_FORMAT_BYTES); segment.start = qtmux->moov_pos;