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

Issue 672895 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO until 2019-01-24
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue 779974
issue 679679
issue 701060



Sign in to add a comment

Video artefacts for some MP4 video textures in WebGL

Reported by postfil...@gmail.com, Dec 9 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36 OPR/41.0.2353.69

Steps to reproduce the problem:
1. Visit these two tests (first one uses the original video, second one uses video re-encoded via ffmpeg to reduce bitrate):

http://alteredqualia.com/xg/examples/webgl_video_performance_MP4_1080p.html
http://alteredqualia.com/xg/examples/webgl_video_performance_MP4_1080p_v2.html

2. Observe bottom rows of videos.

What is the expected behavior?
Full frame of clouds video should be shown (see "video1-firefox.jpg" attached image for how it looks in Firefox where it's rendered correctly).

What went wrong?
In current stable Chrome 55.0.2883.75:

- video 1 is rendered with green-violet artefacts in the last few rows (video1-chrome-55.jpg)
- video 2 is rendered with last real row repeated in last few rendered rows (video2-chrome-55.jpg)

In latest Chrome Canary 57.0.2946.0:

- video 1 is rendered with last few rows black
- video 2 is rendered with last few rows black

Did this work before? N/A 

Chrome version:  57.0.2946.0  Channel: n/a
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: 

This seems to be happening for just some MP4 videos. 

For example this version seems to be rendered ok (rescaled via ffmpeg to from 1920x1080 to 2048x1024):

http://alteredqualia.com/xg/examples/webgl_video_performance_MP4_1024p.html

VP9 videos don't seem to be affected with both POT and NPOT dimensions:

http://alteredqualia.com/xg/examples/webgl_video_performance_VP9_1080p.html
http://alteredqualia.com/xg/examples/webgl_video_performance_VP9_1024p.html

So it looks more like CPU-side video frames upload / conversion issue for MP4 decoder then WebGL rendering issue.
 
video1-firefox.jpg
67.6 KB View Download
video1-chrome-55.jpg
69.4 KB View Download
video2-chrome-55.jpg
67.6 KB View Download
video1-chrome-57.jpg
77.3 KB View Download
video2-chrome-57.jpg
74.5 KB View Download

Comment 1 by kbr@chromium.org, Dec 9 2016

Components: -Blink Internals>GPU>Video Blink>WebGL

Comment 2 by kbr@chromium.org, Dec 9 2016

Owner: kbr@chromium.org
Status: Assigned (was: Unconfirmed)
Reproducible both in Stable 54.0.2840.99 (Official Build) m (64-bit) and Canary 57.0.2945.0 (Official Build) canary (64-bit).

I suspect it's a bug in the uploading of hardware-accelerated decoded videos; in other words, the GPU-GPU copy path. I'm adding some logging now to confirm that.

Comment 3 by kbr@chromium.org, Dec 9 2016

Note: the artifact doesn't occur when playing back the video normally, through the HTMLVideoElement directly to the screen.

video.html
104 bytes View Download

Comment 4 by kbr@chromium.org, Dec 9 2016

Cc: kbr@chromium.org
Owner: jbau...@chromium.org
It's going down the HTMLVideoElement::copyVideoTextureToPlatformTexture path:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp?q=texImageHelperHTMLVideoElement&sq=package:chromium&dr=CSs&l=5146

It ends up in SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture, called from SkCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture:

https://cs.chromium.org/chromium/src/media/renderers/skcanvas_video_renderer.cc?l=749

This ends up calling CopyTextureCHROMIUM. internal_format is GL_RGB, flip_y is true, and premultiply_alpha is false. If flip_y is forced to false, the artifact is still there, just at the top of the uploaded texture.

Something seems to be going wrong in the YUV-to-RGB conversion for the hardware-accelerated video's texture. I don't know that code.

John, it looks like you may be familiar with this based on your work for  Issue 625900 . Could you help me with this bug?

Separately, postfilter@: could you please reduce this test case further, eliminating the use of xg and boiling it down to a single HTML file? There are too many variables in the test right now and it's difficult to understand whether scaling of the video element is occurring, etc. which may be affecting things. Thanks. Also, if you could attach a zip archive to this bug report containing all the resources that would be helpful.

Comment 5 by kbr@chromium.org, Dec 9 2016

Cc: jbau...@chromium.org
Owner: kbr@chromium.org
Taking back after discussion with jbauman; will diagnose a couple more things about this.

Yeah, one possibility we discussed was that there's an issue with the mismatch between the coded size of the video (most likely 1920x1088) and the visible size (probably 1920x1080). 
GpuVideoDecoder::PictureReady has information about calculating the visible size. I don't know how we're handling the size of the webgl texture and the copy into it in this case.

Comment 7 by kbr@chromium.org, Dec 9 2016

As jbauman@ indicated offline, for both AQ's video-to-WebGL upload case and the raw HTMLVideoElement case above, it's going down DXVAVideoDecodeAccelerator::CopyTexture(ID3D11Texture2D* src_texture, ID3D11Texture2D* dest_texture, ...). Not yet clear why this artifact would show up just in the WebGL upload case.

Oh, there's also "natural size". I'm not sure how that interacts with the others.

Comment 9 by kbr@chromium.org, Dec 9 2016

In both test cases, in DXVAVideoDecodeAccelerator::ProcessPendingSamples, the video's width is 1920 and the height is 1088. (John pointed out that the sizes are rounded up to macroblock size, i.e., multiples of 16.)

I wonder whether the last 8 rows in the decoded image are garbage, and the compositor knows to crop its DrawQuad to 1920x1080?

@kbr Here is a smaller test case, a modification of Mozilla's MDN video texture example:

http://alteredqualia.com/tmp/video-artefacts-test/
http://alteredqualia.com/tmp/video-artefacts-test/video-artefacts-test.zip 

At least here it seems to behave the same as my XG tests, exactly the same artefacts.

BTW one curious thing I noticed with this simpler example is that in Chrome Canary those artefact rows are actually transparent, not just black (they take color of canvas element background).


Comment 11 by kbr@chromium.org, Dec 10 2016

Status: Started (was: Assigned)
Thanks AQ for the reduced test case and John for your help too.

The bug is this: while WebGLRenderingContextBase::texImageHelperHTMLVideoElement thinks it's creating a texture with the size of the video's natural width and height, in reality it's using CopyTextureCHROMIUM on the video decoder's output texture, which is larger than that due to the encoder's rounding up to the macroblock size (1920x1088 vs. 1920x1080). The fix is to use CopySubTextureCHROMIUM to select the correct sub-rectangle of the source texture.

AQ, can you reproduce this with any smaller video? I don't have the tools readily available to encode one. Can you see this issue with a solid color video (say, green) whose height is divisible by 8, but not 16? I'd like to check in a WebGL conformance test for this when fixing this bug.

Thanks.

npot-video.mp4 in the webgl conformance suite should have a similar issue (though since the bottom is a uniform color any artifacts may not be noticeable). I think we could check what the actual texture size is.
For what it's worth, I'm actually having hard time reproducing this with simple solid color videos.

So far I tried creating solid green videos of sizes 8x8, 24x24, 1920x1080 with length of 4 frames, I also added some blue frames for total length of 16 frames, and they render in Chrome as expected (they do fail to render at all in Firefox though, and smaller ones also trip VLC media player badly).

Other more complex real 1080p videos do seem to have these artefacts. 

Not sure what's complexity threshold to force MP4 codec into producing these artefacty videos.
Solid-color videos probably won't display the problem, as the most efficient encoding of the video will have the non-visible region the same color as the visible region. You'd need some sort of pattern on the bottom to make it obvious there's garbage below.

Or for something like the npot-video.mp4, the location of the horizontal line separating the colors should be different depending on whether the visible region or the entire coded region is shown.
This codec wrangling seems to be tricky. 

I tried and even with the original clouds video I'm not getting artefacts anymore after resizing to 1/2 and 1/4 sizes.

If it's of any help, at least shrinking clouds video to 1 second length and reducing quality (see attached video file #1) still does preserve the black/transparent rows artefacts in Canary.

Stable Chrome is trickier, as even with other videos artefacts are less obvious because they are derived from neighboring video pixels, so not easily programmatically detected (in some videos they are tinted neighboring pixels, in others they seem to be duplicated neighboring rows).

---------------

I also tried resizing npot-video.mp4 to 1920x1080 (see attached video file #2). This does create the same solid black/transparent rows artefact as for other videos in Canary, but no obvious artefacts in stable Chrome.


clouds_1920x1080_1sec.mp4
22.5 KB View Download
npot-video-1920x1080.mp4
76.8 KB View Download

Comment 16 by kbr@chromium.org, Dec 10 2016

Unfortunately, neither OpenGL ES 2.0 nor 3.0 (and, consequently, neither WebGL 1.0 nor 2.0) support glGetTexLevelParameter, so there's no direct way to query the size of a level of a texture.

Let me see if I can programmatically determine a difference in the line separating the colors of npot-video.mp4 with and without the bug fix.

In WebGL2 there is "textureSize" function on the GLSL side.

I just tested it with few videos and it seems to get the height value of 1088 for 1080p videos which had artefacts (and importantly also height of 1080 for 1080p videos which didn't have artefacts).

highp ivec2 size = textureSize(uSampler,0);
if ( size.y == 1088 ) { ... }


Comment 18 by kbr@chromium.org, Dec 10 2016

Cc: zmo@chromium.org kainino@chromium.org
Thanks AQ, that's an excellent observation. I'd forgotten all about that intrinsic. It seems like the only reliable way to detect this bug.

Thanks for your test video npot-video-1920x1080.mp4 above. I made several efforts to construct tiny test cases using Photoshop and ffmpeg but they all worked correctly. Yours is the only one I could find which reproduced the issue.

https://codereview.chromium.org/2562003003 fixes the bug in Chrome and https://github.com/KhronosGroup/WebGL/pull/2202 adds a WebGL 2.0 regression test for it.

Comment 19 by kbr@chromium.org, Jan 11 2017

Blocking: 679679
Project Member

Comment 20 by bugdroid1@chromium.org, Jan 11 2017

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

commit 4910ae5b242c441bd047b78bb5cb46ad954920fa
Author: kbr <kbr@chromium.org>
Date: Wed Jan 11 00:52:28 2017

Fix the size of video textures uploaded to WebGL.

When the hardware-accelerated video decoding path is in use, the
expected size of the destination WebGL texture is the video's natural
width and height. However, the code was blindly copying the entire
video source texture, which might include padding rows from the video
encoder.

Change to using CopySubTextureCHROMIUM in these code paths, and rely on
the caller to allocate the destination texture with the expected size.

A regression test for this bug is being added to the WebGL 2.0 conformance
suite in https://github.com/KhronosGroup/WebGL/pull/2202 .

BUG= 672895 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2562003003
Cr-Commit-Position: refs/heads/master@{#442745}

[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/content/renderer/media/android/webmediaplayer_android.cc
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/content/renderer/media/android/webmediaplayer_android.h
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/media/renderers/skcanvas_video_renderer.cc
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/media/renderers/skcanvas_video_renderer.h
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/4910ae5b242c441bd047b78bb5cb46ad954920fa/third_party/WebKit/public/platform/WebMediaPlayer.h

Comment 21 by kbr@chromium.org, Jan 11 2017

Finally fixed on top-of-tree. Sorry for the delay.

Comment 22 by kbr@chromium.org, Jan 11 2017

Cc: qiankun....@intel.com yunchao...@intel.com yang...@intel.com
 Issue 679679  has been merged into this issue.

Comment 23 by kbr@chromium.org, Jan 11 2017

Status: Fixed (was: Started)
The new test has been rolled in in https://codereview.chromium.org/2626963002/ and it's passing on all platforms, including Windows where it was previously failing. Closing as Fixed.

AQ, if you can verify this with Chrome Canary on Windows in the next few days, please follow up. Thanks.

@kbr It seems to be fixed, though I'm not 100% sure.

On the notebook where I did originally observe this issue (Quadro 2000M GPU, Windows 7), those multi-row artefacts are now completely gone in the latest Canary.

On another notebook (GTX 970M, Windows 8.1) in the latest Canary there is still green + violet tint in the last pixel row of one video (but just in that particular video):

http://alteredqualia.com/xg/examples/webgl_video_performance_MP4_1080p.html

It's weird though as before, on that other notebook, similarly hued tint was in many more rows (maybe 5-10, see the attached image in the original report).

Firefox on that notebook (GTX 970M) and Chrome Canary on other notebook (Quadro 2000M) don't have that green tint, so it's not something in the video.

Also notably it shows just with DirectX rendering backend but not OpenGL rendering backend, which make it look like it's maybe something in rendering, not video pixels data.

Normally I wouldn't even notice, it's pretty subtle, just now I was looking closely for bug fix verification.

FWIW that new conformance test passes there just fine:

https://www.khronos.org/registry/webgl/sdk/tests/conformance2/textures/misc/npot-video-sizing.html
mp4-video-tint.png
663 KB View Download

Comment 25 by kbr@chromium.org, Jan 13 2017

Thanks for following up AQ. If you just play the video with a video tag rather than through WebGL on that machine, do you see the same strange tint at the bottom?

Maybe this last problem is something Chrome's hardware-accelerated video decoding pipeline is doing wrong, or an edge case in our YUV conversion.

@kbr Video played via DOM tag doesn't have that strange tint.

Some more clues from poking around with the original WebGL version:

1) Tint height seem to be one screen pixel row, not one video pixel row (resizing video on the screen doesn't seem to change the tinted area, which I think should be the case if there was some issue with video decoding)

2) Changing video texture filtering from linear to nearest makes tint disappear.

3) Changing video texture wrapping from clamping to repeat changes appearance of artefacts (there is also extra "out-of-place" color row in addition to tint)

4) It shows just with DirectX rendering backend, not OpenGL rendering backend.

So it seems like some texture sampling edge case thing (maybe something where ANGLE emulates something to be compatible with OpenGL?).

Comment 27 by kbr@chromium.org, Jan 13 2017

Thanks AQ for following up.

I'm sure there's still some problem here, then, but am not sure how we could write a test for it, since this seems fixed on at least some video cards.

Feel free to file a new bug about the continued problem, with whatever test case you can come up with. Calling this one fixed; I'll block your new bug on this one.

Comment 28 by kbr@chromium.org, Mar 14 2017

Blocking: 701060
Labels: Hotlist-ConOps
Labels: -Hotlist-ConOps
Project Member

Comment 31 by bugdroid1@chromium.org, Apr 14 2017

Labels: merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df726b319141fe6869bdae70100bf24daa460d2f

commit df726b319141fe6869bdae70100bf24daa460d2f
Author: Kenneth Russell <kbr@chromium.org>
Date: Fri Apr 14 22:58:07 2017

Revert "Fix the size of video textures uploaded to WebGL."

This reverts commit 4910ae5b242c441bd047b78bb5cb46ad954920fa.

This revert is being done only on the M58 branch, as the lower-risk
option of fixing the draw/upload paths from video to both 2D canvas
and WebGL.

Tested locally. Verified that new WebGL conformance test
texture-corner-case-videos.html passes with this revert applied.

BUG= 672895 , 701060 
TBR=chrishtr@chromium.org,esprehn@chromium.org,dalecurtis@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2820823003 .
Cr-Commit-Position: refs/branch-heads/3029@{#723}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/media/renderers/skcanvas_video_renderer.cc
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/media/renderers/skcanvas_video_renderer.h
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/df726b319141fe6869bdae70100bf24daa460d2f/third_party/WebKit/public/platform/WebMediaPlayer.h

Comment 32 by kbr@chromium.org, Oct 31 2017

Blocking: 779974

Sign in to add a comment