Add GLStreamTextureImage

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 6165f0bfc01810488d24d95cdefd13dc05ad1f66..25d786af2c3e64e1746b488eb0bfe05fadf6ea45 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1597,6 +1597,10 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   void DoUniformMatrix4fv(
       GLint fake_location, GLsizei count, GLboolean transpose,
       const GLfloat* value);
+  void DoUniformMatrix4fvWithCustomMatrixCHROMIUM(GLint fake_location,
+                                                  GLint custom_matrix_id,
+                                                  GLboolean transpose,
+                                                  const GLfloat* value);
   void DoUniformMatrix2x3fv(
       GLint fake_location, GLsizei count, GLboolean transpose,
       const GLfloat* value);
@@ -7441,6 +7445,45 @@ void GLES2DecoderImpl::DoUniformMatrix4fv(
   glUniformMatrix4fv(real_location, count, transpose, value);
 }
 
+void GLES2DecoderImpl::DoUniformMatrix4fvWithCustomMatrixCHROMIUM(
+    GLint fake_location,
+    GLint custom_matrix_id,
+    GLboolean transpose,
+    const GLfloat* default_value) {
+  float gl_matrix[16];
+  bool copied_matrix = false;
+
+  DCHECK(custom_matrix_id == GL_CUSTOM_MATRIX_STREAM_TEXTURE_CHROMIUM);
+
+  // This refers to the bound external texture on the active unit.
+  TextureUnit& unit = state_.texture_units[state_.active_texture_unit];
+  if (TextureRef* texture_ref = unit.bound_texture_external_oes.get()) {
+    if (gl::GLImage* image =
+            texture_ref->texture()->GetLevelImage(GL_TEXTURE_EXTERNAL_OES, 0)) {

[reveman, 2016/02/18 17:16:50]

What if the image here is not a stream texture image? We should fail or use the
default value in that case I think. Do we need a more explicit mechanism to keep
track of stream texture images?

Maybe add GetLevelStreamTextureImage that returns a pointer to a
GLImageStreamTexture instance. Where the GLImageStreamTexture class is derived
from GLImage but with the addition of a GetMatrix() function. That way we keep
all this matrix related code outside the general GLImage interface and specific
to stream textures. Wdyt?

[liberato (no reviews please), 2016/02/18 19:34:41]

sure, sounds great.  i'll add Texture::[GS]etLevelStreamTextureImage.

plus i don't need to update ~one million GLImage implementations to return
false.

+      copied_matrix = image->GetCustomMatrix(custom_matrix_id, gl_matrix);
+    }
+  }
+
+  // If the matrix was unsupported, then supply the default.
+  // TODO(liberato): remove |default_value| and replace with an identity matrix.
+  // It is only present as a transitionary step until StreamTexture supplies
+  // the matrix via GLImage.  Once that happens, GLRenderer can quit sending
+  // in a default.
+  if (!copied_matrix)
+    memcpy(gl_matrix, default_value, sizeof(gl_matrix));
+
+  GLenum type = 0;
+  GLint real_location = -1;
+  GLsizei count = 1;
+  if (!PrepForSetUniformByLocation(fake_location, "glUniformMatrix4fv",
+                                   Program::kUniformMatrix4f, &real_location,
+                                   &type, &count)) {
+    return;
+  }
+
+  glUniformMatrix4fv(real_location, count, transpose, gl_matrix);
+}
+
 void GLES2DecoderImpl::DoUniformMatrix2x3fv(
     GLint fake_location, GLsizei count, GLboolean transpose,
     const GLfloat* value) {
@@ -13361,14 +13404,22 @@ void GLES2DecoderImpl::DoCopyTextureCHROMIUM(
   // before presenting.
   if (source_target == GL_TEXTURE_EXTERNAL_OES) {
     // TODO(hkuang): get the StreamTexture transform matrix in GPU process
-    // instead of using kIdentityMatrix crbug.com/226218.
+    // instead of using kIdentityMatrix crbug.com/226218.  AVDACodecImage does
+    // this correctly, but others (e.g., stream_texture_android.cc) don't.
+    GLfloat transform_matrix[16];
+    bool got_matrix = false;
+    if (gl::GLImage* image =
+            source_texture->GetLevelImage(GL_TEXTURE_EXTERNAL_OES, 0)) {
+      got_matrix = image->GetCustomMatrix(12345 /* TODO(liberato): enum */,
+                                          transform_matrix);

[reveman, 2016/02/18 17:16:49]

I don't think we should deal with the matrix here. Can we do CopyTexture support
as a separate patch?

[liberato (no reviews please), 2016/02/18 19:34:41]

'here' as in 'this CL' or 'here' as in 'DoCopyTextureCHROMIUM'?

i've left it as is for now.

[reveman, 2016/02/18 20:20:56]

'here' as in 'this CL' as I have some questions regarding this and was thinking
it'd be nice to land the other parts of this patch first.

Question; when drawing (BindTexImageCHROMIUM path), the client is responsible
for having this matrix used and can make a decision. When CopyTextureCHROMIUM is
called then it's magically applied without the client being aware, that seems
inconsistent. Can we instead require the client to draw the stream texture to
another texture to apply this matrix instead of using CopyTexture?

[liberato (no reviews please), 2016/02/19 18:39:55]

copy: sure, i'll split it.  my only concern is that it's the major reason of
doing any of this work, so they'd laugh at me at standup if i land this without
that part.  :)

inconsistent: i agree that it's inconsistent. the reason that i chose to do it
this way is that it prevents almost all consumers (e.g., webgl, canvas) from
needing to understand what a "stream texture".  it's a happy coincidence that
everybody except the gl_renderer just copies the thing.

it's somewhat safe, since the lifetime of video frames is tightly controlled.

there was a proposal to avoid the copy in webgl, among other advantages, six to
eight months ago.  i'm not sure where that ended up.  we would likely have to
copy the frame anyway in that case.

[reveman, 2016/02/19 19:15:39]

I'm OK landing something like this but prefer to review it as a separate patch
if possible. Long term it would be nice to move this copy logic to the client
instead of the GPU service as that means less code in the GPU process and more
code in the sandboxed renderer but I don't think that blocks us from making this
work properly using today's mechanism.

[liberato (no reviews please), 2016/02/19 19:48:10]

Acknowledged.

+    }
+    if (!got_matrix)
+      memcpy(transform_matrix, kIdentityMatrix, sizeof(transform_matrix));
     copy_texture_CHROMIUM_->DoCopyTextureWithTransform(
-        this, source_target, source_texture->service_id(),
-        dest_target, dest_texture->service_id(), source_width, source_height,
-        unpack_flip_y == GL_TRUE,
-        unpack_premultiply_alpha == GL_TRUE,
-        unpack_unmultiply_alpha == GL_TRUE,
-        kIdentityMatrix);
+        this, source_target, source_texture->service_id(), dest_target,
+        dest_texture->service_id(), source_width, source_height,
+        unpack_flip_y == GL_TRUE, unpack_premultiply_alpha == GL_TRUE,
+        unpack_unmultiply_alpha == GL_TRUE, transform_matrix);
   } else {
     copy_texture_CHROMIUM_->DoCopyTexture(
         this, source_target, source_texture->service_id(),