1252767 - Remove MediaDecoderStateMachine::mPendingSeek

Status: RESOLVED FIXED

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

Description

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

This somewhat simplifies our seeking pipeline for MDSM. It is often confusing to me to figure out each role of mCurrentSeek, mPendingSeek and mQueuedSeek played in the seeking process.

By calling InitiateSeek() immediately after changing mState to DECODER_STATE_SEEKING without waiting for the next state machine cycle, we can eliminate the need for mPendingSeek which is used to store the seek target and transferred to mCurrentSeek in the next state machine cycle.

Comment 1

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=60356c9f11c9

Comment 2

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

Attachment: MozReview Request: Bug 1252767 - Remove MediaDecoderStateMachine::mPendingSeek. r=cpearce.

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

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8728206 [details]
MozReview Request: Bug 1252767 - Remove MediaDecoderStateMachine::mPendingSeek. r=cpearce.

https://reviewboard.mozilla.org/r/38865/#review35687

::: dom/media/MediaDecoderStateMachine.h:531
(Diff revision 1)
> +    void Steal(SeekJob& aOther)

I know you've just moved this code, but maybe we can use a move constructor/operation here instead of a Steal() function?

::: dom/media/MediaDecoderStateMachine.cpp:957
(Diff revision 1)
>            // Non-precise seek; or a pending seek exists ; we can stop the seek

Does it still make sense for the comment to say "or a pending seek exists"?

::: dom/media/MediaDecoderStateMachine.cpp:2094
(Diff revision 1)
>    // Change state to DECODING or COMPLETED now. SeekingStopped will

This comment refers to SeekingStopped, which appears to no longer exist.

Comment 4

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

https://reviewboard.mozilla.org/r/38865/#review35687

> I know you've just moved this code, but maybe we can use a move constructor/operation here instead of a Steal() function?

Sure. The move semantics is more universal and easier to understand. I will address that in next patch.

> Does it still make sense for the comment to say "or a pending seek exists"?

Will remove the comment in next patch.

> This comment refers to SeekingStopped, which appears to no longer exist.

Will remove the comment in next patch.

Comment 5

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

In fact, I will address the move constructor in next bug which is quite orthogonal to this bug.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e950da72985

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2e950da72985