1128811 - Seeking gets stuck if MediaDecoderStateMachine is in RequestStatus::Waiting mode

Status: RESOLVED FIXED

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

Description

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

While playing this video:
https://www.youtube.com/watch?v=qQD6kM-ScIE

I hit this error in the console:

[6624] WARNING: 'failure == prevFailure', file c:/Users/cpearce/src/mozilla/purple/dom/media/fmp4/MP4Reader.cpp, line 97
[6624] WARNING: Failed reading the same block twice: offset=478601, count=8: file c:/Users/cpearce/src/mozilla/purple/dom/media/fmp4/MP4Reader.cpp, line 99

Video also froze, and I only had audio playing back.

I had played a little bit of video, then seeked to about 3:15, and then I hit that failure.

I could not reproduce it again.

Comment 1

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

This was with a freshly updated m-c; 70cfc57476b7.

Comment 2

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

I can reliably reproduce this like so:
1. Load https://www.youtube.com/watch?v=nz74OzjwXlA
2. Seek to near the end, say 3:25 or so.
3. Play until playback reaches the end.
4. Seek to earlier in the media, say 2:45, the error will trigger, and we'll get no video playback. Seems to happen more when the seek target is unbuffered.

Comment 3

[Bobby Holley (:bholley)]

That error basically just means that the demuxer errored out because the data wasn't there. It doesn't necessarily mean that we've done something wrong, though it seems like we probably should be judiciously using GetBuffered to avoid requesting samples that we don't have. As far as I gathered, Anthony made the demuxer conservative in its reporting, so we should never error out trying to read data that GetBuffered tells us we have.

I can reproduce with the STR in comment 2. Will capture a log and have a look.

Comment 4

[Bobby Holley (:bholley)]

Attachment: chinesesurferlog.txt.zip

Attaching a log if the problem as reproduced by following Chris' STR.

Comment 5

[Bobby Holley (:bholley)]

So the error with the chinese video has to do with the fact that we don't reject data wait promises in MediaSourceReader::Seek. So the seek never completes, because the MDSM thinks it's still waiting for dataa and refuses to dispatch new sample requests.

I actually noticed this by inspection a few days ago, and had a patch to fix this already in my queue for bug 1129246. So I'm just going to transplant that patch over here.

The "Failed reading the same block twice" warning is still worth investigating at some point, but orthogonal to the concrete bug that Chris identified here.

Comment 6

[Bobby Holley (:bholley)]

Attachment: Reject data wait promises when we seek. v1

Comment 7

[Bobby Holley (:bholley)]

This already had a green try run in https://treeherder.mozilla.org/#/jobs?repo=try&revision=579d06a5a63c

Comment 8

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/baffa46b36fe

ni rillian for uplift.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/baffa46b36fe

Comment 10

[Ralph Giles (:rillian)]

Comment on attachment 8559524 [details] [diff] [review]
Reject data wait promises when we seek. v1

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Playback stalls after seeking on YouTube.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: This affects non-MSE video playback, but is straightforward. Risk seems low at this point.
[String/UUID change made/needed]: None.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8559524 [details] [diff] [review]
Reject data wait promises when we seek. v1

Been on m-c for 2 weeks already. Looks safe. Aurora+

Comment 12

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dc9a1813588e