[webkit-reviews] review denied: [Bug 52697] Frame accurate seeking isn't always accurate : [Attachment 81034] Patch

Thu Feb 3 08:35:28 PST 2011

Eric Carlson <eric.carlson at apple.com> has denied Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 52697: Frame accurate seeking isn't always accurate
https://bugs.webkit.org/show_bug.cgi?id=52697

Attachment 81034: Patch
https://bugs.webkit.org/attachment.cgi?id=81034&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=81034&action=review

> LayoutTests/media/video-frame-accurate-seek.html:9
> +<script src="media-file.js"></script>
> +<script src="video-paint-test.js"></script>
> +<body onload="setSrcByTagName('video', findMediaFile('video',
'content/test-25fps')); init()">
> +<p>Test that setting currentTime is frame-accurate. The three videos below
should be showing frames 12, 13, and 14.</p>
> +<video oncanplaythrough='event.target.currentTime=(12/25)'></video>
> +<video oncanplaythrough='event.target.currentTime=(13/25)'></video>
> +<video oncanplaythrough='event.target.currentTime=(14.6/25)'></video>
> +</div>
> +</body>

As long as you are going to update the patch, can you clean up this new test by
adding html and head elements? Bonus points for formatting it to make it easier
to read.

> LayoutTests/platform/gtk/Skipped:5618
> +
> +# This media test fails; possibly because the AAC codec is not installed on
the test bots:
> +# https://bugs.webkit.org/show_bug.cgi?id=53125
> +media/audio-mpeg4-supported.html

As you note, this doesn't belong in this patch.