1120282 - [MSE] "durationchange" event could be fired out of order and duration incorrectly set to an older value.

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

This behaviour can be seen in the following sample code:

appendBuffer(initData);
ms.duration = 5;

initData contains the init segment ; the MediaDecoderStateMachine will read the metadata and call MediaSourceReader::ReadMetadata which in turns call MediaSourceDecoder::SetDecodedDuration.
MediaSourceDecoder::SetDecodedDuration calls MediaDecoderStateMachine::SetDuration

As this is done in a decoder thread ; the durationchange (with infinity duration) is queued to the main thread.

Now ms.duration = 5 gets to run.
It will call MediaSourceDecoder::SetDecodedDuration, and in turn MediaDecoderStateMachine::SetDuration and as it's running in the main, durationchange is run immediately.

Finally, the queued duration change gets to run; which sets the duration to infinity.

The video duration is now set to infinity rather than 5.

This is similar to bug 1118589 in that the duration gets to be incorrectly set under some circumstances, but it's happening differently.

Comment 1

[Jean-Yves Avenard [:jya]]

Note, this is a regression introduced by bug 1097375

Comment 2

[Jean-Yves Avenard [:jya]]

Something of relevance:
In the spec we have:
"6.Update the media controller duration to new duration and run the HTMLMediaElement duration change algorithm."

HTMLMediaElement duration change algorithm:
http://www.w3.org/TR/html5/embedded-content-0.html#durationChange:
"(The event is not fired when the duration is reset as part of loading a new media resource.)"

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Do not fire durationchange during call to ReadMetadata

Do not fire/queue a durationchange in ReadMetadata. The durationchange event will be fired later by HTMLMediaElement when loadedmetadata is fired. The prevents the race of two durationchange being queued in two different threads.

Comment 4

[Jean-Yves Avenard [:jya]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/797403411392

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/797403411392

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 8547460 [details] [diff] [review]
Do not fire durationchange during call to ReadMetadata

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing, sites more likely to serve flash video.
[Describe test coverage new/current, TBPL]: Landed on m-c.
[Risks and why]: MSE-specific, so low.
[String/UUID change made/needed]: None.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/44273aa88b5a
https://hg.mozilla.org/releases/mozilla-beta/rev/9bb138f23d58