1299064 - Remove unnecessary calls to ScheduleStateMachine() following SetState(DECODER_STATE_DECODING)

Status: RESOLVED FIXED

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

Description

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

The entry action of DECODER_STATE_DECODING calls ScheduleStateMachine(). We can remove the calls following |SetState(DECODER_STATE_DECODING)|.

Comment 1

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

Attachment: Bug 1299064. Part 1 - move |SetState(nextState)| and ScheduleStateMachine() to the bottom of SeekCompleted().

UpdatePlaybackPositionInternal(), |mQuickBuffering = false| and |mMediaSink->Redraw| should belong to
the exit action of SEEKING. By change the order of the statements, we have a better definition/scope
for the jobs of each state.

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

Comment 2

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

Attachment: Bug 1299064. Part 2 - remove the if/else statement at the end of SeekCompleted().

|SetState(nextState)| is the equivalent of the if/else.

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

Comment 3

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

Attachment: Bug 1299064. Part 3 - remove the calls to ScheduleStateMachine() following |SetState(DECODER_STATE_DECODING)|.

Note nextState is either DECODER_STATE_COMPLETED or DECODER_STATE_DECODING in SeekCompleted().
Their entry actions call ScheduleStateMachine() directly or indirectly.

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

Comment 4

[Tzuhao Kuo [:kaku]]

Comment on attachment 8786610 [details]
Bug 1299064. Part 1 - move |SetState(nextState)| and ScheduleStateMachine() to the bottom of SeekCompleted().

https://reviewboard.mozilla.org/r/75534/#review73510

Comment 5

[Tzuhao Kuo [:kaku]]

Comment on attachment 8786611 [details]
Bug 1299064. Part 2 - remove the if/else statement at the end of SeekCompleted().

https://reviewboard.mozilla.org/r/75536/#review73512

Comment 6

[Tzuhao Kuo [:kaku]]

Comment on attachment 8786612 [details]
Bug 1299064. Part 3 - remove the calls to ScheduleStateMachine() following |SetState(DECODER_STATE_DECODING)|.

https://reviewboard.mozilla.org/r/75538/#review73514

Comment 7

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

Thanks for the review!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9f208e04b62
Part 1 - move |SetState(nextState)| and ScheduleStateMachine() to the bottom of SeekCompleted(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/437650642dd8
Part 2 - remove the if/else statement at the end of SeekCompleted(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/0e3958b3ee0f
Part 3 - remove the calls to ScheduleStateMachine() following |SetState(DECODER_STATE_DECODING)|. r=kaku

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/c9f208e04b62
https://hg.mozilla.org/mozilla-central/rev/437650642dd8
https://hg.mozilla.org/mozilla-central/rev/0e3958b3ee0f