Closed Bug 1120282 Opened 6 years ago Closed 6 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Note, this is a regression introduced by bug 1097375
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.)"
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.
Attachment #8547460 - Flags: review?(matt.woodrow)
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Attachment #8547460 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/797403411392
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
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.
Attachment #8547460 - Flags: approval-mozilla-beta?
Attachment #8547460 - Flags: approval-mozilla-aurora?
Attachment #8547460 - Flags: approval-mozilla-beta?
Attachment #8547460 - Flags: approval-mozilla-beta+
Attachment #8547460 - Flags: approval-mozilla-aurora?
Attachment #8547460 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.