Bug 82281 – Make media/video-colorspace-yuv*.html tests useful for all ports.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Shadi]

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.

[Shadi]

Created attachment 133962 [details]
Patch

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Eric Carlson]

Does this need to be updated, or is the test failure unrelated?

[Eric Carlson]

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.

[Shadi]

(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.

[Eric Carlson]

(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?

[Shadi]

(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? :)

[WebKit Review Bot]

Comment on attachment 133962 [details]
Patch

Rejecting attachment 133962 [details] from commit-queue.

[e-mail] 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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Shadi]

Created attachment 138660 [details]
Patch

[Shadi]

Eric, 

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

Please review and commit when you can.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Shadi]

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?

[Eric Carlson]

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.

[Andrew Scherkus]

Committed as http://trac.webkit.org/changeset/115510