media/gpu/v4l2(s)vda: also adjust imported output buffer height

When importing a buffer, the provided buffer's size may differ from the
size we requested from ProvidePictureBuffersWithVisibleRect(). We
already accounted the width difference, but not the height, which
resulted in green lines being visible when the provided buffer differed
in height, and was single-planar: the image processor would render using
the assumed height, while the GPU would render using the real height,
resulting in the color plane being offset by one or two lines.

Since the VDA and SVDA both need to do this, factorize the actual buffer
size's computation code into a VDA helper, and call it from both.

BUG=chromium:982172
BUG=b:141579960
BUG=b:146599071
BUG=b:141965953
TEST=Youtube playing in 240, 360, 480 and 720p on desktop and Android
with Hana, Krane and Kevin. No artefact or green line visible.

Change-Id: I0e28446805fd382cbe0d3aab30b704c85ca52b72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011784
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: David Staessens <dstaessens@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733899}

media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc

@@ -1484,34 +1484,25 @@ void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask(
     return;
   }
 
-  // TODO(crbug.com/982172): This must be done in AssignPictureBuffers().
-  // However the size of PictureBuffer might not be adjusted by ARC++. So we
-  // keep this until ARC++ side is fixed.
+  // TODO(crbug.com/982172): ARC++ may adjust the size of the buffer due to
+  // allocator constraints, but the VDA API does not provide a way for it to
+  // communicate the actual buffer size. If we are importing, make sure that the
+  // actual buffer size is coherent with what we expect, and adjust our size if
+  // needed.
   if (output_mode_ == Config::OutputMode::IMPORT) {
     DCHECK_GT(handle.planes.size(), 0u);
-    const int32_t stride = handle.planes[0].stride;
-    const int plane_horiz_bits_per_pixel =
-        VideoFrame::PlaneHorizontalBitsPerPixel(
-            gl_image_format_fourcc_->ToVideoPixelFormat(), 0);
-    if (plane_horiz_bits_per_pixel == 0 ||
-        (stride * 8) % plane_horiz_bits_per_pixel != 0) {
-      VLOGF(1) << "Invalid format " << gl_image_format_fourcc_->ToString()
-               << " or stride " << stride;
-      NOTIFY_ERROR(INVALID_ARGUMENT);
-      return;
-    }
+    const gfx::Size handle_size = v4l2_vda_helpers::NativePixmapSizeFromHandle(
+        handle, *gl_image_format_fourcc_, gl_image_size_);
 
-    int adjusted_coded_width = stride * 8 / plane_horiz_bits_per_pixel;
     // If this is the first picture, then adjust the EGL width.
     // Otherwise just check that it remains the same.
     if (state_ == kAwaitingPictureBuffers) {
-      DCHECK_GE(adjusted_coded_width, gl_image_size_.width());
-      gl_image_size_.set_width(adjusted_coded_width);
+      DCHECK_GE(handle_size.width(), gl_image_size_.width());
+      DVLOGF(3) << "Original gl_image_size=" << gl_image_size_.ToString()
+                << ", adjusted buffer size=" << handle_size.ToString();
+      gl_image_size_ = handle_size;
     }
-    DCHECK_EQ(gl_image_size_.width(), adjusted_coded_width);
-
-    DVLOGF(3) << "Original gl_image_size=" << gl_image_size_.ToString()
-              << ", adjusted coded width=" << adjusted_coded_width;
+    DCHECK_EQ(gl_image_size_, handle_size);
 
     // For allocate mode, the IP will already have been created in
     // AssignPictureBuffersTask.

media/gpu/v4l2/v4l2_vda_helpers.cc

@@ -109,5 +109,34 @@ std::unique_ptr<ImageProcessor> CreateImageProcessor(
   return image_processor;
 }
 
+gfx::Size NativePixmapSizeFromHandle(const gfx::NativePixmapHandle& handle,
+                                     const Fourcc fourcc,
+                                     const gfx::Size& current_size) {
+  const uint32_t stride = handle.planes[0].stride;
+  const uint32_t horiz_bits_per_pixel =
+      VideoFrame::PlaneHorizontalBitsPerPixel(fourcc.ToVideoPixelFormat(), 0);
+  DCHECK_NE(horiz_bits_per_pixel, 0u);
+  // Stride must fit exactly on a byte boundary (8 bits per byte)
+  DCHECK_EQ((stride * 8) % horiz_bits_per_pixel, 0u);
+
+  // Actual width of buffer is stride (in bits) divided by bits per pixel.
+  int adjusted_coded_width = stride * 8 / horiz_bits_per_pixel;
+  // If the buffer is multi-planar, then the height of the buffer does not
+  // matter as long as it covers the visible area and we can just return
+  // the current height.
+  // For single-planar however, the actual height can be inferred by dividing
+  // the start offset of the second plane by the stride of the first plane,
+  // since the second plane is supposed to start right after the first one.
+  int adjusted_coded_height =
+      handle.planes.size() > 1 && handle.planes[1].offset != 0
+          ? handle.planes[1].offset / adjusted_coded_width
+          : current_size.height();
+
+  DCHECK_GE(adjusted_coded_width, current_size.width());
+  DCHECK_GE(adjusted_coded_height, current_size.height());
+
+  return gfx::Size(adjusted_coded_width, adjusted_coded_height);
+}
+
 }  // namespace v4l2_vda_helpers
 }  // namespace media

media/gpu/v4l2/v4l2_vda_helpers.h

@@ -57,6 +57,14 @@ std::unique_ptr<ImageProcessor> CreateImageProcessor(
     scoped_refptr<base::SequencedTaskRunner> client_task_runner,
     ImageProcessor::ErrorCB error_cb);
 
+// When importing a buffer (ARC++ use-case), the buffer's actual size may
+// be different from the requested one. However, the actual size is never
+// provided to us - so we need to compute it from the NativePixmapHandle.
+// Given the |handle| and |fourcc| of the buffer, adjust |current_size| to
+// the actual computed size of the buffer and return the new size.
+gfx::Size NativePixmapSizeFromHandle(const gfx::NativePixmapHandle& handle,
+                                     const Fourcc fourcc,
+                                     const gfx::Size& current_size);
 }  // namespace v4l2_vda_helpers
 }  // namespace media
 

media/gpu/v4l2/v4l2_video_decode_accelerator.cc

@@ -626,34 +626,25 @@ void V4L2VideoDecodeAccelerator::ImportBufferForPictureTask(
     return;
   }
 
-  // TODO(crbug.com/982172): This must be done in AssignPictureBuffers().
-  // However the size of PictureBuffer might not be adjusted by ARC++. So we
-  // keep this until ARC++ side is fixed.
+  // TODO(crbug.com/982172): ARC++ may adjust the size of the buffer due to
+  // allocator constraints, but the VDA API does not provide a way for it to
+  // communicate the actual buffer size. If we are importing, make sure that the
+  // actual buffer size is coherent with what we expect, and adjust our size if
+  // needed.
   if (output_mode_ == Config::OutputMode::IMPORT) {
     DCHECK_GT(handle.planes.size(), 0u);
-    const int32_t stride = handle.planes[0].stride;
-    const int plane_horiz_bits_per_pixel =
-        VideoFrame::PlaneHorizontalBitsPerPixel(
-            egl_image_format_fourcc_->ToVideoPixelFormat(), 0);
-    if (plane_horiz_bits_per_pixel == 0 ||
-        (stride * 8) % plane_horiz_bits_per_pixel != 0) {
-      VLOGF(1) << "Invalid format " << egl_image_format_fourcc_->ToString()
-               << " or stride " << stride;
-      NOTIFY_ERROR(INVALID_ARGUMENT);
-      return;
-    }
+    const gfx::Size handle_size = v4l2_vda_helpers::NativePixmapSizeFromHandle(
+        handle, *egl_image_format_fourcc_, egl_image_size_);
 
-    int adjusted_coded_width = stride * 8 / plane_horiz_bits_per_pixel;
     // If this is the first picture, then adjust the EGL width.
     // Otherwise just check that it remains the same.
     if (decoder_state_ == kAwaitingPictureBuffers) {
-      DCHECK_GE(adjusted_coded_width, egl_image_size_.width());
-      egl_image_size_.set_width(adjusted_coded_width);
+      DCHECK_GE(handle_size.width(), egl_image_size_.width());
+      DVLOGF(3) << "Original egl_image_size=" << egl_image_size_.ToString()
+                << ", adjusted buffer size=" << handle_size.ToString();
+      egl_image_size_ = handle_size;
     }
-    DCHECK_EQ(egl_image_size_.width(), adjusted_coded_width);
-
-    DVLOGF(3) << "Original egl_image_size=" << egl_image_size_.ToString()
-              << ", adjusted coded width=" << adjusted_coded_width;
+    DCHECK_EQ(egl_image_size_, handle_size);
 
     // For allocate mode, the IP will already have been created in
     // AssignPictureBuffersTask.