1298217 - Move some code to the entry actions for MDSM

Status: RESOLVED FIXED

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

Description

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

A step forward to bug 1295892.

By moving code to the entry actions, it is easier to define the responsibility of each state.

Comment 1

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

Attachment: Bug 1298217. Part 1 - Move the code about decoding metadata to the entry action of DECODER_STATE_DECODING_METADATA.

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

Comment 2

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

Attachment: Bug 1298217. Part 2 - Move some code to the entry action of DECODER_STATE_DORMANT.

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

Comment 3

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

Attachment: Bug 1298217. Part 3 - Remove the check for IsShutdown() from CheckIfDecodeComplete() because it is called only when mState is DECODING or BUFFERING.

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

Comment 4

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

Attachment: Bug 1298217. Part 4 - Remove the call to DispatchDecodeTasksIfNeeded() from CheckIfDecodeComplete() because we've decoded all samples.

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

Comment 5

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

Attachment: Bug 1298217. Part 5 - Move the call to |SetState(DECODER_STATE_COMPLETED)| out of CheckIfDecodeComplete() to make it easier to move more code to entry actions.

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

Comment 6

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

Attachment: Bug 1298217. Part 6 - Move some code to the entry action of DECODER_STATE_COMPLETED.

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

Comment 7

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

Attachment: Bug 1298217. Part 7 - Assert |mState != DECODER_STATE_DECODING| in StartDecoding().

StartDecoding() is called from several places where mState is proven to be not DECODER_STATE_DECODING:
1. Called by PlayStateChanged() when mState is BUFFERING.
2. Called by OnMetadataRead() where mState is DECODING_METADATA.
3. Called by SeekCompleted() where mState is SEEKING.
4. Called by RunStateMachine() when the case is BUFFERING.
5. Called by OnCDMProxyReady() when mState is WAIT_FOR_CDM.

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

Comment 8

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

Attachment: Bug 1298217. Part 8 - Move some code to the entry action of DECODER_STATE_DECODING.

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

Comment 9

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

Attachment: Bug 1298217. Part 9 - Assert |mState == DECODER_STATE_DECODING| in MaybeStartBuffering() which is only called in the DECODER_STATE_DECODING case of RunStateMachine().

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

Comment 10

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

Attachment: Bug 1298217. Part 10 - Assert |mState == DECODER_STATE_DECODING| in StartBuffering() which is only called by MaybeStartBuffering() which also asserts that.

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

Comment 11

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

Attachment: Bug 1298217. Part 11 - Move some code to the entry action of DECODER_STATE_BUFFERING.

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

Comment 12

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785807 [details]
Bug 1298217. Part 1 - Move the code about decoding metadata to the entry action of DECODER_STATE_DECODING_METADATA.

https://reviewboard.mozilla.org/r/74870/#review73000

Comment 13

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785808 [details]
Bug 1298217. Part 2 - Move some code to the entry action of DECODER_STATE_DORMANT.

https://reviewboard.mozilla.org/r/74872/#review73002

Comment 14

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785809 [details]
Bug 1298217. Part 3 - Remove the check for IsShutdown() from CheckIfDecodeComplete() because it is called only when mState is DECODING or BUFFERING.

https://reviewboard.mozilla.org/r/74874/#review73004

Comment 15

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785810 [details]
Bug 1298217. Part 4 - Remove the call to DispatchDecodeTasksIfNeeded() from CheckIfDecodeComplete() because we've decoded all samples.

https://reviewboard.mozilla.org/r/74876/#review73006

Comment 16

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785811 [details]
Bug 1298217. Part 5 - Move the call to |SetState(DECODER_STATE_COMPLETED)| out of CheckIfDecodeComplete() to make it easier to move more code to entry actions.

https://reviewboard.mozilla.org/r/74878/#review73008

Comment 17

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785812 [details]
Bug 1298217. Part 6 - Move some code to the entry action of DECODER_STATE_COMPLETED.

https://reviewboard.mozilla.org/r/74880/#review73010

Comment 18

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785813 [details]
Bug 1298217. Part 7 - Assert |mState != DECODER_STATE_DECODING| in StartDecoding().

https://reviewboard.mozilla.org/r/74882/#review73012

Comment 19

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785814 [details]
Bug 1298217. Part 8 - Move some code to the entry action of DECODER_STATE_DECODING.

https://reviewboard.mozilla.org/r/74884/#review73016

Comment 20

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785815 [details]
Bug 1298217. Part 9 - Assert |mState == DECODER_STATE_DECODING| in MaybeStartBuffering() which is only called in the DECODER_STATE_DECODING case of RunStateMachine().

https://reviewboard.mozilla.org/r/74886/#review73018

Comment 21

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785816 [details]
Bug 1298217. Part 10 - Assert |mState == DECODER_STATE_DECODING| in StartBuffering() which is only called by MaybeStartBuffering() which also asserts that.

https://reviewboard.mozilla.org/r/74888/#review73020

Comment 22

[Tzuhao Kuo [:kaku]]

Comment on attachment 8785817 [details]
Bug 1298217. Part 11 - Move some code to the entry action of DECODER_STATE_BUFFERING.

https://reviewboard.mozilla.org/r/74890/#review73022

Comment 23

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

Thanks for the review!

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ec8d889e021
Part 1 - Move the code about decoding metadata to the entry action of DECODER_STATE_DECODING_METADATA. r=kaku
https://hg.mozilla.org/integration/autoland/rev/4e6e02e3ef1a
Part 2 - Move some code to the entry action of DECODER_STATE_DORMANT. r=kaku
https://hg.mozilla.org/integration/autoland/rev/55d0d97dccd1
Part 3 - Remove the check for IsShutdown() from CheckIfDecodeComplete() because it is called only when mState is DECODING or BUFFERING. r=kaku
https://hg.mozilla.org/integration/autoland/rev/495056dc8ed5
Part 4 - Remove the call to DispatchDecodeTasksIfNeeded() from CheckIfDecodeComplete() because we've decoded all samples. r=kaku
https://hg.mozilla.org/integration/autoland/rev/e3989f634b74
Part 5 - Move the call to |SetState(DECODER_STATE_COMPLETED)| out of CheckIfDecodeComplete() to make it easier to move more code to entry actions. r=kaku
https://hg.mozilla.org/integration/autoland/rev/b06577b71a3e
Part 6 - Move some code to the entry action of DECODER_STATE_COMPLETED. r=kaku
https://hg.mozilla.org/integration/autoland/rev/8d191355cb13
Part 7 - Assert |mState != DECODER_STATE_DECODING| in StartDecoding(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/21020a8a6ea7
Part 8 - Move some code to the entry action of DECODER_STATE_DECODING. r=kaku
https://hg.mozilla.org/integration/autoland/rev/a10febe05935
Part 9 - Assert |mState == DECODER_STATE_DECODING| in MaybeStartBuffering() which is only called in the DECODER_STATE_DECODING case of RunStateMachine(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/c48f06ea74fe
Part 10 - Assert |mState == DECODER_STATE_DECODING| in StartBuffering() which is only called by MaybeStartBuffering() which also asserts that. r=kaku
https://hg.mozilla.org/integration/autoland/rev/57a2f1d5bc60
Part 11 - Move some code to the entry action of DECODER_STATE_BUFFERING. r=kaku

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7ec8d889e021
https://hg.mozilla.org/mozilla-central/rev/4e6e02e3ef1a
https://hg.mozilla.org/mozilla-central/rev/55d0d97dccd1
https://hg.mozilla.org/mozilla-central/rev/495056dc8ed5
https://hg.mozilla.org/mozilla-central/rev/e3989f634b74
https://hg.mozilla.org/mozilla-central/rev/b06577b71a3e
https://hg.mozilla.org/mozilla-central/rev/8d191355cb13
https://hg.mozilla.org/mozilla-central/rev/21020a8a6ea7
https://hg.mozilla.org/mozilla-central/rev/a10febe05935
https://hg.mozilla.org/mozilla-central/rev/c48f06ea74fe
https://hg.mozilla.org/mozilla-central/rev/57a2f1d5bc60