Use GLHelper for glReadPixels and format detection

content/renderer/media/renderer_gpu_video_accelerator_factories.cc

Index: content/renderer/media/renderer_gpu_video_accelerator_factories.cc
diff --git a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
index 94e0d78217fa621974b945fd79bbaee2756e8fd3..7b04bc3bf3b103014f86d6ea9406d4dfe6cc122c 100644
--- a/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
+++ b/content/renderer/media/renderer_gpu_video_accelerator_factories.cc
@@ -64,11 +64,24 @@ RendererGpuVideoAcceleratorFactories::GetContext3d() {
   if (context_provider_->IsContextLost()) {
     context_provider_->VerifyContexts();
     context_provider_ = NULL;
+    gl_helper_.reset(NULL);
     return NULL;
   }
   return context_provider_->WebContext3D();
 }
 
+GLHelper* RendererGpuVideoAcceleratorFactories::GetGLHelper() {
+  if (!GetContext3d())
+    return NULL;
+
+  if (gl_helper_.get() == NULL) {
+    gl_helper_.reset(new GLHelper(GetContext3d()->GetImplementation(),
+                                  GetContext3d()->GetContextSupport()));
+  }
+
+  return gl_helper_.get();
+}
+
 scoped_ptr<media::VideoDecodeAccelerator>
 RendererGpuVideoAcceleratorFactories::CreateVideoDecodeAccelerator() {
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ -178,59 +191,20 @@ void RendererGpuVideoAcceleratorFactories::ReadPixels(
     const SkBitmap& pixels) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  WebGraphicsContext3DCommandBufferImpl* context = GetContext3d();
-  if (!context)
+  GLHelper* gl_helper = GetGLHelper();
+  if (!gl_helper)
     return;
 
-  gpu::gles2::GLES2Implementation* gles2 = context->GetImplementation();
+  unsigned char* pixel_data =
+      static_cast<unsigned char*>(pixels.pixelRef()->pixels());
+
+  if (gl_helper->IsReadbackConfigSupported(pixels.colorType())) {
+    gl_helper->ReadbackTextureSync(
+        texture_id, visible_rect, pixel_data, pixels.colorType());
+  } else if (pixels.colorType() == kN32_SkColorType) {
+    gl_helper->ReadbackTextureSync(
+        texture_id, visible_rect, pixel_data, kRGBA_8888_SkColorType);
 
-  GLuint tmp_texture;
-  gles2->GenTextures(1, &tmp_texture);
-  gles2->BindTexture(GL_TEXTURE_2D, tmp_texture);
-  gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-  gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-  gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-  gles2->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
-  context->copyTextureCHROMIUM(
-      GL_TEXTURE_2D, texture_id, tmp_texture, 0, GL_RGBA, GL_UNSIGNED_BYTE);

[oetuaho-nv, 2014/08/11 14:56:41]

I don't think that the code can work without the copyTextureCHROMIUM part, which
you have removed from here. The texture might be external
(GL_TEXTURE_EXTERNAL_OES) in which case it is not possible to attach it directly
to a framebuffer.

-
-  GLuint fb;
-  gles2->GenFramebuffers(1, &fb);
-  gles2->BindFramebuffer(GL_FRAMEBUFFER, fb);
-  gles2->FramebufferTexture2D(
-      GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tmp_texture, 0);
-  gles2->PixelStorei(GL_PACK_ALIGNMENT, 4);
-
-#if SK_B32_SHIFT == 0 && SK_G32_SHIFT == 8 && SK_R32_SHIFT == 16 && \
-    SK_A32_SHIFT == 24
-  GLenum skia_format = GL_BGRA_EXT;
-  GLenum read_format = GL_BGRA_EXT;
-  GLint supported_format = 0;
-  GLint supported_type = 0;
-  gles2->GetIntegerv(GL_IMPLEMENTATION_COLOR_READ_FORMAT, &supported_format);
-  gles2->GetIntegerv(GL_IMPLEMENTATION_COLOR_READ_TYPE, &supported_type);
-  if (supported_format != GL_BGRA_EXT || supported_type != GL_UNSIGNED_BYTE) {
-    read_format = GL_RGBA;
-  }
-#elif SK_R32_SHIFT == 0 && SK_G32_SHIFT == 8 && SK_B32_SHIFT == 16 && \
-    SK_A32_SHIFT == 24
-  GLenum skia_format = GL_RGBA;
-  GLenum read_format = GL_RGBA;
-#else
-#error Unexpected Skia ARGB_8888 layout!
-#endif
-  gles2->ReadPixels(visible_rect.x(),
-                    visible_rect.y(),
-                    visible_rect.width(),
-                    visible_rect.height(),
-                    read_format,
-                    GL_UNSIGNED_BYTE,
-                    pixels.pixelRef()->pixels());
-  gles2->DeleteFramebuffers(1, &fb);
-  gles2->DeleteTextures(1, &tmp_texture);
-
-  if (skia_format != read_format) {
-    DCHECK(read_format == GL_RGBA);
     int pixel_count = visible_rect.width() * visible_rect.height();
     uint32_t* pixels_ptr = static_cast<uint32_t*>(pixels.pixelRef()->pixels());
     for (int i = 0; i < pixel_count; ++i) {
@@ -243,9 +217,9 @@ void RendererGpuVideoAcceleratorFactories::ReadPixels(
                         (b << SK_B32_SHIFT) |
                         (a << SK_A32_SHIFT);
     }
+  } else {
+    NOTREACHED();
   }
-
-  DCHECK_EQ(gles2->GetError(), static_cast<GLenum>(GL_NO_ERROR));
 }
 
 base::SharedMemory* RendererGpuVideoAcceleratorFactories::CreateSharedMemory(