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

RESOLVED FIXED

Status

()

Core
Audio/Video
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: jrmuizel, Assigned: cpearce)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
This happens on this page:
http://hacks.mozilla.org/2010/02/an-html5-offline-image-editor-and-uploader-application/
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.
(Assignee)

Comment 2

7 years ago
Created attachment 458849 [details] [diff] [review]
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).
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+
(Assignee)

Comment 4

7 years ago
Created attachment 459169 [details] [diff] [review]
Patch v1 with review comments addressed.
Attachment #458849 - Attachment is obsolete: true
Attachment #459169 - Flags: review+
(Assignee)

Updated

7 years ago
Attachment #459169 - Flags: approval2.0?
blocking2.0: --- → final+
Attachment #459169 - Flags: approval2.0?
(Assignee)

Comment 5

7 years ago
http://hg.mozilla.org/mozilla-central/rev/8d7913fe2ecf
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Blocks: 1237616
You need to log in before you can comment on or make changes to this bug.