470983 - Video controls should ...buffering... indicate when ...buffering... playback is stalled

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect)

Description

[Justin Dolske [:Dolske]]

The video controls should provide some kind of UI feedback when the video playback halts because it's run out of available data to play (ie, it's expecting more, but the network/server is slow).

RealPlayer's infamous "buffering" message is one example of this. YouTube just pauses with a centered throbber (which is usually hard to see against a busy background). The BBC News' media player, OTOH, has a pleasing effect for this state: the video fades to a darker color over 1-2 seconds, and a centered throbber is shown. A fade is great and should be simple to implement, the throbber might be trickier. [If it's centered in the video frame, it probably needs to scale with video size. Perhaps we could place a fixed-size throbber on the controls or in the corner, but I'm not sure how that would look. Or maybe we can find a fixed size that doesn't look enormous on small videos, yet doesn't get lost inside a giant video.]

The HTML5 spec has a "stalled" event that should trigger this. Looks like this isn't currently supported, but will be added in the patch to bug 464376.

Comment 1

[Justin Dolske [:Dolske]]

Talked with Boriss about this for a bit. Let's try fixed-size throbber + fading to see if it works well enough. If that's insufficient, we can can look at scaling/location alternatives.

[An in-between idea that occurred to me was to generate a large throbber, say 128x128, and downscale that with CSS as needed.]

Comment 2

[Justin Dolske [:Dolske]]

Summary of some more discussion, in the context of loading an autoplay video. The basic desired steps are this:

0) Video element created
1) Load begins, <video> content area is empty (controls not addressed here)
2) Fade in  a throbber + dim video (w/semitransparent black overlay) when the
   first of these happens:
     - |loadeddata| fires (1st frame is visible)
     - about 2 seconds elapse
3) ...waiting for enough data to buffer before starting autoplay...
4) If we stop receiving data (|stalled| event fires), additionally add some text (eg "Stalled...") below the throbber. Remove text when we start getting data again.
5) When playback beings (|playing| fires), remove the throbber + undim video.

Key points of note are:

* Don't show a throbber/dimming right away, since usually you'll get the 1st frame quickly. Having a single "1st frame + throbber" transition is cleaner than blinking both in at slightly different times. But don't wait too long, or the user won't know what's happening with the video.

* The addition of the text (@ #4) is to help mark it as a different state. We considered just rolling these states together (since for the user it's still "video isn't playing"), but thought the distinction was useful... One state means playback should start soon (at least ideally) so stay tuned, while the other state means you can divert your attention elsewhere because things are not going so well.

We should also do the same thing (wait ~2s, then throbber+dim) when seeking, especially since it's so slow with Ogg.

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Patch v.1 (WIP)

Most of this is refactoring fadeControls() and part of onMouseInOut() so that the fading login is reusuable. This gets reused to fade in the "busy" UI, and will also be used later when we add a time indicator that fades in over the scrubber thumb.

Right now this is only hooked up to seeking, so that long seeks trigger a throbber. The delay and throbber are just placeholders at the moment.

I next need to look at exactly what events to watch for the other times we want to show the throbber UI, and if the video backend is correcting supporting those yet.

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Patch v.2 (WIP)

This is a mostly-complete implementation, but only hooked up for slow seeking. Waiting on a new throbber design from Boriss, and we need to figure out if we can get away with a single throbber size or if we'll need small/medium/large versions for various video sizes.

Eventually this UI needs to be hooked up to the other "busy" states (see comment 0 and comment 2), but since those seem to need some more backend work, it might make sense to land this as-is (ie, slow-seeking feedback only), and hook up the other bits later.

Comment 5

[Justin Dolske [:Dolske]]

Attachment: events flowchart

New patch coming up. Here's a crude flowchart with the relevant events, which kind of gives a high-level view of what the code is trying to do.

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Patch v.3

Some of these events are tricky to catch in normal usage, so it's not really possible to write automated tests for this (yet?). I've written a PHP script to control the data transfer, so that it's easy to simulate a slow or stalled network... See: http://dolske.net/mozilla/tests/video/tests.html

A fair chunk of the patch is refactoring the code which drives the control fading, so that both the controls and throbber can reuse the same code. Basically, the state for each is moved into an object (controlFader, throbberFader), and the fading code is told which one it should use. This unfortunately churns some code for the diff, but it's largely just renaming variables, and not a change in behavior.

Comment 7

[Neil Deakin]

Comment on attachment 363813 [details] [diff] [review]
Patch v.3

>+            <vbox class="throbberOverlay">
>+                <box class="throbber" flex="1"/>
>+                <spacer class="controlsSpacer"/>
>+            </vbox>

You could just use a single element here and just use a 28 pixel bottom margin.

Also, <image> could be used which should layout to the size of the actual image rather than hardcoding a size.


>+                        case "canplaythrough":
>+                            this.startFadeOut(this.throbberFader);
>+                            break;

You could combine this case with the 'playing' event.

Comment 8

[Jennifer Morrow [:Boriss] (UX)]

Comment on attachment 363813 [details] [diff] [review]
Patch v.3

Marking UI+ on throbber

Comment 9

[Justin Dolske [:Dolske]]

Attachment: Patch v.4

Updated with nits. Also, Boriss wanted a slightly smaller throbber than was in the last patch.

Comment 10

[Justin Dolske [:Dolske]]

Oops, forgot to reply to this bit:

(In reply to comment #7)
> Also, <image> could be used which should layout to the size of the actual image
> rather than hardcoding a size.

We can't use <image>s in the controls, because they cause |load| events to be fired when the image loads, and could cause a load-listener on the video element to become confused.

Comment 11

[Justin Dolske [:Dolske]]

Pushed http://hg.mozilla.org/mozilla-central/rev/c0c058f2f503

Comment 12

[Christopher Blizzard (:blizzard)]

Does this need 1.9.1 landing?

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 364197 [details] [diff] [review]
Patch v.4

a191=beltzner

Comment 14

[Justin Dolske [:Dolske]]

Pushed to 191: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/01ce368c98e6

Comment 15

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre Ubiquity/0.1.5 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre