New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

[HTML5 video]: video playback is upside down

Project Member Reported by djkurtz@chromium.org, Apr 1 2016

Issue description

Chrome Version: 51.0.2695.1
Chrome OS Version: 8136.0.0
Chrome OS Platform: samus & oak

Steps To Reproduce:
(1) watch youtube video, or do a hangout
(2) laugh
(3) turn machine aussie-style and enjoy.

Expected Result:

Right-side up videos.

Actual Result:

A sudden hankering for vegemite, mate.

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)

Always

What is the impact to the user, and is there a workaround? If so, what is
it?

Turns their world view upside down.

Please provide any additional information below. Attach a screen shot or
log if possible.

As of at least 8136.0.0.  This does not happen w/ 8134.0.0.

For original report, and pictures, see http://crosbug.com/p/51937
 
Cc: dcheng@chromium.org enne@chromium.org danakj@chromium.org siev...@chromium.org liber...@chromium.org posciak@chromium.org dalecur...@chromium.org
Owner: tobiasjs@chromium.org
Status: Assigned (was: Available)
A bisect reveals that:
8f2032aeb81c2a6a48ed2ed622859b59d84fbbaa is the first bad commit
commit 8f2032aeb81c2a6a48ed2ed622859b59d84fbbaa
Author: tobiasjs <tobiasjs@chromium.org>
Date:   Wed Mar 30 15:38:13 2016 -0700

    Make Android StreamTexture implement GLStreamTextureImage
    
    Then make CopySubTextureCHROMIUM use the transform matrix provided
    by GLStreamTextureImage, and stop informing the compositor of the
    transform matrix. Video quad drawing applies the matrix automatically
    via the CHROMIUM_stream_texture_matrix extension.
    
    This has has the advantage that we no longer need custom texture copying
    code in StreamTextureAndroid and we also don't need the IPC that updates
    the compositor view of the transform matrix (because it is applied
    transparently when the texture is sampled from). This in turn means that
    the transform matrix can't get out of sync with the texture data.
    
    BUG= 226218 , 371500 , 588837 
    CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
    
    Review URL: https://codereview.chromium.org/1746983002
    
    Cr-Commit-Position: refs/heads/master@{#384122}

:040000 040000 6f4c92a085f9f2c4780e8a879f8d0e4bca708634 eba4ae5ac0143064b22532c789442374004bdd89 M	cc
:040000 040000 a9941dd1c1ba162339c6083bc36d6921a0433f07 1d158bda4075ee19b634caaaeb2313e4dab8cf52 M	content
:040000 040000 1b26a115d8506eaf6329dbf56e9819c02549ce1c df301796dce160943909fb1d39872d15fa52dec2 M	gpu

Cc: sande...@chromium.org jorgelo@chromium.org
 Issue 599839  has been merged into this issue.
Cc: durga.behera@chromium.org kathrelk...@chromium.org pucchakayala@chromium.org songsuk@chromium.org ajha@chromium.org kavvaru@chromium.org
 Issue 599827  has been merged into this issue.
Cc: brajkumar@chromium.org
 Issue 599824  has been merged into this issue.
Cc: rohi...@chromium.org
Labels: M-51 ReleaseBlock-Dev
Cc: hsiangc@chromium.org avkodipelli@chromium.org
Summary: [HTML5 video]: video playback is upside down (was: samus: youtube is.... upside down?!)
issue also see on Amazon, Netflix and local video playback
Cc: -jorgelo@chromium.org
This is also seen on CrOS Cyan device 8138.0.0 / 51.0.2695.1 as well.

Issue 600034 has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6dac6465349eccf7d9cf5850be54cea24631095

commit e6dac6465349eccf7d9cf5850be54cea24631095
Author: djkurtz <djkurtz@chromium.org>
Date: Sun Apr 03 03:26:03 2016

Revert of Make Android StreamTexture implement GLStreamTextureImage (patchset #9 id:180001 of https://codereview.chromium.org/1746983002/ )

Reason for revert:
A git bisect suggests that this patch makes Youtube and Hangouts appear upside-down on Chrome OS.

BUG= 599848 

Original issue's description:
> Make Android StreamTexture implement GLStreamTextureImage
>
> Then make CopySubTextureCHROMIUM use the transform matrix provided
> by GLStreamTextureImage, and stop informing the compositor of the
> transform matrix. Video quad drawing applies the matrix automatically
> via the CHROMIUM_stream_texture_matrix extension.
>
> This has has the advantage that we no longer need custom texture copying
> code in StreamTextureAndroid and we also don't need the IPC that updates
> the compositor view of the transform matrix (because it is applied
> transparently when the texture is sampled from). This in turn means that
> the transform matrix can't get out of sync with the texture data.
>
> BUG= 226218 , 371500 , 588837 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
>
> Committed: https://crrev.com/8f2032aeb81c2a6a48ed2ed622859b59d84fbbaa
> Cr-Commit-Position: refs/heads/master@{#384122}

TBR=dalecurtis@chromium.org,danakj@chromium.org,dcheng@chromium.org,liberato@chromium.org,sievers@chromium.org,enne@chromium.org,tobiasjs@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 226218 , 371500 , 588837 

Review URL: https://codereview.chromium.org/1848923003

Cr-Commit-Position: refs/heads/master@{#384824}

[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/cc/layers/video_frame_provider.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/cc/layers/video_frame_provider_client_impl.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/cc/layers/video_frame_provider_client_impl.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/cc/layers/video_frame_provider_client_impl_unittest.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/cc/layers/video_layer_impl.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/common/gpu/stream_texture_android.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/common/gpu/stream_texture_android.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/renderer/gpu/stream_texture_host_android.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/renderer/gpu/stream_texture_host_android.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/renderer/media/android/stream_texture_factory_impl.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/gpu/command_buffer/service/gl_stream_texture_image.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/e6dac6465349eccf7d9cf5850be54cea24631095/media/blink/video_frame_compositor_unittest.cc

same issue here. youtube works fine but all other videos (from facebook or streaming websites show upside down)


Google Chrome	51.0.2695.1 (Official Build) canary (64-bit)
Platform	8138.0.0 (Official Build) canary-channel lumpy
Yep. This was my fault, sorry. I didn't realise that there was a platform that used the stream texture infrastructure in a partial way, which caused the removal of some otherwise dead code to not be a NOP.
 Issue 600492  has been merged into this issue.
Issue 600050 has been merged into this issue.
FWIW the revert in comment #12 is in Chrome 51.0.2699.0 however the issue is still present, did we expect this to fix it?

If not do we have any leads on a fix?

We were looking to push a Dev release today but this is going to block it.
I'm looking into it, but I'm not familiar with ChromeOS at all, and I'm having trouble getting an environment for testing up and running at the same time as everything else I've got to deal with at the same time.

I honestly expected that that revert would fix it. At least I have a reasonable hypothesis why my CL could cause it (and if it is I have a followup CL that should fix it, too).
> did we expect this to fix it?

yes.  it's quite unexpected that it did not.

given that it's a dev blocker, it might be better if somebody who is already familiar with the CrOS dev workflow does another bisect.

Cc: scunning...@chromium.org
+scunningham to confirm

I am under the impression CrOS 8150 which contains 51.0.2699.0 Chrome was still exhibiting this failure.
Tested on 8150.0.0/ 51.0.2699.0 Samus and Cyan. The  issue seems to be fixed
tobiasj@: Check out https://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chromium-browser for how to build the browser with a known ChromeOS image.
Cc: -danakj@chromium.org
only GPU decoded video is flipped. I guess one of vaapi_xxx files has root bug.
Fundamentally the problem is that there are (at least) 3 different paths where video frames are provided in a GL_TEXTURE_EXTERNAL_OES texture. The paths meet at various points in the rendering pipeline and at various points we y-flip the texture (multiple times). My CL was trying to clean that up in two stages, but in the first stage I managed to remove one flip from the ChromeOS path while fixing an Android path.

I have a plan for fixing this now (assuming that the above is accurate), and I should have it done today.
Cc: piman@chromium.org ananta@chromium.org zmo@chromium.org vmi...@chromium.org kbr@chromium.org
 Issue 145806  has been merged into this issue.
Project Member

Comment 27 by bugdroid1@chromium.org, Apr 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9268401feeaaa16ffe7719d25e165108bddf9141

commit 9268401feeaaa16ffe7719d25e165108bddf9141
Author: tobiasjs <tobiasjs@chromium.org>
Date: Thu Apr 07 11:48:55 2016

Make Android StreamTexture implement GLStreamTextureImage

Then make CopySubTextureCHROMIUM use the transform matrix provided
by GLStreamTextureImage, and stop informing the compositor of the
transform matrix. Video quad drawing applies the matrix automatically
via the CHROMIUM_stream_texture_matrix extension.

This has has the advantage that we no longer need custom texture copying
code in StreamTextureAndroid and we also don't need the IPC that updates
the compositor view of the transform matrix (because it is applied
transparently when the texture is sampled from). This in turn means that
the transform matrix can't get out of sync with the texture data.

BUG= 226218 , 371500 , 588837 , 599848 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/8f2032aeb81c2a6a48ed2ed622859b59d84fbbaa
Cr-Commit-Position: refs/heads/master@{#384122}

Review URL: https://codereview.chromium.org/1746983002

Cr-Commit-Position: refs/heads/master@{#385714}

[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/cc/layers/video_frame_provider.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/cc/layers/video_frame_provider_client_impl.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/cc/layers/video_frame_provider_client_impl.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/cc/layers/video_frame_provider_client_impl_unittest.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/common/gpu/stream_texture_android.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/common/gpu/stream_texture_android.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/renderer/gpu/stream_texture_host_android.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/renderer/gpu/stream_texture_host_android.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/renderer/media/android/stream_texture_factory_impl.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/gpu/command_buffer/service/gl_stream_texture_image.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/gpu/ipc/common/gpu_messages.h
[modify] https://crrev.com/9268401feeaaa16ffe7719d25e165108bddf9141/media/blink/video_frame_compositor_unittest.cc

Status: Fixed (was: Assigned)
Corrected CL relanded. Based upon the lack of further issues, marking as fixed.
Cc: srcv@chromium.org
srcv@ - please verify, thanks!
Status: Verified (was: Fixed)
Verified on 8172.1.0 / 51.0.2704.0.

Comment 31 by srcv@chromium.org, Apr 12 2016

Verified this issue on device Ultima with version 51.0.2701.0 / 8174.0.0 

Comment 32 by kbr@chromium.org, Apr 13 2016

Thanks tobiasjs@! Is this exercised by the WebGL conformance tests? If so, are there any suppressions in src/content/test/gpu/gpu_tests/webgl_conformance_expectations.py that can be removed after your fix?

Texture copies are, and those conformance test suppressions have now been reverted (thanks for LGTMing and landing that revert). AFAICT there are no automated tests that assert that the result of rendering a video frame is in the expected orientation, so there's always the chance that that path can break again. https://codereview.chromium.org/1818073002/ simplifies that path further, removing another pair of matching y-flips.

Comment 34 by kbr@chromium.org, Apr 14 2016

That's very good.

Would you like to add a test to src/content/test/gpu/page_sets/pixel_tests.py (which will be picked up by src/content/test/gpu/gpu_tests/pixel.py -- see https://www.chromium.org/developers/testing/gpu-testing#TOC-Updating-and-Adding-New-Pixel-Tests-to-the-GPU-Bots ) which renders a well-known video (like something with red on top and green on the bottom) directly, rather than through WebGL or other APIs? If you can either provide the video or the page itself I would be happy to help you integrate the test.

Thanks for the pointer. I'd be happy to add the test.
Cc: -scunning...@chromium.org

Comment 37 by kbr@chromium.org, Apr 14 2016

Cool. Do you want to file a new bug about adding the test and block it on this one? Tell me how I can help.

Sign in to add a comment