Closed Bug 543769 Opened 12 years ago Closed 11 years ago

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


(Core :: Audio/Video, defect)

Not set



Tracking Status
blocking2.0 --- final+


(Reporter: jrmuizel, Assigned: cpearce)




(1 file, 1 obsolete file)

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.
Attached patch Patch v1 (obsolete) — Splinter Review
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).
Assignee: nobody → chris
Attachment #458849 - Flags: review?(roc)
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.
Attachment #458849 - Flags: review?(roc) → review+
Attachment #459169 - Flags: approval2.0?
blocking2.0: --- → final+
Attachment #459169 - Flags: approval2.0?
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.