soundcloud / areweplayingyet

html5 audio benchmarks
BSD 2-Clause "Simplified" License
311 stars 69 forks source link

Faulty assumption in preload=metadata test #9

Open foolip opened 13 years ago

foolip commented 13 years ago

http://areweplayingyet.org/attribute-preload-metadata assumes that preload="metadata" will be reached in 1 second, which the spec says nothing about. We try to buffer 30 seconds of data, regardless of how long that takes.

tsenart commented 13 years ago

Actually that is not what the code does. The code waits for the loadedmetadata event to be triggered and after that checks if there is still data being loaded, which is not supposed to happen.

tsenart commented 13 years ago

http://www.w3.org/TR/html5/video.html#attr-media-preload-metadata It's not reasonable to load 30 seconds of data with preload="metadata".

foolip commented 13 years ago

The spec says "The preload attribute is intended to provide a hint to the user agent about what the author thinks will lead to the best user experience. The attribute may be ignored altogether, for example based on explicit user preferences or based on the available connectivity." I have personally objected to the "hintiness" of preload, but it does mean that the spec quite definitely allows the behavior that we have chosen. Since metadata is the default preload state, our assessment was that loading 30 s is better than getting only the duration and dimensions. Otherwise, any time that a user starts playing a video it will immediately stall for buffering.

Also note that the loadedmetadata event is related to readyState and the progress events are related to networkState. These two states aren't tightly coupled (at least not in the spec) in a way that one could ever guarantee that the network stops loading immediately when the decoding pipeline reaches a certain state. (One could spec that we should lie about it of course, if scripts came to assume such things.)

zcorpan commented 13 years ago

This issue should be reopened. As foolip said, the spec certainly allows the UA to wait for 30s to be loaded for preload=metadata. A testsuite like this should test for things the spec requires, not things the test author thinks is "reasonable".

yvg commented 13 years ago

As it creates unexpected behavior this should get specified, in the meantime we want to test for "reasonable" real-world applications. The goal of this project is to point out exactly these kind of issues that we can have in practice.

foolip commented 13 years ago

If you want it to be specified in some particular way, e.g. forbidding any progress events after the loadedmetadata event, please send an email to the WHATWG, HTML WG or use the bug submission form at the bottom of http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#attr-media-preload

Please do not mix tests for specified behavior with tests for deliberately unspecified behavior. It is not an Opera bug that we treat preload=metadata the way we do.

tsenart commented 13 years ago

@foolip We're trying to bring harmony to the HTML5 implementations. That's the purpose of this project. Your implementation of the preload="metadata" breaks expectations even if it is compliant. So we're asking you to please consider the broader scope of the problem and be open to discussion. I am going to loop in other browser vendors on this issue for gathering more opinions.

doublec commented 13 years ago

If you think the spec is unreasonable in allowing implementations to treat preload=metadata as a hint it would be good if you could provide this feedback to the WHATWG or HTML WG. Feedback from web developers as well as user agent implementers is important and with a spec change it's much more likely that implementations will conform.

foolip commented 13 years ago

What @doublec said. I'm quite willing to reconsider what we've implemented, but lacking a state between metadata and auto I think what we've done leads to a better experience, since you can start playing a video without immediately pausing for buffering. We could perhaps decrease it from 30 or 15 seconds (or whatever), but I don't think that stopping immediately when reaching HAVE_CURRENT_DATA (not HAVE_METADATA, because the first frame is not available then) is really that useful -- if bandwidth is super-precious you should be using preload=none.

tsenart commented 13 years ago

@foolip isn't that missing state the auto value?

foolip commented 13 years ago

No, people use preload=auto expecting that as much as possible of the resource will load, not just x seconds.

kinetiknz commented 13 years ago

Since preload is specified as a hint only (and may be ignored by the UA), it's not possible to test the behaviour without encoding implementation-specific assumptions in the test, which is not an approach I recommend. Firefox already treats preload differently on mobile vs desktop (by treating preload="auto" as preload="metadata" on mobile).

tsenart commented 13 years ago

I understand that @kinetiknz. How do you think we can bring harmony to this attribute among implementations?