Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(8)

Issue 565743003: Use AcceleratedImageBufferSurfaces for video texImage2D calls. (Closed)

Created:
6 years, 3 months ago by rileya (GONE FROM CHROMIUM)
Modified:
6 years ago
CC:
blink-reviews, jamesr, krit, blink-reviews-html_chromium.org, jbroman, danakj, dglazkov+blink, Rik, Stephen Chennney, aandrey+blink_chromium.org, pdr., rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Use AcceleratedImageBufferSurfaces for video texImage2D calls. This un-deletes AcceleratedImageBufferSurface and uses it when copying video frames to WebGL via TexImage2d. This allows the WebMediaPlayer to do YUV conversion on the GPU, resulting in significant speedups. https://codereview.chromium.org/531353002/ on the Chromium side is necessary for this performance improvement to take effect. On the Skia side, https://codereview.chromium.org/516463005/ will need to land in order for this to affect videos of the most common YUV color space. BUG=91208 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182029

Patch Set 1 #

Patch Set 2 : Remove unnecessary(?) condition #

Total comments: 6

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 7

Patch Set 7 : address comments #

Total comments: 2

Patch Set 8 : add comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -42 lines) Patch
M Source/core/html/canvas/WebGLRenderingContextBase.cpp View 1 2 3 4 5 6 7 3 chunks +21 lines, -1 line 0 comments Download
M Source/platform/blink_platform.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/platform/graphics/ImageBuffer.cpp View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
A + Source/platform/graphics/gpu/AcceleratedImageBufferSurface.h View 1 chunk +16 lines, -12 lines 0 comments Download
A + Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp View 1 2 1 chunk +18 lines, -28 lines 0 comments Download

Messages

Total messages: 15 (3 generated)
rileya (GONE FROM CHROMIUM)
I'm not very up-to-date on graphics stuff in Blink so there may be a better ...
6 years, 3 months ago (2014-09-11 21:34:51 UTC) #2
Justin Novosad
https://codereview.chromium.org/565743003/diff/20001/Source/core/html/canvas/WebGLRenderingContextBase.cpp File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/565743003/diff/20001/Source/core/html/canvas/WebGLRenderingContextBase.cpp#newcode3447 Source/core/html/canvas/WebGLRenderingContextBase.cpp:3447: ImageBuffer* buf = m_generatedImageCache.imageBuffer(size, false); This does not meet ...
6 years, 3 months ago (2014-09-12 15:43:35 UTC) #3
rileya (GONE FROM CHROMIUM)
Alright thanks for the suggestions! I switched to using scratch render targets instead of the ...
6 years, 3 months ago (2014-09-12 23:26:41 UTC) #4
Justin Novosad
+kbr for WebGL context changes. https://codereview.chromium.org/565743003/diff/100001/Source/core/html/canvas/WebGLRenderingContextBase.cpp File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/565743003/diff/100001/Source/core/html/canvas/WebGLRenderingContextBase.cpp#newcode3590 Source/core/html/canvas/WebGLRenderingContextBase.cpp:3590: OwnPtr<ImageBuffer> buf(ImageBuffer::create(adoptPtr(new AcceleratedImageBufferSurface(IntSize(video->videoWidth(), video->videoHeight()))))); ...
6 years, 3 months ago (2014-09-13 00:22:09 UTC) #6
rileya (GONE FROM CHROMIUM)
https://codereview.chromium.org/565743003/diff/100001/Source/core/html/canvas/WebGLRenderingContextBase.cpp File Source/core/html/canvas/WebGLRenderingContextBase.cpp (right): https://codereview.chromium.org/565743003/diff/100001/Source/core/html/canvas/WebGLRenderingContextBase.cpp#newcode3590 Source/core/html/canvas/WebGLRenderingContextBase.cpp:3590: OwnPtr<ImageBuffer> buf(ImageBuffer::create(adoptPtr(new AcceleratedImageBufferSurface(IntSize(video->videoWidth(), video->videoHeight()))))); On 2014/09/13 00:22:09, junov wrote: ...
6 years, 3 months ago (2014-09-13 00:40:39 UTC) #7
Justin Novosad
lgtm
6 years, 3 months ago (2014-09-13 02:37:21 UTC) #8
Ken Russell (switch to Gerrit)
Thanks very much for working on this. LGTM. Could you give any indication of the ...
6 years, 3 months ago (2014-09-14 22:53:01 UTC) #9
rileya (GONE FROM CHROMIUM)
> Could you give any indication of the speedups involved? Is more work needed beyond ...
6 years, 3 months ago (2014-09-15 22:27:17 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/565743003/130001
6 years, 3 months ago (2014-09-15 23:11:59 UTC) #12
Ken Russell (switch to Gerrit)
On 2014/09/15 22:27:17, rileya wrote: > > Could you give any indication of the speedups ...
6 years, 3 months ago (2014-09-16 00:42:58 UTC) #13
commit-bot: I haz the power
Committed patchset #8 (id:130001) as 182029
6 years, 3 months ago (2014-09-16 01:12:14 UTC) #14
dshwang
6 years ago (2014-12-17 13:49:52 UTC) #15
Message was sent while issue was closed.
nice optimization!

I measure perf improvement in Haswell i7 using upload-video-to-texture test
tools/perf/run_benchmark --browser=content-shell-release blink_perf.canvas
--page-filter=upload-video-to-texture
From 214.59 runs/sec To 1597.45 runs/sec

Powered by Google App Engine
This is Rietveld 408576698