Bug 82281 - Make media/video-colorspace-yuv*.html tests useful for all ports.
Summary: Make media/video-colorspace-yuv*.html tests useful for all ports.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shadi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-26 18:23 PDT by Shadi
Modified: 2012-04-27 16:18 PDT (History)
6 users (show)

See Also:


Attachments
Patch (271.28 KB, patch)
2012-03-26 19:33 PDT, Shadi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (9.43 MB, application/zip)
2012-03-26 22:37 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from ec2-cq-02 (5.75 MB, application/zip)
2012-04-20 12:17 PDT, WebKit Review Bot
no flags Details
Patch (281.45 KB, patch)
2012-04-24 15:10 PDT, Shadi
eric.carlson: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.02 MB, application/zip)
2012-04-25 03:40 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shadi 2012-03-26 18:23:10 PDT
The current yuv422p and yuv420p tests media/video-colorspace-yuv420.html and media/video-colorspace-yuv422.html hard code the video to use.

We should make the tests work on different ports by querying the UA to find out what formats is supported and using an appropriate file.

The original tests were part of https://bugs.webkit.org/show_bug.cgi?id=69113.
Comment 1 Shadi 2012-03-26 19:33:37 PDT
Created attachment 133962 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-26 22:37:10 PDT
Comment on attachment 133962 [details]
Patch

Attachment 133962 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12141347

New failing tests:
media/video-colorspace-yuv422.html
media/video-colorspace-yuv420.html
Comment 3 WebKit Review Bot 2012-03-26 22:37:17 PDT
Created attachment 133978 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 4 Eric Carlson 2012-04-19 10:48:32 PDT
Does this need to be updated, or is the test failure unrelated?
Comment 5 Eric Carlson 2012-04-19 11:10:54 PDT
Comment on attachment 133962 [details]
Patch

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

> LayoutTests/media/video-colorspace-yuv420.html:10
> +        setSrcByTagName('video', findMediaFile('video', 'content/test_yuv420'));

findMediaFile uses canPlayType to ask the UA if it supports  "video/mp4", "video/mpeg", "video/quicktime", "video/ogg", in that order, and it concatenates the extension of the first type that is supported. to the second parameter passed to the it. We only have one media file with "test_yuv420" in its name, test_yuv420.ogv, so "findMediaFile('video', 'content/test_yuv420')" can only succeed in a browser that supports video/ogg but none of the other MIME types.

> LayoutTests/media/video-colorspace-yuv422.html:10
> +        setSrcByTagName('video', findMediaFile('video', 'content/test_yuv422'));

Ditto.
Comment 6 Shadi 2012-04-19 11:17:38 PDT
(In reply to comment #5)
> (From update of attachment 133962 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133962&action=review
> 
> > LayoutTests/media/video-colorspace-yuv420.html:10
> > +        setSrcByTagName('video', findMediaFile('video', 'content/test_yuv420'));
> 
> findMediaFile uses canPlayType to ask the UA if it supports  "video/mp4", "video/mpeg", "video/quicktime", "video/ogg", in that order, and it concatenates the extension of the first type that is supported. to the second parameter passed to the it. We only have one media file with "test_yuv420" in its name, test_yuv420.ogv, so "findMediaFile('video', 'content/test_yuv420')" can only succeed in a browser that supports video/ogg but none of the other MIME types.
> 
> > LayoutTests/media/video-colorspace-yuv422.html:10
> > +        setSrcByTagName('video', findMediaFile('video', 'content/test_yuv422'));
> 
> Ditto.

That's right, and that's what this CL is for. The CL includes 2 new mp4 files LayoutTests/media/content/test_yuv420.mp4 and LayoutTests/media/content/test_yuv422.mp4 for that purpose.

As for the failures, I do not think they are related. However, new baselines would be required once submitted.
Comment 7 Eric Carlson 2012-04-19 12:09:23 PDT
(In reply to comment #6)
> 
> That's right, and that's what this CL is for. The CL includes 2 new mp4 files LayoutTests/media/content/test_yuv420.mp4 and LayoutTests/media/content/test_yuv422.mp4 for that purpose.
> 
Silly me, I overlooked the new files! Sorry for the noise :-)

> As for the failures, I do not think they are related. However, new baselines would be required once submitted.

Do you want this patch to be reviewed then?
Comment 8 Shadi 2012-04-19 12:13:13 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > 
> > That's right, and that's what this CL is for. The CL includes 2 new mp4 files LayoutTests/media/content/test_yuv420.mp4 and LayoutTests/media/content/test_yuv422.mp4 for that purpose.
> > 
> Silly me, I overlooked the new files! Sorry for the noise :-)
> 
> > As for the failures, I do not think they are related. However, new baselines would be required once submitted.
> 
> Do you want this patch to be reviewed then?

Yes, can you please review? :)
Comment 9 WebKit Review Bot 2012-04-20 11:19:24 PDT
Comment on attachment 133962 [details]
Patch

Rejecting attachment 133962 [details] from commit-queue.

shadi@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 10 WebKit Review Bot 2012-04-20 12:17:02 PDT
Comment on attachment 133962 [details]
Patch

Rejecting attachment 133962 [details] from commit-queue.

New failing tests:
media/video-colorspace-yuv422.html
media/video-colorspace-yuv420.html
Full output: http://queues.webkit.org/results/12474276
Comment 11 WebKit Review Bot 2012-04-20 12:17:12 PDT
Created attachment 138142 [details]
Archive of layout-test-results from ec2-cq-02

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 12 Shadi 2012-04-24 15:10:33 PDT
Created attachment 138660 [details]
Patch
Comment 13 Shadi 2012-04-24 15:14:57 PDT
Eric, 

I updated the patch to use new ogv files due to bug crbug.com/124777.

Please review and commit when you can.
Comment 14 WebKit Review Bot 2012-04-25 03:40:20 PDT
Comment on attachment 138660 [details]
Patch

Attachment 138660 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12520505

New failing tests:
media/video-colorspace-yuv420.html
Comment 15 WebKit Review Bot 2012-04-25 03:40:26 PDT
Created attachment 138775 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 16 Shadi 2012-04-25 13:25:26 PDT
As far as I can tell, the failure is due to need to rebaseline, and other failures are not related. 

Can we commit the change and then submit rebaselines for all platforms?
Comment 17 Eric Carlson 2012-04-26 11:03:59 PDT
Comment on attachment 138660 [details]
Patch

Marking r+ but not cq+ because this is likely to break the build when it lands so you need to be on hand to deal with getting new results committed when it lands. I suggest that you get someone in your office to committed.
Comment 18 Andrew Scherkus 2012-04-27 16:18:29 PDT
Committed as http://trac.webkit.org/changeset/115510