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

Unified Diff: media/renderers/skcanvas_video_renderer.cc

Issue 2791813003: Fix broken draw/upload paths from videos to 2D canvas and WebGL. (Closed)
Patch Set: Disable GPU-GPU copies to floating-point textures on Android. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/renderers/skcanvas_video_renderer.h ('k') | third_party/WebKit/Source/core/html/HTMLVideoElement.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/renderers/skcanvas_video_renderer.cc
diff --git a/media/renderers/skcanvas_video_renderer.cc b/media/renderers/skcanvas_video_renderer.cc
index 1e2e9b21ab1a7e25977f25aeb0a8452a56a533ca..d60e854997dd984c3a06f0eba6f89de7ee58286b 100644
--- a/media/renderers/skcanvas_video_renderer.cc
+++ b/media/renderers/skcanvas_video_renderer.cc
@@ -160,6 +160,20 @@ sk_sp<SkImage> NewSkImageFromVideoFrameYUVTextures(
return img;
}
+bool VideoTextureNeedsClipping(const VideoFrame* video_frame) {
+ // There are multiple reasons that the size of the video frame's
+ // visible rectangle may differ from the coded size, including the
+ // encoder rounding up to the size of a macroblock, or use of
+ // non-square pixels.
+ //
+ // Some callers of these APIs (HTMLVideoElement and the 2D canvas
+ // context) already clip to the video frame's visible rectangle.
+ // WebGL on the other hand assumes that only the valid pixels are
+ // contained in the destination texture. This helper function
+ // determines whether this slower path is needed.
+ return video_frame->visible_rect().size() != video_frame->coded_size();
+}
+
// Creates a SkImage from a |video_frame| backed by native resources.
// The SkImage will take ownership of the underlying resource.
sk_sp<SkImage> NewSkImageFromVideoFrameNative(VideoFrame* video_frame,
@@ -184,12 +198,10 @@ sk_sp<SkImage> NewSkImageFromVideoFrameNative(VideoFrame* video_frame,
gl->GenTextures(1, &source_texture);
DCHECK(source_texture);
gl->BindTexture(GL_TEXTURE_2D, source_texture);
- const gfx::Size& natural_size = video_frame->natural_size();
- gl->TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, natural_size.width(),
- natural_size.height(), 0, GL_RGBA, GL_UNSIGNED_BYTE,
- nullptr);
SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture(
- gl, video_frame, source_texture, true, false);
+ gl, video_frame,
+ SkCanvasVideoRenderer::SingleFrameForVideoElementOrCanvas,
+ source_texture, GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE, true, false);
} else {
gl->WaitSyncTokenCHROMIUM(mailbox_holder.sync_token.GetConstData());
source_texture = gl->CreateAndConsumeTextureCHROMIUM(
@@ -753,7 +765,11 @@ void SkCanvasVideoRenderer::ConvertVideoFrameToRGBPixels(
void SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture(
gpu::gles2::GLES2Interface* gl,
VideoFrame* video_frame,
+ SingleFrameCopyMode copy_mode,
unsigned int texture,
+ unsigned int internal_format,
+ unsigned int format,
+ unsigned int type,
bool premultiply_alpha,
bool flip_y) {
DCHECK(video_frame);
@@ -776,14 +792,35 @@ void SkCanvasVideoRenderer::CopyVideoFrameSingleTextureToGLTexture(
// "flip_y == true" means to reverse the video orientation while
// "flip_y == false" means to keep the intrinsic orientation.
- // The video's texture might be larger than the natural size because
- // the encoder might have had to round up to the size of a macroblock.
- // Make sure to only copy the natural size to avoid putting garbage
- // into the bottom of the destination texture.
- const gfx::Size& natural_size = video_frame->natural_size();
- gl->CopySubTextureCHROMIUM(source_texture, 0, GL_TEXTURE_2D, texture, 0, 0, 0,
- 0, 0, natural_size.width(), natural_size.height(),
- flip_y, premultiply_alpha, false);
+ if (copy_mode == SingleFrameForVideoElementOrCanvas ||
+ !VideoTextureNeedsClipping(video_frame)) {
+ // No need to clip the source video texture.
+ gl->CopyTextureCHROMIUM(source_texture, 0, GL_TEXTURE_2D, texture, 0,
+ internal_format, type, flip_y, premultiply_alpha,
+ false);
+ } else {
+ // Must reallocate the destination texture and copy only a sub-portion.
+ gfx::Rect dest_rect = video_frame->visible_rect();
+#if DCHECK_IS_ON()
+ // The caller should have bound _texture_ to the GL_TEXTURE_2D
+ // binding point already.
+ GLuint current_texture = 0;
+ gl->GetIntegerv(GL_TEXTURE_BINDING_2D,
+ reinterpret_cast<GLint*>(&current_texture));
+ DCHECK_EQ(current_texture, texture);
+ // There should always be enough data in the source texture to
+ // cover this copy.
+ DCHECK_LE(dest_rect.width(), video_frame->coded_size().width());
+ DCHECK_LE(dest_rect.height(), video_frame->coded_size().height());
+#endif
+ gl->TexImage2D(GL_TEXTURE_2D, 0, internal_format, dest_rect.width(),
+ dest_rect.height(), 0, format, type, nullptr);
+ gl->CopySubTextureCHROMIUM(source_texture, 0, GL_TEXTURE_2D, texture, 0, 0,
+ 0, dest_rect.x(), dest_rect.y(),
+ dest_rect.width(), dest_rect.height(), flip_y,
+ premultiply_alpha, false);
+ }
+
gl->DeleteTextures(1, &source_texture);
gl->Flush();
@@ -796,6 +833,9 @@ bool SkCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture(
gpu::gles2::GLES2Interface* destination_gl,
const scoped_refptr<VideoFrame>& video_frame,
unsigned int texture,
+ unsigned int internal_format,
+ unsigned int format,
+ unsigned int type,
bool premultiply_alpha,
bool flip_y) {
DCHECK(thread_checker_.CalledOnValidThread());
@@ -832,15 +872,35 @@ bool SkCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture(
destination_gl->CreateAndConsumeTextureCHROMIUM(
mailbox_holder.texture_target, mailbox_holder.mailbox.name);
- // The video's texture might be larger than the natural size because
- // the encoder might have had to round up to the size of a macroblock.
- // Make sure to only copy the natural size to avoid putting garbage
- // into the bottom of the destination texture.
- const gfx::Size& natural_size = video_frame->natural_size();
- destination_gl->CopySubTextureCHROMIUM(
- intermediate_texture, 0, GL_TEXTURE_2D, texture, 0, 0, 0, 0, 0,
- natural_size.width(), natural_size.height(), flip_y, premultiply_alpha,
- false);
+ // See whether the source video texture must be clipped.
+ if (VideoTextureNeedsClipping(video_frame.get())) {
+ // Reallocate destination texture and copy only valid region.
+ gfx::Rect dest_rect = video_frame->visible_rect();
+#if DCHECK_IS_ON()
+ // The caller should have bound _texture_ to the GL_TEXTURE_2D
+ // binding point already.
+ GLuint current_texture = 0;
+ destination_gl->GetIntegerv(GL_TEXTURE_BINDING_2D,
+ reinterpret_cast<GLint*>(&current_texture));
+ DCHECK_EQ(current_texture, texture);
+ // There should always be enough data in the source texture to
+ // cover this copy.
+ DCHECK_LE(dest_rect.width(), video_frame->coded_size().width());
+ DCHECK_LE(dest_rect.height(), video_frame->coded_size().height());
+#endif
+ destination_gl->TexImage2D(GL_TEXTURE_2D, 0, internal_format,
+ dest_rect.width(), dest_rect.height(), 0,
+ format, type, nullptr);
+ destination_gl->CopySubTextureCHROMIUM(
+ intermediate_texture, 0, GL_TEXTURE_2D, texture, 0, 0, 0,
+ dest_rect.x(), dest_rect.y(), dest_rect.width(), dest_rect.height(),
+ flip_y, premultiply_alpha, false);
+ } else {
+ destination_gl->CopyTextureCHROMIUM(
+ intermediate_texture, 0, GL_TEXTURE_2D, texture, 0, internal_format,
+ type, flip_y, premultiply_alpha, false);
+ }
+
destination_gl->DeleteTextures(1, &intermediate_texture);
// Wait for destination context to consume mailbox before deleting it in
@@ -855,8 +915,9 @@ bool SkCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture(
SyncTokenClientImpl client(canvas_gl);
video_frame->UpdateReleaseSyncToken(&client);
} else {
- CopyVideoFrameSingleTextureToGLTexture(destination_gl, video_frame.get(),
- texture, premultiply_alpha, flip_y);
+ CopyVideoFrameSingleTextureToGLTexture(
+ destination_gl, video_frame.get(), SingleFrameForWebGL, texture,
+ internal_format, format, type, premultiply_alpha, flip_y);
}
return true;
« no previous file with comments | « media/renderers/skcanvas_video_renderer.h ('k') | third_party/WebKit/Source/core/html/HTMLVideoElement.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698