1033172 - Should mCurrentSeekTarget be reset at exit of MediaDecoderStateMachine::SeekCompleted?

Status: RESOLVED INVALID

(Core :: Audio/Video, defect)

Description

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

|mCurrentSeekTarget| is reset at exit of MediaDecoderStateMachine::SeekCompleted to indicate the end of seek operation. [1]

It could be seeking immediately again when releasing the monitor [2]. It is possible for EnqueueDecodeSeekTask() to be run and |mCurrentSeekTarget| is assigned a new value [3] before the monitor is acquired again in SeekCompleted(). In such a case, |mCurrentSeekTarget| will lose the value assigned in EnqueueDecodeSeekTask() at exit of SeekCompleted().

[1] http://hg.mozilla.org/mozilla-central/file/3a28d3bb4f55/content/media/MediaDecoderStateMachine.cpp#l2108
[2] http://hg.mozilla.org/mozilla-central/file/3a28d3bb4f55/content/media/MediaDecoderStateMachine.cpp#l2181
[3] http://hg.mozilla.org/mozilla-central/file/3a28d3bb4f55/content/media/MediaDecoderStateMachine.cpp#l1631

Comment 1

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

Hi Chris,
Can you confirm whether this is a valid case?

Comment 2

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

JW: You're right. This could be a problem. Good spotting.

We should ensure mCurrentSeekTarget is reset before we drop the monitor. mQuickBuffering too. Either that, or make the stopEvent dispatch async. I can't remember why it's synchronous.

Comment 3

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

Since bug 996465 is landed (https://hg.mozilla.org/integration/mozilla-inbound/rev/6331bc47ce77), the situation described in comment 0 will not happen anymore. Since we will only thaw scheduling at the end of SeekCompleted(), it is impossible for EnqueueDecodeSeekTask() to run before the exit of SeekCompleted().

I guess we can close this bug as INVALID.

Comment 4

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

Excellent.