Bug 24588 - Update media element implementation to current spec
: Update media element implementation to current spec
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: All All
: P2 Major
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-13 14:24 PDT by Eric Carlson
Modified: 2009-03-23 08:31 PDT (History)
3 users (show)

See Also:


Attachments
Work-in-progress patch (130.54 KB, patch)
2009-03-13 14:30 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
updated patch (160.10 KB, patch)
2009-03-14 11:43 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
updated patch (161.34 KB, patch)
2009-03-15 21:20 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
updated patch (161.11 KB, patch)
2009-03-15 21:23 PDT, Eric Carlson
adele: review-
Details | Formatted Diff | Diff
updated patch (161.75 KB, patch)
2009-03-19 07:38 PDT, Eric Carlson
adele: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2009-03-13 14:24:47 PDT
The current implementation of <video> and <audio> elements is out of sync with the HTML5 spec. Fix it.
Comment 1 Eric Carlson 2009-03-13 14:30:16 PDT
Created attachment 28600 [details]
Work-in-progress patch

I believe the changes to HTMLMediaElement, MediaPlayer, MediaPlayerPrivateQTKit, and LayoutTests match the 26 February 2009 spec and are complete with the exception of Cue Ranges. Still need to change all of the other media engines.
Comment 2 Eric Carlson 2009-03-14 11:43:55 PDT
Created attachment 28620 [details]
updated patch

This includes changes for all media engines. I have only built and tested on the Mac, but the review might as well happen while I work on getting my Windows machine working.
Comment 3 Eric Carlson 2009-03-15 21:20:31 PDT
Created attachment 28644 [details]
updated patch

Fix a couple of minor typos.
Comment 4 Eric Carlson 2009-03-15 21:23:58 PDT
Created attachment 28645 [details]
updated patch

Oops, attached the wrong version last time.
Comment 5 Adele Peterson 2009-03-17 00:23:46 PDT
Comment on attachment 28645 [details]
updated patch

Looks pretty good.  Minor comments below.  I found a problem with this patch and the load event on Windows.  Eric's going to diagnose tomorrow, so I'll r- this for now, but I expect this will be ready to check in fairly soon once that issue is figured out.

> Index: WebCore/ChangeLog
typo "obsolute"
typo "ticke"

> Index: WebCore/html/HTMLMediaElement.cpp
> ===================================================================

I think it would be cleaner here to remove the commented out code, and stick it in a bug.  Then you can just reference the bug here.

> +    // FIXME: don't schedule timeupdate or progress events unless there are registered listeners
> +/*
> +    if (eventType == eventNames().progressEvent && !document()->hasListenerType(Document::xxxxxx_LISTENER))
> +        return;
> +    if (eventType == eventNames().timeupdateEvent && !document()->hasListenerType(Document::xxxxxx_LISTENER))
> +        return;
> +*/
> +
Comment 6 Eric Carlson 2009-03-19 07:38:45 PDT
Created attachment 28751 [details]
updated patch

Two changes:
 + don't stop timeupdate event timer when reach loaded state. 
 + minor efficiency fix on Windows, don't call setRate(0) when rate is already 0.
Comment 7 Eric Carlson 2009-03-23 08:31:42 PDT
Committed revision 41907.