91562 2012-07-17 17:19:31 -0700 [chromium] On Windows the compositor incorrectly flips videos 2012-07-18 15:20:29 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Windows 7 RESOLVED FIXED P2 Normal --- 1 ananta ananta cc-bugs enne eric.carlson feature-media-reviews fischman jamesr webkit.review.bot oldest_to_newest 671780 0 ananta 2012-07-17 17:19:31 -0700 This can be reproduced in Chromium with the command line switch --enable-accelerated-video-decode which enables accelerated video decoding. With this flag on, if you navigate to http://w3schools.com/html5/tryit.asp?filename=tryhtml5_video_all you can see that the video is flipped. The flipped flag needs to be set to false in the CCVideoLayerImpl::appendQuads function in the CCVideoLayerImpl.cpp file. Will upload a patch in a bit. 671825 1 152893 ananta 2012-07-17 18:03:21 -0700 Created attachment 152893 Initial patch 671829 2 enne 2012-07-17 18:06:04 -0700 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? 671836 3 ananta 2012-07-17 18:09:53 -0700 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. 671846 4 ananta 2012-07-17 18:16:50 -0700 The elif defined(OS_WINDOWS) in that patch was wrong. For webkit we need to use OS(WINDOWS) 671849 5 enne 2012-07-17 18:19:31 -0700 (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. 671859 6 ananta 2012-07-17 18:37:26 -0700 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. 671871 7 ananta 2012-07-17 18:58:02 -0700 Based on a discussion with Ami Fischman logged a webkit bug here https://bugs.webkit.org/show_bug.cgi?id=91569 671893 8 152893 enne 2012-07-17 19:34:55 -0700 Comment on attachment 152893 Initial patch R=me. Thanks for filing the bug about tests. 672831 9 152893 webkit.review.bot 2012-07-18 15:20:16 -0700 Comment on attachment 152893 Initial patch Clearing flags on attachment: 152893 Committed r123029: <http://trac.webkit.org/changeset/123029> 672832 10 webkit.review.bot 2012-07-18 15:20:29 -0700 All reviewed patches have been landed. Closing bug. 152893 2012-07-17 18:03:21 -0700 2012-07-18 15:20:16 -0700 Initial patch mypatch.txt text/plain 1640 ananta SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyMjkwNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDEyLTA3LTE3ICBBbmFudGFu YXJheWFuYW4gRyBJeWVuZ2FyICA8YW5hbnRhQGNocm9taXVtLm9yZz4KKworICAgICAgICBbY2hy b21pdW1dIEVuc3VyZSB0aGF0IHRoZSBjb21wb3NpdG9yIGNvZGUgd2hpY2ggaXMgYXdhcmUgb2Yg ZmxpcHBlZCBzdGF0dXMgb2YgdmlkZW8tdGV4dHVyZXMKKyAgICAgICAgcGVyIHBsYXRmb3JtIHNl dHMgdGhlIGZsaXBwZWQgYml0IHRvIGZhbHNlIG9uIFdpbmRvd3MuCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05MTU2MgorICAgICAgICAKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLiAoSFcg dmlkZW8gZGVjb2RlIGlzIHN0aWxsIG9ubHkgYmVpbmcgdGVzdGVkIG1hbnVhbGx5IGZvciBvcmll bnRhdGlvbikKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDVmlk ZW9MYXllckltcGwuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q0NWaWRlb0xheWVySW1wbDo6YXBw ZW5kUXVhZHMpOgorCiAyMDEyLTA3LTE3ICBBbGVjIEZsZXR0ICA8YWxlY2ZsZXR0QGNocm9taXVt Lm9yZz4KIAogICAgICAgICBJbmRleGVkREI6IGNyZWF0ZUluZGV4IHNob3VsZCB0aHJvdyBJTlZB TElEX0FDQ0VTU19FUlIgaW5zdGVhZCBvZiBOT1RfU1VQUE9SVEVEX0VSUgpJbmRleDogU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NWaWRlb0xheWVySW1wbC5j cHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21p dW0vY2MvQ0NWaWRlb0xheWVySW1wbC5jcHAJKHJldmlzaW9uIDEyMjkwNSkKKysrIFNvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDVmlkZW9MYXllckltcGwuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC0yMTQsNyArMjE0LDcgQEAgdm9pZCBDQ1ZpZGVvTGF5ZXJJbXBs OjphcHBlbmRRdWFkcyhDQ1F1YQogI2lmIGRlZmluZWQoT1NfQ0hST01FT1MpICYmIGRlZmluZWQo X19BUk1FTF9fKQogICAgICAgICBib29sIGZsaXBwZWQgPSB0cnVlOyAvLyBVbmRlciB0aGUgY292 ZXJzLCBpbXBsZW1lbnRlZCBieSBPcGVuTUFYIElMLgogI2VsaWYgT1MoV0lORE9XUykKLSAgICAg ICAgYm9vbCBmbGlwcGVkID0gdHJ1ZTsgLy8gVW5kZXIgdGhlIGNvdmVycywgaW1wbGVtZW50ZWQg YnkgRFhWQS4KKyAgICAgICAgYm9vbCBmbGlwcGVkID0gZmFsc2U7IC8vIFVuZGVyIHRoZSBjb3Zl cnMsIGltcGxlbWVudGVkIGJ5IERYVkEuCiAjZWxzZQogICAgICAgICBib29sIGZsaXBwZWQgPSBm YWxzZTsgLy8gTGliVkEgKGNyb3MvaW50ZWwpLCBNYWNPUy4KICNlbmRpZgo=