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.
Created attachment 133962 [details] Patch
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
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
Does this need to be updated, or is the test failure unrelated?
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.
(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.
(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?
(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 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 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
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
Created attachment 138660 [details] Patch
Eric, I updated the patch to use new ogv files due to bug crbug.com/124777. Please review and commit when you can.
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
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
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 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.
Committed as http://trac.webkit.org/changeset/115510