543769 - Video starts playing, plays for a second and then starts buffering

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This happens on this page:
http://hacks.mozilla.org/2010/02/an-html5-offline-image-editor-and-uploader-application/

Comment 1

[Matthew Gregan [:kinetik]]

This is a non-autobuffer video, so we stop downloading immediately after we've got the first frame and don't get another chance to buffer until the user clicks play.  We need to make the behaviour smarter in this case because it's possible we buffered just enough during the initial load that we won't start buffering more data until we've played what we downloaded and decoded earlier.

Comment 2

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

Attachment: Patch v1

I solve this by moving the nsBuiltinDecoderStateMachine into BUFFERING state upon the first play of a non-autobuffer media. I also tweak the buffering logic so that we move out of buffering state when we think we can play through the entire media, so that the wait after the first play is initiated (and at other times) is not longer than necessary.

* Move logic to determine if we can play through the media from nsHTMLMediaElement::UpdateReadyStateForData() to new function nsMediaDecoder::CanPlayThrough().
* Add PRBool parameter to nsMediaDecoder::Resume(). When PR_TRUE we move the decoder state machine into buffering state before beginning playback, rather than trying to playback immediately. In nsHTMLMediaElement::StopSuspendingAfterFirstFrame() we call Resume(PR_TRUE), so we'll start buffering immediately when we play a non-autobuffer media. In the only other call to Resume(), after a media element is retrieved from bfcache, we call with PR_FALSE, so behaviour doesn't change in that case.
* In the nsBuiltinDecoderStateMachine::Run() buffering case, we stop buffering if we think we can play through the entire media (as per nsMediaDecoder::CanPlayThrough()). This prevents us from buffering longer than we need to, in particular during the initial play of a non-autobuffer media.
* Refactor the "move decoder state machine into buffering state" code out of nsBuiltinDecoderStateMachine::Run() play case, into its own method so we can call it from functions which implement nsMediaDecoder::Resume().
* I haven't implemented this for nsWaveDecoder. WAVs should be small, and so I don't think they'll be used in a way that this bug would be an issue. Our buffering logic doesn't seem to be working correctly for WAV either, and we'd have to fix that first (Bug 580461).

Comment 3

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

Comment on attachment 458849 [details] [diff] [review]
Patch v1

Call Buffer() StartBuffering(), so it's more clearly a verb.

+  return (timeToDownload <= timeToPlay) ? PR_TRUE : PR_FALSE;

lose the ? PR_TRUE : PR_FALSE.

Comment 4

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

Attachment: Patch v1 with review comments addressed.

Comment 5

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

http://hg.mozilla.org/mozilla-central/rev/8d7913fe2ecf