Closed Bug 610570 Opened 9 years ago Closed 9 years ago

Keyframe skipping activates when low on downloaded data

Categories

(Core :: Audio/Video, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(1 file)

We don't display a few video frames before we stop playback to start buffering, and we don't show any video frames after we've resumed playback after buffering.

The keyframe skipping logic is supposed to help us when the video decode is too slow to be able to keep up with the rate of consumption. So when we run low on decoded data, the keyframe skipping logic kicks in, and we'll start skipping to the next keyframe.

But if the download is not fast enough to beat the decode, we'll run low on decoded data also. The keyframe skipping logic then kicks in, and we start skipping to the next keyframe. We then go into buffering mode, but we'll still be skipping to the next keyframe when playback resumes, and so we'll discard all frames up to the next keyframe after playback resumes. There's no need to discard these frames, we can play them.
Assignee: nobody → chris
Other issues:
1. Buffering can start when we're low on decoded data due to slow decode times. i.e. if the decode takes so long that while the decoder thread is blocked decoding, we've played all decoded data, we go into buffering state (because we're now low on decoded data), even though we've got lots of downloaded undecoded data.
2. The keyframe skipping logic isn't run on video only media. This can mean that we may never get to render any video frames when the decode is slow, if the decode gets falls too far behind the current playback position that all video frames are late.
3. The audio decode tries to maintain about 1.5s more decoded audio than video. But if we block on I/O in an audio decode, we'll delay the decode of video frames which are closer to the current playback position (needed sooner), possibly to the extent that they're late, so we end up dropping them anyway. This can cause us to drop a noticable number of frames just before we go into buffering, and causes us to queue up frames which are behind the current playback position while we buffer, so that when we come out of buffering we drop all frames in the queue, and then decide we need to keyframe skip since we've got no decoded video frames any more.
Component: Video/Audio → Tracking
Assignee: chris → nobody
Component: Tracking → Video/Audio
Assignee: nobody → chris
Attached patch Patch v1Splinter Review
* Don't skip to keyframe if we've only got an estimated 100ms worth of undecoded data left. This prevents us skipping to the next keyframe when we run out of decoded data due to their not being any data to decode.
* Dynamically increase our "low audio threshold". When we have less than the "low audio threshold" worth of undecoded audio, we'll skip to the next keyframe. We now time how long we spend in the video decode, and if it takes a long time, we'll increase our audio threshold. This means we'll start skipping to the next keyframe sooner when we're running on hardware which struggles to keep the audio decode ahead of the video decode. This is to ensure that we get uninterrupted audio playback (by skipping to the next keyframe more often) when running on underpowered hardware. Without this, I don't get uninterrupted audio when playing 1080p videos on my netbook.
* Remove the "are we low on decoded data" conditions in the "should we buffer" logic. We now start buffering if we've only got an estimated 100ms worth of undecoded data left. This is to prevent us from going into buffering mode when we're running on hardware which can't keep up with the decode even when there's plenty of data buffered to play(!!). If the estimate is inaccurate, we can end up refusing to play a section of the progress bar which the progress bar indicates we should be able to play, but there's not much we can do about that.
* Change keyframe skipping logic so that we skip to next keyframe on video-only streams.

I've tested these changes extensively on a rate controlled web server locally on my netbook, which keyframe skips a lot, and the buffering and keyframe skipping is much more reliable now. This fixes the issues I identified in my comments above.
Attachment #490995 - Flags: review?(roc)
Attachment #490995 - Flags: feedback?(kinetik)
This requires the patch in bug 578536 in order to have reliable playback statistics for WebM videos.

This bug should be a blocker, we should get the keyframe skipping and buffering working properly.
blocking2.0: --- → ?
Depends on: 578536
+#define LIVE_BUFFER_MARGIN 100000

why not static const int?

> If the estimate is inaccurate, we can end
> up refusing to play a section of the progress bar which the progress bar
> indicates we should be able to play, but there's not much we can do about that.

Why not? This seems bad.
(In reply to comment #4)
> +#define LIVE_BUFFER_MARGIN 100000
> 
> why not static const int?

I'll change it.

> 
> > If the estimate is inaccurate, we can end
> > up refusing to play a section of the progress bar which the progress bar
> > indicates we should be able to play, but there's not much we can do about that.
> 
> Why not? This seems bad.

Because if we've got less than estimated 100ms of data left to play, we'll switch to buffering mode when we try to play that segment of video. Usually 100ms of estimated data represents a pretty small segment of the progress bar, so the playback head will stop close to the extremity of buffered data.
Right, but why can't we fix that?
Attachment #490995 - Flags: approval2.0?
http://hg.mozilla.org/mozilla-central/rev/7db5a5d297c6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #490995 - Flags: feedback?(kinetik)
You need to log in before you can comment on or make changes to this bug.