493187 - readyState stuck at HAVE_CURRENT_DATA after seeking when media cache is full

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Justin Dolske [:Dolske]]

Spun out from bug 492213.

Loading the large OGG file (linked in URL field) results in the progress bar advancing about halfway and then stopping. It's a 118MB file, so the 50MB cache becomes full and buffering stops.

While it's still buffering, I see that .readyState will ready HAVE_ENOUGH_DATA, and after seeking within the buffered range .readyState will return to the HAVE_ENOUGH_DATA state. But once buffering has stopped (due to the full cache), .readyState and a seek is performed, .readyState gets stuck in the HAVE_CURRENT_DATA state.

At the very least, it should be in the HAVE_FUTURE_DATA state. It should probably also advance to HAVE_ENOUGH_DATA, although that might be tricky to do accurately when seeking to a spot near the end of the buffered data.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

What happens is that we come out of seeking and the frame queue is empty. We run UpdateReadyStateForData which, since we have no frames queued, says we should be in HAVE_CURRENT_DATA. That's OK. Then some frames get decoded and we should be able to advance to HAVE_FUTURE_DATA at least, but in this case, where no data is arriving, nothing triggers UpdateReadyStateForData so readyState doesn't change.

This patch triggers UpdateReadyStateForData whenever the frame queue changes from empty to non-empty. We should do this because UpdateReadyStateForData depends on the emptiness of the frame queue.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2 with test

Writing a test for this showed me some new issues.

I discovered bug 493443 and fixed that there --- we weren't entering HAVE_CURRENT_DATA during seeks on fully buffered videos, which seems anomalous.

Then I discovered that under many conditions the first version of this patch doesn't work. In particular, if you seek while paused, HaveNextFrameData returns false by design while we're in DECODER_STATE_SEEKING, and then in DECODER_STATE_DECODING we never get around to calling QueueDecodedFrames because DECODER_STATE_SEEKING put a frame in the queue and PlayFrame knows we're paused so just waits for something to change.

So I've changed HaveNextFrameData to ignore the current state, but be smarter about inspecting the frame queue. If the frame queue has just one frame whose time is the current time, it now returns false.

The remaining problem is getting frames from the decoder thread into mDecodedFrames while we're paused. So I have DECODER_STATE_DECODING unconditionally try QueueDecodedFrames in each iteration. I also have the decoder thread do a NotifyAll on the monitor after each oggplay_step_decoding, so PlayFrame wakes up and we'll return for another iteration of the DECODER_STATE_DECODING logic. This means all available frames will get copied to mDecodedFrames until it's full, even when paused.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The test here also tests the patch in bug 493443, because without the patch in bug 493443 our readyState remains HAVE_ENOUGH_DATA throughout the seek operation and so canplaythrough does not fire after the seek, as this test expects.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/269ec95e8516

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d94fb5c713c4