Closed Bug 1270698 Opened 4 years ago Closed 4 years ago

MDSM sometimes doesn't enter buffering mode when running out of decoded audio/video data

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

Details

Attachments

(3 files)

No description provided.
Assignee: nobody → jwwang
This patch adds a 2s sleep after rejecting the promise with WAITING_FOR_DATA and makes it super easy to repro the problem.

repro steps:
1. apply the patch
2. run |./mach mochitest dom/media/mediasource/test/test_DrainOnMissingData_mp4.html|

result: test timeout
https://hg.mozilla.org/mozilla-central/file/369a5ee3a2880a4a98df3a00bf3db8d8f36b181b/dom/media/MediaDecoderStateMachine.cpp#l1016

The problem is:
|shouldBuffer| depends on the values of OutOfDecodedAudio(), mAudioWaitRequest.Exists(), OutOfDecodedVideo() and mVideoWaitRequest.Exists(). We need to check the need of buffering whenever these values change. However, MaybeStartBuffering() is run in the pop listeners which don't care for changes in m{Audio,Video}WaitRequest.Exists().

It is easy to check buffering when m{Audio,Video}WaitRequest.Exists() changes. However, OutOfDecodedAudio() depends on |mMediaSink->HasUnplayedFrames(TrackInfo::kAudioTrack)| which is much harder to monitor.

I will take a much easier approach which calls MaybeStartBuffering() periodically in RunStateMachine() to ensure MDSM enters buffering when running out of decoded audio/video data.

This is not optimal because we don't enter buffering immediately when running out of decoded data. However, given RunStateMachine() is run at most 40ms once, the delay should be insignificant to the user.
Attachment #8749503 - Flags: review?(cpearce)
Even with the P2 fix, we still have a timeout in test_WaitingToEndedTransition_mp4.html. I believe this is exactly the same issue in bug 1258922.

repro steps:
1. apply P1 ~ P3.
2. run |./mach mochitest dom/media/mediasource/test/test_WaitingToEndedTransition_mp4.html|

result:
test timeout with the logs:

WARNING: Decoder=12aaac800 GetMediaTime=3853061 GetClock=3853061 mState=BUFFERING mPlayState=3 mDecodingFirstFrame=0 IsPlaying=0 mAudioStatus=idle mVideoStatus=pending mDecodedAudioEndTime=3854511 mDecodedVideoEndTime=2270000 mIsAudioPrerolling=0 mIsVideoPrerolling=0

MDSM is stuck in BUFFERING with video promise pending (mVideoStatus=pending).
Flags: needinfo?(jyavenard)
This may get fixed with bug 1269408 and bug 1258922 (which are unfortunately dependent on this one)
Flags: needinfo?(jyavenard)
Comment on attachment 8749503 [details] [diff] [review]
1270698_part2_fix.patch

Review of attachment 8749503 [details] [diff] [review]:
-----------------------------------------------------------------

fly by review, this has fixed all my intermittents. and I'd like this in asap.
Attachment #8749503 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/1a00d3da0e9e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.