Closed Bug 1161901 Opened 9 years ago Closed 9 years ago

Move more stuff to the state machine task queue

Categories

(Core :: Audio/Video, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(5 files)

Continuing the long march.
Attachment #8601905 - Flags: review?(jwwang) → review+
Attachment #8601906 - Flags: review?(jwwang) → review+
Attachment #8601907 - Flags: review?(jwwang) → review+
Attachment #8601908 - Flags: review?(jwwang) → review+
Attachment #8601909 - Flags: review?(jwwang) → review+
(In reply to Bobby Holley (:bholley) from comment #6)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4ec57886635

Hm, so Part 4 is hitting the this assertion in GetClock:

https://hg.mozilla.org/mozilla-central/annotate/60349cbc3d4e/dom/media/MediaDecoderStateMachine.cpp#l2912

mAudioCaptured is true, but the state is DECODER_STATE_DECODING.

jww, it looks like you were involved in this assertion as it is. The opt-out for DECODER_STATE_SEEKING seems suspicious to me - is there a clear reason why this only happens in SEEKING state? What is the broken invariant that I should be looking for in this case? I'm not super familiar with MDSM::SendStreamData, and I'm hoping to avoid diving into it too much. ;-)
Flags: needinfo?(jwwang)
https://hg.mozilla.org/mozilla-central/annotate/1c6b15488543/dom/media/MediaDecoderStateMachine.cpp

Once upon a time...
1. MediaDecoderStateMachine::Seek() calls mDecoder->RecreateDecodedStream() synchorously.
2. mDecoder->RecreateDecodedStream() changes the return value of GetClock() immediately.
3. InitiateSeek() => StopPlayback() => GetClock() returns a time smaller than GetMediaTime() if step 1 seeks to an earlier time.

Therefore I opt out |mAudioCaptured && mState == DECODER_STATE_SEEKING|.

However, after the introduction of tail dispatch, InitiateSeek() => StopPlayback() should always happen before MediaDecoder::RecreateDecodedStream(), we should be able to remove |mAudioCaptured && mState == DECODER_STATE_SEEKING| from the assertion in GetClock().

Btw, test_peerConnection_capturedVideo.html doesn't seek at all. I wonder if it is some other bug.
Flags: needinfo?(jwwang)
Btw, we already have bug 1161816.
(In reply to JW Wang [:jwwang] from comment #9)
> Btw, we already have bug 1161816.

Ok, so this is already happening, and this bug just makes it happen more reliably. Would you be ok with loosening that assertion in the mAudioCaptured case and filing a followup bug to sort it out? I'd like to get this stuff landed.
Flags: needinfo?(jwwang)
https://hg.mozilla.org/try/file/a04e8995ee20/dom/media/MediaDecoderStateMachine.h#l155
I wonder this is the cause.

Sure we can loose the restriction and fix it later.
Flags: needinfo?(jwwang)
Blocks: 1162381
You need to log in before you can comment on or make changes to this bug.