1245375 - Use BlockDuration (when available) rather than estimating frame duration

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Matthew Gregan [:kinetik]]

Right now, WebMDemuxer calculates the duration of a frame by subtracting the frame's start time from the next frame's start time.

Whenever the next frame is not available (at the end of the file or buffer for MSE), we fall back to using the duration we calculated for the previous frame.  Being a guess, this can be incorrect.

Existing code here:  https://dxr.mozilla.org/mozilla-central/source/dom/media/webm/WebMDemuxer.cpp#504

WebM has two additional ways to signal the duration of frames which we should use in preference to estimating.  These are BlockDuration (nestegg_packet_duration) and DefaultDuration (nestegg_track_default_duration), both of which are optional.

The frame duration logic should be (according to the WebM spec):
1. Use BlockDuration if present.
2. Use DefaultDuration if present.
3. Use (next_frame - current_frame) to calculate duration.

It's not clear from the spec how to handle cases that aren't addressed by the above, so I propose:

4. Use previous frame's duration as an estimate.
  1. If the media has a duration present, clamp the final frame's duration to the media's duration.

Comment 1

[Matthew Gregan [:kinetik]]

Note that using nestegg_packet_duration relies on the following upstream fix: https://github.com/kinetiknz/nestegg/commit/6439d944761e5526e1dd637cf2cc56c110c13db8

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

This may impact external media tests also: http://pf-jenkins.qa.mtv2.mozilla.com:8080/job/mn-mse-youtube-basic-y-aurora-win7_32_64/904/artifact/logs/media_tests.html

In the above case it looks like the duration of the video has changed from the expected value due to re-evaluation of the duration. I don't think this caused the fail in the test, but wanted to make sure the issue is referenced for the future.

Comment 3

[Matthew Gregan [:kinetik]]

Bug 1261900 is going to introduce these changes as part of a larger patchset, so we can probably dupe this to that.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Mass change P2 -> P3

Comment 5

[Matthew Gregan [:kinetik]]

Done in bug 1261900