Bug 91562 – [chromium] On Windows the compositor incorrectly flips videos

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Ananta Iyengar]

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.

[Ananta Iyengar]

Created attachment 152893 [details]
Initial patch

[Adrienne Walker]

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?

[Ananta Iyengar]

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.

[Ananta Iyengar]

The elif defined(OS_WINDOWS) in that patch was wrong. For webkit we need to use OS(WINDOWS)

[Adrienne Walker]

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

[Ananta Iyengar]

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.

[Ananta Iyengar]

Based on a discussion with Ami Fischman logged a webkit bug here https://bugs.webkit.org/show_bug.cgi?id=91569

[Adrienne Walker]

Comment on attachment 152893 [details]
Initial patch

R=me.  Thanks for filing the bug about tests.

[WebKit Review Bot]

Comment on attachment 152893 [details]
Initial patch

Clearing flags on attachment: 152893

Committed r123029: <http://trac.webkit.org/changeset/123029>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.