Summary: | [chromium] On Windows the compositor incorrectly flips videos | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ananta Iyengar <ananta> | ||||
Component: | WebCore Misc. | Assignee: | Ananta Iyengar <ananta> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cc-bugs, enne, eric.carlson, feature-media-reviews, fischman, jamesr, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | Windows 7 | ||||||
Attachments: |
|
Description
Ananta Iyengar
2012-07-17 17:19:31 PDT
Created attachment 152893 [details]
Initial patch
What changed from https://bugs.webkit.org/show_bug.cgi?id=89189#c10? Was that just incorrect? Also, that's unfortunate that there aren't any tests for hardware decode. Why is that? No tests as yet as it is still under development and is manually enabled in Chrome via command line switches. Tests will come in soon hopefully. The elif defined(OS_WINDOWS) in that patch was wrong. For webkit we need to use OS(WINDOWS) (In reply to comment #4) > The elif defined(OS_WINDOWS) in that patch was wrong. For webkit we need to use OS(WINDOWS) How did this work when you tested it locally, then? (In reply to comment #3) > No tests as yet as it is still under development and is manually enabled in Chrome via command line switches. Tests will come in soon hopefully. Sorry to be reluctant, but I'm frustrated that this never worked. If we had had tests in the first place, there'd be no need for this patch. It seems like it shouldn't be that hard to add at least one layout test that draws a video with this patch. You could either explicitly enable the hardware video path via a new function in Chromium's LayoutTestController.cpp (look for how we handle loseCompositorContext as an example). Alternatively, you could create a (hypothetical) LayoutTests/platform/chromium/compositing/hardwarevideo/ directory and then modify TestShell::runFileTest to set these flags on anything in that directory. Then I think you could just take a basic video layout test and run it through the hardware path. On Windows the VDA code uses the Microsoft media foundation API which brings with it COM and a dependency on Windows 7. I agree that we need tests for this. I think a better place for this would be in content shell or something similar in Chromium where we can exercise this end to end. We have a basic test for the VDA stuff in the form of video_decode_accelerator_unittest.cc under src\content\common\gpu\media. It would be great if we could land this patch. In any case this feature will not be enabled with comprehensive tests in place. Based on a discussion with Ami Fischman logged a webkit bug here https://bugs.webkit.org/show_bug.cgi?id=91569 Comment on attachment 152893 [details]
Initial patch
R=me. Thanks for filing the bug about tests.
Comment on attachment 152893 [details] Initial patch Clearing flags on attachment: 152893 Committed r123029: <http://trac.webkit.org/changeset/123029> All reviewed patches have been landed. Closing bug. |