Closed Bug 1252767 Opened 6 years ago Closed 6 years ago

Remove MediaDecoderStateMachine::mPendingSeek

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: nobody → jwwang
Blocks: 1253184
Priority: -- → P2
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.
Attachment #8728206 - Flags: review?(cpearce) → review+
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.
In fact, I will address the move constructor in next bug which is quite orthogonal to this bug.
Blocks: 1255268
https://hg.mozilla.org/mozilla-central/rev/2e950da72985
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.