Bug 69113 - Video format yuv422p doesn't show properly.
Summary: Video format yuv422p doesn't show properly.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Shadi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-29 17:21 PDT by Shadi
Modified: 2012-04-30 14:23 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.90 KB, patch)
2011-09-29 17:24 PDT, Shadi
no flags Details | Formatted Diff | Diff
Patch (346.21 KB, patch)
2011-09-29 17:44 PDT, Shadi
no flags Details | Formatted Diff | Diff
Patch (2.56 KB, patch)
2012-03-23 18:08 PDT, Shadi
no flags Details | Formatted Diff | Diff
Patch (173.37 KB, patch)
2012-03-23 18:12 PDT, Shadi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (9.99 MB, application/zip)
2012-03-23 20:25 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 2011-09-29 17:21:29 PDT
Playing videos with yuv422p format (YV16) doesn't render colors properly.
Comment 1 Shadi 2011-09-29 17:24:27 PDT
Created attachment 109224 [details]
Patch
Comment 2 Shadi 2011-09-29 17:44:17 PDT
Created attachment 109227 [details]
Patch
Comment 3 Alexey Proskuryakov 2011-09-29 21:54:28 PDT
Is it intentional that this patch only contains a regression test, not a fix?
Comment 4 Shadi 2011-09-30 16:36:53 PDT
(In reply to comment #3)
> Is it intentional that this patch only contains a regression test, not a fix?

The fix has been submitted to chrome, and will be committed today. 
Issue: http://code.google.com/p/chromium/issues/detail?id=95642, 
patch: http://codereview.chromium.org/8052002/
Comment 5 Eric Seidel (no email) 2011-12-19 14:04:46 PST
Comment on attachment 109227 [details]
Patch

Other platforms may want this test, so it seems reasonable to have it in WebKit.  Your ChangeLog should have explained why there was no associated code change.
Comment 6 WebKit Review Bot 2011-12-21 17:29:36 PST
Comment on attachment 109227 [details]
Patch

Clearing flags on attachment: 109227

Committed r103477: <http://trac.webkit.org/changeset/103477>
Comment 7 WebKit Review Bot 2011-12-21 17:29:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Ryosuke Niwa 2011-12-22 19:56:00 PST
Chromium rebaselines done in http://trac.webkit.org/changeset/103553 (CPU) and http://trac.webkit.org/changeset/103603 (GPU).
Comment 9 Eric Carlson 2012-03-21 17:14:38 PDT
(In reply to comment #5)
> (From update of attachment 109227 [details])
> Other platforms may want this test, so it seems reasonable to have it in WebKit.  Your ChangeLog should have explained why there was no associated code change.

As written, this test is worthless to some ports because it is hard coded to use an Ogg video file instead of querying the UA to find out what formats is supports and using an appropriate file - the technique used by all the other media tests.
Comment 10 Shadi 2012-03-21 17:46:15 PDT
How can we guarantee to get a yuv422 video format by through a query? Other media files care about the codec of a file, however in this case both yuv422 and yuv420 are for the same codec.

findMediaFile() returns the first file format that is supported which I don't see it helpful.
Comment 11 Andrew Scherkus 2012-03-22 11:11:19 PDT
I agree with Eric. This test can be useful for all ports in which case there needs to be a copy of test_yuv420.ogv that works for webkit ports that don't support ogg (i.e., check in a test_yuv420.mp4).

That way findMediaFile("video", "content/test_yuv420") will choose the appropriate ogv/mp4 extension based on UA support and this test can be enabled for all ports.
Comment 12 Shadi 2012-03-23 18:08:28 PDT
Reopening to attach new patch.
Comment 13 Shadi 2012-03-23 18:08:30 PDT
Created attachment 133606 [details]
Patch
Comment 14 Shadi 2012-03-23 18:12:03 PDT
Created attachment 133608 [details]
Patch
Comment 15 Shadi 2012-03-23 18:14:02 PDT
I am not very familiar with the process in WebKit. Should I open a new bug to update those tests?

Should these tests be committed first and after running on the bots we open a new bug to create a baseline for all ports?
Comment 16 WebKit Review Bot 2012-03-23 20:25:15 PDT
Comment on attachment 133608 [details]
Patch

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

New failing tests:
media/video-colorspace-yuv422.html
media/video-colorspace-yuv420.html
Comment 17 WebKit Review Bot 2012-03-23 20:25:22 PDT
Created attachment 133616 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 18 Alexey Proskuryakov 2012-03-23 22:29:20 PDT
Yes, it is much preferred to have one bug per patch, and certainly not to revive bugs that have been resolved for a long time.
Comment 19 Eric Carlson 2012-04-20 10:40:27 PDT
Is this still necessary now that https://bugs.webkit.org/show_bug.cgi?id=82281 is in?