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

Unified Diff: media/renderers/skcanvas_video_renderer.cc

Issue 2818493002: Revert of Fix broken draw/upload paths from videos to 2D canvas and WebGL. (Closed)
Patch Set: 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 d60e854997dd984c3a06f0eba6f89de7ee58286b..1e2e9b21ab1a7e25977f25aeb0a8452a56a533ca 100644
--- a/media/renderers/skcanvas_video_renderer.cc
+++ b/media/renderers/skcanvas_video_renderer.cc
@@ -160,20 +160,6 @@
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,
@@ -198,10 +184,12 @@
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,
- SkCanvasVideoRenderer::SingleFrameForVideoElementOrCanvas,
- source_texture, GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE, true, false);
+ gl, video_frame, source_texture, true, false);
} else {
gl->WaitSyncTokenCHROMIUM(mailbox_holder.sync_token.GetConstData());
source_texture = gl->CreateAndConsumeTextureCHROMIUM(
@@ -765,11 +753,7 @@
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);
@@ -792,35 +776,14 @@
// "flip_y == true" means to reverse the video orientation while
// "flip_y == false" means to keep the intrinsic orientation.
- 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);
- }
-
+ // 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);
gl->DeleteTextures(1, &source_texture);
gl->Flush();
@@ -833,9 +796,6 @@
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());
@@ -872,35 +832,15 @@
destination_gl->CreateAndConsumeTextureCHROMIUM(
mailbox_holder.texture_target, mailbox_holder.mailbox.name);
- // 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);
- }
-
+ // 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);
destination_gl->DeleteTextures(1, &intermediate_texture);
// Wait for destination context to consume mailbox before deleting it in
@@ -915,9 +855,8 @@
SyncTokenClientImpl client(canvas_gl);
video_frame->UpdateReleaseSyncToken(&client);
} else {
- CopyVideoFrameSingleTextureToGLTexture(
- destination_gl, video_frame.get(), SingleFrameForWebGL, texture,
- internal_format, format, type, premultiply_alpha, flip_y);
+ CopyVideoFrameSingleTextureToGLTexture(destination_gl, video_frame.get(),
+ texture, 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