1385699 - webm duration is set to Infinity when no duration is specified in metadata

Status: RESOLVED FIXED

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

Description

[Kaiido]

Attachment: duration.html

When loading a 'video/webm' media generated by the MediaRecorder API, the `duration` property of the video returns Infinity.

These files don't have a 'duration' set in their metadata (if I got it right, it's because of the way MediaRecorder works : metadata are written first).

This affects both files generated by chrome[1] and files generated by Firefox.

Example output of `ffmpeg -i` on such a file :

Input #0, matroska,webm, from 'video.webm':
  Metadata:
    encoder         : QTmuxingAppLibWebM-0.0.1
  Duration: N/A, start: 0.000000, bitrate: N/A
    Stream #0:0(eng): Video: vp8, yuv420p(progressive), 640x480, SAR 1:1 DAR 4:3, 1k tbr, 1k tbn, 1k tbc (default)

On version 54 and above, Firefox were able to retrieve the correct duration anyway ; since 55 it returns Infinity.

Note that chrome always returned Infinity, but there is a workaround[2] for them, which consists in setting the currentTime of the video to a large number. The duration is then correctly set. FF55+ doesn't allow this workaround to work : duration is always Infinity, even when it knows the actual duration.

The current workaround I use is to grab the value of currentTime after I set it to the large number. But currentTime property can't be overridden by scripts, so this implies an ugly _duration property...

To make things even worse, when the ended event fires, the currentTime property too is set to Infinity.

So all in all, the best would be to retrieve the previous behavior (automagically grab the duration), and if it's really too hard, at least update the duration once it is known.

[1](https://bugs.chromium.org/p/chromium/issues/detail?id=642012)
[2](https://stackoverflow.com/questions/38443084)

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Can you upload the file generated by MediaRecorder? It would be nice for you to update the test case to exclude the MediaRecorder API because the issue is in how our media playback stack fetches duration from the metadata, not in how the file is generated.

Comment 2

[Kaiido]

Attachment: webm movie from MediaRecorder (no duration metadata)

Comment 3

[Kaiido]

Attachment: duration.html

updated to use static video file as requested

Comment 4

[Kaiido]

Attachment: duration.html

Sorry always have trouble with relative urls not working here...

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Attachment: test_duration.html

A simplified version of the test case.

Firefox output:
got durationchange, duration=Infinity, currentTime=0
got loadedmetadata, duration=Infinity, currentTime=0
got ended, duration=Infinity, currentTime=Infinity

Chrome output:
got durationchange, duration=Infinity, currentTime=0
got loadedmetadata, duration=Infinity, currentTime=0
got durationchange, duration=6.558, currentTime=3.981834
got ended, duration=6.558, currentTime=6.558

There are several issues revealed by this bug:
1. We have MediaDecoder::mInfiniteStream==true to present an infinite stream. However, mDuration==inf can also present an infinite stream. This causes inconsistency and fails to mark the stream as finite when playback reaches the end.

2. http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/dom/media/MediaDecoderStateMachine.cpp#2009-2012
MDSM adjust playback position to max(clockTime, mMaster->Duration()) when playback reaches the end. Since mMaster->Duration() returns inf in this case, we end up having mediaElement.currentTime==inf.

3. http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/dom/media/MediaDecoderStateMachine.cpp#2252
It is bad to modify mDuration directly which should the result of RecomputeDuration(). Changing mDuration directly will break data consistency because any changes to mExplicitDuration or mObservedDuration will overwrite mDuration.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1385699. P1 - don't adjust playback position to the duration in MDSM at the end of playback.

We will let MediaDecoder do the final adjustment.

Review commit: https://reviewboard.mozilla.org/r/164222/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164222/

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1385699. P2 - rewrite the code of calculating duration in MDSM.

We pretty much rewrite the whole logic of duration calculation of MDSM.
The new logic is much simpler for we have only one duration to manage
which is mDuration. Below is the details of the code changes:

1. remove the mExplicitDuration mirror since it should be handled in MediaDecoder.
2. remove mObservedDuration and mDuration will take its place.
3. mDuration is updated from:
   a. metadata
   b. playback position as playback progresses.
   c. buffer ranges.
4. change mDuration to be finite when playback reaches the end.

Review commit: https://reviewboard.mozilla.org/r/164224/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164224/

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1385699. P3 - remove MediaDecoder::SetInfinite() and related code.

Since inf can be encoded in MDSM::mDuration, we don't need an additional flag
in MediaDecoder to indicate 'infinite' anymore. Note duration change from infinite
to finite is handled by MDSM, so we can remove the explicit calls to SetInfinite(false).

Review commit: https://reviewboard.mozilla.org/r/164226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164226/

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1385699. P4 - MediaDecoder::mExplicitDuration doesn't need to be a canonical anymore.

Review commit: https://reviewboard.mozilla.org/r/164228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164228/

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1385699. P5 - fix AudioEndTime() and VideoEndTime().

AudioEndTime() returns 0 if the media sink is not started. This is wrong
because the audio/video end time should be at least the current position
to ensure playback won't go backwards.

http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/dom/media/MediaDecoderStateMachine.cpp#2009-2012
This fix the case where we wrongly set playback position to 0 after seeking
to the end of resource.

Review commit: https://reviewboard.mozilla.org/r/164230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164230/

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8893192 [details]
Bug 1385699. P1 - don't adjust playback position to the duration in MDSM at the end of playback.

https://reviewboard.mozilla.org/r/164222/#review170060

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8893193 [details]
Bug 1385699. P2 - rewrite the code of calculating duration in MDSM.

https://reviewboard.mozilla.org/r/164224/#review170064

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8893194 [details]
Bug 1385699. P3 - remove MediaDecoder::SetInfinite() and related code.

https://reviewboard.mozilla.org/r/164226/#review170066

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8893195 [details]
Bug 1385699. P4 - MediaDecoder::mExplicitDuration doesn't need to be a canonical anymore.

https://reviewboard.mozilla.org/r/164228/#review170068

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8893196 [details]
Bug 1385699. P5 - fix AudioEndTime() and VideoEndTime().

https://reviewboard.mozilla.org/r/164230/#review170070

Nice, this all makes handling of infinite much better. Thanks!

Comment 16

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the reviews!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e10236332d1d
P1 - don't adjust playback position to the duration in MDSM at the end of playback. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/90612a88de7c
P2 - rewrite the code of calculating duration in MDSM. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/c03cc5e6a7c3
P3 - remove MediaDecoder::SetInfinite() and related code. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/3741783d35c1
P4 - MediaDecoder::mExplicitDuration doesn't need to be a canonical anymore. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/01a413050ab7
P5 - fix AudioEndTime() and VideoEndTime(). r=cpearce

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e10236332d1d
https://hg.mozilla.org/mozilla-central/rev/90612a88de7c
https://hg.mozilla.org/mozilla-central/rev/c03cc5e6a7c3
https://hg.mozilla.org/mozilla-central/rev/3741783d35c1
https://hg.mozilla.org/mozilla-central/rev/01a413050ab7

Comment 19

[Jean-Yves Avenard [:jya]]

I believe that the removal of explicit duration (which is used with MSE, where we never need to set the duration in the MDSM nor guess the duration) will regress the MSE handling of duration.

Comment 20

[JW Wang [:jwwang] [:jw_wang]]

Do you have a test case to repro the regression?