1325905 - Remove calls to MDSM::Ensure{Audio,Video}DecodeTaskQueued from DecodingFirstFrameState

Status: RESOLVED FIXED

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

Description

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

Ensure{Audio,Video}DecodeTaskQueued checks mState and break the encapsulation. DecodingFirstFrameState should just call Request{Audio,Video} to decode first frames.

Comment 1

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

Attachment: Bug 1325905. Part 1 - add assertions to MDSM::Request{Audio,Video}Data.

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

Comment 2

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

Attachment: Bug 1325905. Part 2 - move IsWaitForDataSupported() assertions into MediaDecoderReaderWrapper.

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

Comment 3

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

Attachment: Bug 1325905. Part 3 - split HandleNotDecoded() into HandleAudioNotDecoded() and HandleVideoNotDecoded().

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

Comment 4

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

Attachment: Bug 1325905. Part 4 - let DecodingFirstFrameState override Handle{Audio,Video}NotDecoded.

We want to call Request{Audio,Video}Data() instead of Ensure{Audio,Video}DecodeTaskQueued
which checks mState and breaks the encapsulation of the state objects.

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

Comment 5

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

Attachment: Bug 1325905. Part 5 - let DecodingFirstFrameState override Handle{Audio,Video}Waited.

We want to call Request{Audio,Video}Data() instead of Ensure{Audio,Video}DecodeTaskQueued
which checks mState and breaks the encapsulation of the state objects.

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

Comment 6

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

Attachment: Bug 1325905. Part 6 - let DecodingFirstFrameState::Enter() call Request{Audio,Video} instead.

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

Comment 7

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

Attachment: Bug 1325905. Part 7 - add assertions to ensure Ensure{Audio,Video}DecodeTaskQueued is never called by DecodingFirstFrameState.

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

Comment 8

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821957 [details]
Bug 1325905. Part 2 - move IsWaitForDataSupported() assertions into MediaDecoderReaderWrapper.

https://reviewboard.mozilla.org/r/101036/#review101760

You don't want the MDSM to care about "IsWaitForDataSupported()", how about just remove it form reader wrapper?

Comment 9

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821958 [details]
Bug 1325905. Part 3 - split HandleNotDecoded() into HandleAudioNotDecoded() and HandleVideoNotDecoded().

https://reviewboard.mozilla.org/r/101038/#review101766

Comment 10

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821956 [details]
Bug 1325905. Part 1 - add assertions to MDSM::Request{Audio,Video}Data.

https://reviewboard.mozilla.org/r/101034/#review101768

Comment 11

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821959 [details]
Bug 1325905. Part 4 - let DecodingFirstFrameState override Handle{Audio,Video}NotDecoded.

https://reviewboard.mozilla.org/r/101040/#review101762

Comment 12

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821960 [details]
Bug 1325905. Part 5 - let DecodingFirstFrameState override Handle{Audio,Video}Waited.

https://reviewboard.mozilla.org/r/101042/#review101770

Comment 13

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821961 [details]
Bug 1325905. Part 6 - let DecodingFirstFrameState::Enter() call Request{Audio,Video} instead.

https://reviewboard.mozilla.org/r/101044/#review101772

Comment 14

[Tzuhao Kuo [:kaku]]

Comment on attachment 8821962 [details]
Bug 1325905. Part 7 - add assertions to ensure Ensure{Audio,Video}DecodeTaskQueued is never called by DecodingFirstFrameState.

https://reviewboard.mozilla.org/r/101046/#review101774

Comment 15

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

Comment on attachment 8821957 [details]
Bug 1325905. Part 2 - move IsWaitForDataSupported() assertions into MediaDecoderReaderWrapper.

https://reviewboard.mozilla.org/r/101036/#review101760

Good point! I will remove the function.

Comment 16

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

Comment on attachment 8821956 [details]
Bug 1325905. Part 1 - add assertions to MDSM::Request{Audio,Video}Data.

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

Comment 17

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

Comment on attachment 8821957 [details]
Bug 1325905. Part 2 - move IsWaitForDataSupported() assertions into MediaDecoderReaderWrapper.

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

Comment 18

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

Comment on attachment 8821958 [details]
Bug 1325905. Part 3 - split HandleNotDecoded() into HandleAudioNotDecoded() and HandleVideoNotDecoded().

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

Comment 19

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

Comment on attachment 8821959 [details]
Bug 1325905. Part 4 - let DecodingFirstFrameState override Handle{Audio,Video}NotDecoded.

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

Comment 20

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

Comment on attachment 8821960 [details]
Bug 1325905. Part 5 - let DecodingFirstFrameState override Handle{Audio,Video}Waited.

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

Comment 21

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

Comment on attachment 8821961 [details]
Bug 1325905. Part 6 - let DecodingFirstFrameState::Enter() call Request{Audio,Video} instead.

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

Comment 22

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

Comment on attachment 8821962 [details]
Bug 1325905. Part 7 - add assertions to ensure Ensure{Audio,Video}DecodeTaskQueued is never called by DecodingFirstFrameState.

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

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dce407eb2097
Part 1 - add assertions to MDSM::Request{Audio,Video}Data. r=kaku
https://hg.mozilla.org/integration/autoland/rev/6cc82c8ec58f
Part 2 - move IsWaitForDataSupported() assertions into MediaDecoderReaderWrapper. r=kaku
https://hg.mozilla.org/integration/autoland/rev/baa5c68871ef
Part 3 - split HandleNotDecoded() into HandleAudioNotDecoded() and HandleVideoNotDecoded(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/26f72dcb2f40
Part 4 - let DecodingFirstFrameState override Handle{Audio,Video}NotDecoded. r=kaku
https://hg.mozilla.org/integration/autoland/rev/fc9bd237bd08
Part 5 - let DecodingFirstFrameState override Handle{Audio,Video}Waited. r=kaku
https://hg.mozilla.org/integration/autoland/rev/5da156cd377b
Part 6 - let DecodingFirstFrameState::Enter() call Request{Audio,Video} instead. r=kaku
https://hg.mozilla.org/integration/autoland/rev/1ac358b52045
Part 7 - add assertions to ensure Ensure{Audio,Video}DecodeTaskQueued is never called by DecodingFirstFrameState. r=kaku

Comment 24

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

https://hg.mozilla.org/mozilla-central/rev/dce407eb2097
https://hg.mozilla.org/mozilla-central/rev/6cc82c8ec58f
https://hg.mozilla.org/mozilla-central/rev/baa5c68871ef
https://hg.mozilla.org/mozilla-central/rev/26f72dcb2f40
https://hg.mozilla.org/mozilla-central/rev/fc9bd237bd08
https://hg.mozilla.org/mozilla-central/rev/5da156cd377b
https://hg.mozilla.org/mozilla-central/rev/1ac358b52045