1308078 - Don't decode metadata again when exiting the dormant state

Status: RESOLVED FIXED

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

Description

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

MDSM enters the dormant state only after decoding metadata. There is no need to decode metadata again when going out of the dormant state. It should transition to DECODING_FIRSTFRAME to start decoding.

Comment 1

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

Attachment: Bug 1308078. Part 1 - Don't decode metadata again when exiting the dormant state.

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

Comment 2

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

Attachment: Bug 1308078. Part 2 - no need to store mQueuedSeek in WaitForCDMState::HandleDormant.

Because we haven't decoded any data and are at position 0, there is no need to seek when exiting the dormant state.

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

Comment 3

[Tzuhao Kuo [:kaku]]

Comment on attachment 8800105 [details]
Bug 1308078. Part 1 - Don't decode metadata again when exiting the dormant state.

https://reviewboard.mozilla.org/r/85138/#review83694

Comment 4

[Tzuhao Kuo [:kaku]]

Comment on attachment 8800106 [details]
Bug 1308078. Part 2 - no need to store mQueuedSeek in WaitForCDMState::HandleDormant.

https://reviewboard.mozilla.org/r/85140/#review83696

Could we also apply this to DecodingFirstFrameState if nothing has been decoded yet?

Comment 5

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

Comment on attachment 8800106 [details]
Bug 1308078. Part 2 - no need to store mQueuedSeek in WaitForCDMState::HandleDormant.

https://reviewboard.mozilla.org/r/85140/#review83696

We can't. Suppose we have decoded the 1st audio sample and are waiting for the 1st video sample to finish decoding first frames. And then we enter dormant without storing the queued seek. When we exit the dormant state, we will enter DecodingFirstFrameState again. The 1st audio sample we receive is actually the 2nd sample since we didn't seek to re-position the demuxer.

Comment 6

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

Thanks for the review!

Comment 7

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

Comment on attachment 8800105 [details]
Bug 1308078. Part 1 - Don't decode metadata again when exiting the dormant state.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85138/diff/1-2/

Comment 8

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

Comment on attachment 8800106 [details]
Bug 1308078. Part 2 - no need to store mQueuedSeek in WaitForCDMState::HandleDormant.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/85140/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c26806bad1b7
Part 1 - Don't decode metadata again when exiting the dormant state. r=kaku
https://hg.mozilla.org/integration/autoland/rev/a6a7e9eca986
Part 2 - no need to store mQueuedSeek in WaitForCDMState::HandleDormant. r=kaku

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/c26806bad1b7
https://hg.mozilla.org/mozilla-central/rev/a6a7e9eca986

Comment 13

[Marco Castelluccio [:marco]]

Does this affect 51?
There are crashes on 51 with this signature, I'm not sure if it's the same crash.

Comment 14

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

No. The crash is caused by bug 1307677 and fixed by bug 1308078.