Bug 124262 - Add support for HTMLMediaElement.fastSeek()
Summary: Add support for HTMLMediaElement.fastSeek()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords:
: 111983 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-11-12 22:51 PST by Jer Noble
Modified: 2014-02-07 17:22 PST (History)
9 users (show)

See Also:


Attachments
Patch (17.78 KB, patch)
2013-11-12 22:59 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (466.94 KB, application/zip)
2013-11-13 00:58 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (571.59 KB, application/zip)
2013-11-13 01:56 PST, Build Bot
no flags Details
Patch (17.79 KB, patch)
2013-11-13 09:08 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (60.56 KB, patch)
2013-11-15 14:47 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2013-11-12 22:51:07 PST
Add support for HTMLMediaElement.fastSeek()
Comment 1 Jer Noble 2013-11-12 22:59:56 PST
Created attachment 216769 [details]
Patch
Comment 2 Build Bot 2013-11-13 00:58:38 PST
Comment on attachment 216769 [details]
Patch

Attachment 216769 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22639943

New failing tests:
media/video-fast-seek.html
fullscreen/video-controls-timeline.html
media/audio-delete-while-slider-thumb-clicked.html
media/controls-drag-timebar.html
Comment 3 Build Bot 2013-11-13 00:58:41 PST
Created attachment 216779 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Build Bot 2013-11-13 01:56:11 PST
Comment on attachment 216769 [details]
Patch

Attachment 216769 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22888965

New failing tests:
fullscreen/video-controls-timeline.html
media/audio-delete-while-slider-thumb-clicked.html
media/controls-drag-timebar.html
Comment 5 Build Bot 2013-11-13 01:56:14 PST
Created attachment 216784 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Jer Noble 2013-11-13 09:08:34 PST
Created attachment 216808 [details]
Patch
Comment 7 Eric Carlson 2013-11-13 09:45:22 PST
Comment on attachment 216808 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216808&action=review

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:691
> +        // Do a precise seek when we lift the mouse:
> +        this.video.currentTime = this.controls.timeline.value;

Nice!

> Source/WebCore/html/HTMLMediaElement.idl:78
> +[RaisesException] void fastSeek(double time);

It doesn't look like this (or currentTime) is supposed to throw.

> LayoutTests/media/video-fast-seek.html:4
> +<p>Test that fastSeek() commands work correctly
> + </p>
> +<script src=media-file.js></script>

Odd indentation here.

It would be nice to stick the <script> in a <head>, include a <!DOCTYPE html>, etc.

> LayoutTests/media/video-fast-seek.html:9
> +    // The test.mp4 file has sync samples at the following presentation time stamps:
> +    // 0.0000, 0.7968, 1.5936, 2.3904, 3.1872, 3.9840, 4.7808, 5.5776

This mentions test.mp4, but uses findMediaFile() so other ports could end up with a different file. It may be worth renaming this test and hard coding the test.mp4.
Comment 8 Jer Noble 2013-11-13 10:24:45 PST
(In reply to comment #7)
> (From update of attachment 216808 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=216808&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.idl:78
> > +[RaisesException] void fastSeek(double time);
> 
> It doesn't look like this (or currentTime) is supposed to throw.

I'll fix this in a future patch.

> > LayoutTests/media/video-fast-seek.html:4
> > +<p>Test that fastSeek() commands work correctly
> > + </p>
> > +<script src=media-file.js></script>
> 
> Odd indentation here.
> 
> It would be nice to stick the <script> in a <head>, include a <!DOCTYPE html>, etc.

Fixed and added.

> > LayoutTests/media/video-fast-seek.html:9
> > +    // The test.mp4 file has sync samples at the following presentation time stamps:
> > +    // 0.0000, 0.7968, 1.5936, 2.3904, 3.1872, 3.9840, 4.7808, 5.5776
> 
> This mentions test.mp4, but uses findMediaFile() so other ports could end up with a different file. It may be worth renaming this test and hard coding the test.mp4.

Ok.
Comment 9 Jer Noble 2013-11-13 10:27:54 PST
Committed r159208: <http://trac.webkit.org/changeset/159208>
Comment 10 Jer Noble 2013-11-15 14:47:33 PST
Reopening to attach new patch.
Comment 11 Jer Noble 2013-11-15 14:47:36 PST
Created attachment 217087 [details]
Patch
Comment 12 Jer Noble 2013-11-15 14:48:32 PST
Comment on attachment 217087 [details]
Patch

My mistake.
Comment 13 Eric Carlson 2014-02-07 17:22:08 PST
*** Bug 111983 has been marked as a duplicate of this bug. ***