GpuVDA: Override ProvidePictureBuffersWithVisibleRect().

CL:2122484 overrode ProvidePictureBuffersWithVisibleRect() in
VdaVideoDecoder in order to fix an issue where the coded size of a
VideoFrame was not being advertised correctly to the client.

This fix must also be applied to GpuVideoDecodeAccelerator. Otherwise,
an AssignPictureBuffers() call can fail due to the validation in [1].
This CL makes the default implementation of
ProvidePictureBuffersWithVisibleRect() behave as in VdaVideoDecoder.
However, it only does so when the texture target is
GL_TEXTURE_EXTERNAL_OES; otherwise, it uses the same implementation as
before.

In order to test, I adapted the video_decode example that comes with the
Native Client SDK to use a 1920x1080 video which triggers this problem.
Before this CL, no video would be rendered and we would get validation
errors from [1]. After this CL, no errors are observed in the logs and
the video is rendered.

I tested this across multiple devices. Kukui is the only one that failed
(Native Client says it's a crash) but this happens even before
CL:2122484, so it's not a regression caused by the series of fixes for
the referenced bug. Nyan_kitty shows a small white artifact on the
bottom that will need to be addressed later.

The test results are in [2].

[1] https://source.chromium.org/chromium/chromium/src/+/master:media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc;l=1275-1286;drc=2b1b0cc6e2fa7c34965c39caa561e28e9e831626?originalUrl=https:%2F%2Fcs.chromium.org%2F
[2] https://drive.google.com/open?id=1ErWDq9bd3ZnLnPU3uhlGQ2zV6zJw7wyT

Bug: 1062002,1067794
Test: kevin, kukui, eve, hana, nyan_kitty with procedure described above.
Change-Id: I0177f63ab7d19ca196646a017ee532880ec555fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134417
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756494}

media/gpu/ipc/service/gpu_video_decode_accelerator.cc

@@ -386,6 +386,13 @@ bool GpuVideoDecodeAccelerator::Initialize(
       stub_->channel()->gpu_channel_manager()->gpu_driver_bug_workarounds();
   const gpu::GpuPreferences& gpu_preferences =
       stub_->channel()->gpu_channel_manager()->gpu_preferences();
+
+  if (config.output_mode !=
+      VideoDecodeAccelerator::Config::OutputMode::ALLOCATE) {
+    DLOG(ERROR) << "Only ALLOCATE mode is supported";
+    return false;
+  }
+
   video_decode_accelerator_ =
       vda_factory->CreateVDA(this, config, gpu_workarounds, gpu_preferences);
   if (!video_decode_accelerator_) {

media/gpu/ipc/service/vda_video_decoder.cc

@@ -518,34 +518,6 @@ void VdaVideoDecoder::ProvidePictureBuffers(uint32_t requested_num_of_buffers,
                      textures_per_buffer, dimensions, texture_target));
 }
 
-void VdaVideoDecoder::ProvidePictureBuffersWithVisibleRect(
-    uint32_t requested_num_of_buffers,
-    VideoPixelFormat format,
-    uint32_t textures_per_buffer,
-    const gfx::Size& dimensions,
-    const gfx::Rect& visible_rect,
-    uint32_t texture_target) {
-  // In ALLOCATE mode, |vda_| is responsible for allocating storage for the
-  // result of the decode. However, we are responsible for creating the GL
-  // texture to which we'll attach the decoded image. The decoder needs the
-  // buffer to be of size = coded size, but for the purposes of working with a
-  // graphics API (e.g., GL), the client does not need to know about the
-  // non-visible area. For example, for a 360p H.264 video with a visible
-  // rectangle of 0,0,640x360, the coded size is 640x368, but the GL texture
-  // that the client uses should be only 640x360. Therefore, we pass
-  // |visible_rect|.size() here as the requested size of the picture buffers.
-  //
-  // Note that we use GetRectSizeFromOrigin() to handle the unusual case in
-  // which the visible rectangle does not start at (0, 0). For example, for an
-  // H.264 video with a visible rectangle of 2,2,635x360, the coded size is
-  // 640x360, but the GL texture that the client uses should be 637x362. This is
-  // because we want the texture to include the non-visible area to the left and
-  // on the top of the visible rectangle so that the compositor can calculate
-  // the UV coordinates to omit the non-visible area.
-  ProvidePictureBuffers(requested_num_of_buffers, format, textures_per_buffer,
-                        GetRectSizeFromOrigin(visible_rect), texture_target);
-}
-
 void VdaVideoDecoder::ProvidePictureBuffersAsync(uint32_t count,
                                                  VideoPixelFormat pixel_format,
                                                  uint32_t planes,

media/gpu/ipc/service/vda_video_decoder.h

@@ -126,12 +126,6 @@ class VdaVideoDecoder : public VideoDecoder,
                              uint32_t textures_per_buffer,
                              const gfx::Size& dimensions,
                              uint32_t texture_target) override;
-  void ProvidePictureBuffersWithVisibleRect(uint32_t requested_num_of_buffers,
-                                            VideoPixelFormat format,
-                                            uint32_t textures_per_buffer,
-                                            const gfx::Size& dimensions,
-                                            const gfx::Rect& visible_rect,
-                                            uint32_t texture_target) override;
   void DismissPictureBuffer(int32_t picture_buffer_id) override;
   void PictureReady(const Picture& picture) override;
   void NotifyEndOfBitstreamBuffer(int32_t bitstream_buffer_id) override;

media/video/video_decode_accelerator.cc

@@ -5,7 +5,10 @@
 #include "media/video/video_decode_accelerator.h"
 
 #include <GLES2/gl2.h>
+#include <GLES2/gl2ext.h>
+
 #include "base/logging.h"
+#include "media/base/video_util.h"
 
 namespace media {
 
@@ -36,8 +39,31 @@ void VideoDecodeAccelerator::Client::ProvidePictureBuffersWithVisibleRect(
     const gfx::Size& dimensions,
     const gfx::Rect& visible_rect,
     uint32_t texture_target) {
-  ProvidePictureBuffers(requested_num_of_buffers, format, textures_per_buffer,
-                        dimensions, texture_target);
+  if (texture_target == GL_TEXTURE_EXTERNAL_OES) {
+    // In ALLOCATE mode and when the texture target is GL_TEXTURE_EXTERNAL_OES,
+    // |video_decode_accelerator_| is responsible for allocating storage for the
+    // result of the decode. However, we are responsible for creating the GL
+    // texture to which we'll attach the decoded image. The decoder needs the
+    // buffer to be of size = coded size, but for the purposes of working with a
+    // graphics API (e.g., GL), the client does not need to know about the
+    // non-visible area. For example, for a 360p H.264 video with a visible
+    // rectangle of 0,0,640x360, the coded size is 640x368, but the GL texture
+    // that the client uses should be only 640x360. Therefore, we pass
+    // |visible_rect|.size() here as the requested size of the picture buffers.
+    //
+    // Note that we use GetRectSizeFromOrigin() to handle the unusual case in
+    // which the visible rectangle does not start at (0, 0). For example, for an
+    // H.264 video with a visible rectangle of 2,2,635x360, the coded size is
+    // 640x360, but the GL texture that the client uses should be 637x362. This
+    // is because we want the texture to include the non-visible area to the
+    // left and on the top of the visible rectangle so that the compositor can
+    // calculate the UV coordinates to omit the non-visible area.
+    ProvidePictureBuffers(requested_num_of_buffers, format, textures_per_buffer,
+                          GetRectSizeFromOrigin(visible_rect), texture_target);
+  } else {
+    ProvidePictureBuffers(requested_num_of_buffers, format, textures_per_buffer,
+                          dimensions, texture_target);
+  }
 }
 
 VideoDecodeAccelerator::~VideoDecodeAccelerator() = default;

media/video/video_decode_accelerator.h

@@ -216,8 +216,10 @@ class MEDIA_EXPORT VideoDecodeAccelerator {
                                        uint32_t texture_target) = 0;
 
     // This is the same as ProvidePictureBuffers() except that |visible_rect| is
-    // also included. The default implementation of VDA would call
-    // ProvidePictureBuffers().
+    // also included. The default implementation calls ProvidePictureBuffers()
+    // setting |dimensions| = GetRectSizeFromOrigin(|visible_rect|) when
+    // |texture_target| is GL_TEXTURE_EXTERNAL_OES; otherwise, it passes along
+    // all parameters to ProvidePictureBuffers() as they are.
     virtual void ProvidePictureBuffersWithVisibleRect(
         uint32_t requested_num_of_buffers,
         VideoPixelFormat format,