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 793959 - qtmux: round to nearest when computing mehd.fragment_duration
qtmux: round to nearest when computing mehd.fragment_duration
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
unspecified
Other All
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-03-01 16:59 UTC by Alicia Boya García
Modified: 2018-10-27 12:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtmux: round to nearest when computing mehd.fragment_duration (3.00 KB, patch)
2018-03-01 16:59 UTC, Alicia Boya García
reviewed Details | Review
qtmux: round to nearest when computing mehd and tkhd duration (3.73 KB, patch)
2018-10-27 11:50 UTC, Alicia Boya García
committed Details | Review

Description Alicia Boya García 2018-03-01 16:59:38 UTC
In some cases mehd.fragment_duration is one unit less than the actual duration of the fragmented movie.

$ gst-launch-1.0 -v videotestsrc num-buffers=3 \
  ! video/x-raw, framerate="(fraction)3/1" \
  ! x264enc \
  ! identity silent=false \
  ! mp4mux movie-timescale=30 trak-timescale=30 fragment-duration=5000 \
  ! filesink location=bad-duration.mp4 2>&1

$ mp4dump bad-duration.mp4 |grep duration -B6

Expected:
[mehd] size=12+8, version=1
  duration = 30

Actual:
[mehd] size=12+8, version=1
  duration = 29
Comment 1 Alicia Boya García 2018-03-01 16:59:43 UTC
Created attachment 369152 [details] [review]
qtmux: round to nearest when computing mehd.fragment_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.
Comment 2 Sebastian Dröge (slomo) 2018-10-27 09:41:54 UTC
Comment on attachment 369152 [details] [review]
qtmux: round to nearest when computing mehd.fragment_duration

Makes sense but I'm worried about this breaking some gst-validate tests. Did you check? :)

Also the same problem should exist at the places where the duration in the tkhd and mdhd are calculated. They're basically the same calculations. We should keep them consistent. Can you check?
Comment 3 Alicia Boya García 2018-10-27 11:47:32 UTC
(In reply to Sebastian Dröge (slomo) from comment #2)
> Comment on attachment 369152 [details] [review] [review]
> qtmux: round to nearest when computing mehd.fragment_duration
> 
> Makes sense but I'm worried about this breaking some gst-validate tests. Did
> you check? :)

Yes.

> Also the same problem should exist at the places where the duration in the
> tkhd and mdhd are calculated. They're basically the same calculations. We
> should keep them consistent. Can you check?

mdhd needs no rounding (it defines its own timescale).

tkhd is computed from mdhd, but rounding was also not used (incorrect). I just updated the patch to fix that too (and ran the tests again).
Comment 4 Alicia Boya García 2018-10-27 11:50:28 UTC
Created attachment 374059 [details] [review]
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.
Comment 5 Sebastian Dröge (slomo) 2018-10-27 12:13:57 UTC
Attachment 374059 [details] pushed as 5fcb7f7 - qtmux: round to nearest when computing mehd and tkhd duration