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




10 years ago
4 years ago


(Reporter: jrmuizel, Assigned: cpearce)


Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)



(1 attachment, 1 obsolete attachment)

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.
Posted 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 #458849 - Attachment is obsolete: true
Attachment #459169 - Flags: review+


9 years ago
Attachment #459169 - Flags: approval2.0?


9 years ago
blocking2.0: --- → final+


9 years ago
Attachment #459169 - Flags: approval2.0?
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.