Fix visible size for V4L2 VDA

content/common/gpu/media/v4l2_video_decode_accelerator.cc

Index: content/common/gpu/media/v4l2_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/v4l2_video_decode_accelerator.cc b/content/common/gpu/media/v4l2_video_decode_accelerator.cc
index 7aa737406c9f2f92347c9024e4f03f2146254f8d..65e820f03edb2d274c4a4d6dfea97d6f3a295336 100644
--- a/content/common/gpu/media/v4l2_video_decode_accelerator.cc
+++ b/content/common/gpu/media/v4l2_video_decode_accelerator.cc
@@ -21,6 +21,7 @@
 #include "content/common/gpu/media/v4l2_video_decode_accelerator.h"
 #include "media/base/media_switches.h"
 #include "media/filters/h264_parser.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/gl/scoped_binders.h"
 
 #define NOTIFY_ERROR(x)                        \
@@ -339,7 +340,7 @@ void V4L2VideoDecodeAccelerator::AssignPictureBuffers(
   // thread is waiting on pictures_assigned_.
   DCHECK(free_output_buffers_.empty());
   for (size_t i = 0; i < output_buffer_map_.size(); ++i) {
-    DCHECK(buffers[i].size() == frame_buffer_size_);
+    DCHECK(buffers[i].size() == coded_size_);

[Pawel Osciak, 2015/02/02 08:54:16]

I have a feeling there was a reason why we didn't use DCHECK_EQ here?

[kcwu, 2015/02/02 09:01:47]

DCHECK_EQ doesn't like objects unable send to output stream.
gfx::Size needs to be ToString() for output explicitly.

 
     OutputRecord& output_record = output_buffer_map_[i];
     DCHECK(!output_record.at_device);
@@ -349,13 +350,9 @@ void V4L2VideoDecodeAccelerator::AssignPictureBuffers(
     DCHECK_EQ(output_record.picture_id, -1);
     DCHECK_EQ(output_record.cleared, false);
 
-    EGLImageKHR egl_image = device_->CreateEGLImage(egl_display_,
-                                                    egl_context_,
-                                                    buffers[i].texture_id(),
-                                                    frame_buffer_size_,
-                                                    i,
-                                                    output_format_fourcc_,
-                                                    output_planes_count_);
+    EGLImageKHR egl_image = device_->CreateEGLImage(

[Pawel Osciak, 2015/02/02 08:54:16]

To be honest I really preferred the previous indent, more readable.

[kcwu, 2015/02/02 09:01:47]

It's reindented by git-cl format. Do you still prefer old indent?

[wuchengli, 2015/02/02 09:05:15]

Let's follow git cl format. If the intent looks bad, we should fix git cl
format. If we manually change this code, the next person who uses git cl format
will still face the same problem.

+        egl_display_, egl_context_, buffers[i].texture_id(), coded_size_, i,
+        output_format_fourcc_, output_planes_count_);
     if (egl_image == EGL_NO_IMAGE_KHR) {
       LOG(ERROR) << "AssignPictureBuffers(): could not create EGLImageKHR";
       // Ownership of EGLImages allocated in previous iterations of this loop
@@ -729,8 +726,9 @@ bool V4L2VideoDecodeAccelerator::DecodeBufferInitial(
 
   // Check and see if we have format info yet.
   struct v4l2_format format;
+  gfx::Size visible_size;
   bool again = false;
-  if (!GetFormatInfo(&format, &again))
+  if (!GetFormatInfo(&format, &visible_size, &again))
     return false;
 
   if (again) {
@@ -743,7 +741,7 @@ bool V4L2VideoDecodeAccelerator::DecodeBufferInitial(
   if (decoder_state_ == kInitialized) {
     DVLOG(3) << "DecodeBufferInitial(): running initialization";
     // Success! Setup our parameters.
-    if (!CreateBuffersForFormat(format))
+    if (!CreateBuffersForFormat(format, visible_size))
       return false;
 
     // We expect to process the initial buffer once during stream init to
@@ -1084,9 +1082,8 @@ void V4L2VideoDecodeAccelerator::Dequeue() {
       DVLOG(3) << "Dequeue(): returning input_id=" << dqbuf.timestamp.tv_sec
                << " as picture_id=" << output_record.picture_id;
       const media::Picture& picture =
-          media::Picture(output_record.picture_id,
-                         dqbuf.timestamp.tv_sec,
-                         gfx::Rect(frame_buffer_size_));
+          media::Picture(output_record.picture_id, dqbuf.timestamp.tv_sec,
+                         gfx::Rect(visible_size_));
       pending_picture_ready_.push(
           PictureRecord(output_record.cleared, picture));
       SendPictureReady();
@@ -1534,14 +1531,15 @@ void V4L2VideoDecodeAccelerator::FinishResolutionChange() {
 
   struct v4l2_format format;
   bool again;
-  bool ret = GetFormatInfo(&format, &again);
+  gfx::Size visible_size;
+  bool ret = GetFormatInfo(&format, &visible_size, &again);
   if (!ret || again) {
     LOG(ERROR) << "Couldn't get format information after resolution change";
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return;
   }
 
-  if (!CreateBuffersForFormat(format)) {
+  if (!CreateBuffersForFormat(format, visible_size)) {
     LOG(ERROR) << "Couldn't reallocate buffers after resolution change";
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return;
@@ -1616,7 +1614,8 @@ void V4L2VideoDecodeAccelerator::SetErrorState(Error error) {
 }
 
 bool V4L2VideoDecodeAccelerator::GetFormatInfo(struct v4l2_format* format,
-                                                 bool* again) {
+                                               gfx::Size* visible_size,
+                                               bool* again) {
   DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
 
   *again = false;
@@ -1640,17 +1639,22 @@ bool V4L2VideoDecodeAccelerator::GetFormatInfo(struct v4l2_format* format,
     return false;
   }
 
+  gfx::Size coded_size(format->fmt.pix_mp.width, format->fmt.pix_mp.height);
+  if (!GetVisibleSize(coded_size, visible_size))
+    *visible_size = coded_size;
+
   return true;
 }
 
 bool V4L2VideoDecodeAccelerator::CreateBuffersForFormat(
-    const struct v4l2_format& format) {
+    const struct v4l2_format& format,
+    const gfx::Size& visible_size) {
   DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
   output_planes_count_ = format.fmt.pix_mp.num_planes;
-  frame_buffer_size_.SetSize(
-      format.fmt.pix_mp.width, format.fmt.pix_mp.height);
+  coded_size_.SetSize(format.fmt.pix_mp.width, format.fmt.pix_mp.height);
+  visible_size_ = visible_size;
   DVLOG(3) << "CreateBuffersForFormat(): new resolution: "
-           << frame_buffer_size_.ToString();
+           << coded_size_.ToString();
 
   if (!CreateOutputBuffers())
     return false;
@@ -1658,6 +1662,40 @@ bool V4L2VideoDecodeAccelerator::CreateBuffersForFormat(
   return true;
 }
 
+bool V4L2VideoDecodeAccelerator::GetVisibleSize(const gfx::Size& coded_size,
+                                                gfx::Size* visible_size) {
+  DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
+
+  struct v4l2_crop crop_arg;
+  memset(&crop_arg, 0, sizeof(crop_arg));
+  crop_arg.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+
+  IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_G_CROP, &crop_arg);
+
+  gfx::Rect rect(crop_arg.c.left, crop_arg.c.top, crop_arg.c.width,
+                 crop_arg.c.height);
+  DVLOG(3) << "visible rectangle is " << rect.ToString();
+  if (!gfx::Rect(coded_size).Contains(rect)) {
+    DLOG(ERROR) << "visible rectangle " << rect.ToString()
+                << " is not inside coded size " << coded_size.ToString();
+    return false;
+  }
+  if (rect.IsEmpty()) {
+    DLOG(ERROR) << "visible size is empty";
+    return false;
+  }
+
+  // Chrome assume picture frame is coded at (0, 0).
+  if (!rect.origin().IsOrigin()) {
+    DLOG(ERROR) << "Unexpected visible rectangle " << rect.ToString()
+                << ", top-left is not origin";
+    return false;
+  }
+  *visible_size = rect.size();
+
+  return true;
+}
+
 bool V4L2VideoDecodeAccelerator::CreateInputBuffers() {
   DVLOG(3) << "CreateInputBuffers()";
   // We always run this as we prepare to initialize.
@@ -1787,14 +1825,11 @@ bool V4L2VideoDecodeAccelerator::CreateOutputBuffers() {
 
   DVLOG(3) << "CreateOutputBuffers(): ProvidePictureBuffers(): "
            << "buffer_count=" << output_buffer_map_.size()
-           << ", width=" << frame_buffer_size_.width()
-           << ", height=" << frame_buffer_size_.height();
-  child_message_loop_proxy_->PostTask(FROM_HERE,
-                                      base::Bind(&Client::ProvidePictureBuffers,
-                                                 client_,
-                                                 output_buffer_map_.size(),
-                                                 frame_buffer_size_,
-                                                 device_->GetTextureTarget()));
+           << ", coded_size=" << coded_size_.ToString();
+  child_message_loop_proxy_->PostTask(
+      FROM_HERE, base::Bind(&Client::ProvidePictureBuffers, client_,

[Pawel Osciak, 2015/02/02 08:54:16]

Ditto preferred the old indent...

[kcwu, 2015/02/02 09:01:47]

It's reindented by git-cl format. Do you still prefer old indent?

+                            output_buffer_map_.size(), coded_size_,
+                            device_->GetTextureTarget()));
 
   // Wait for the client to call AssignPictureBuffers() on the Child thread.
   // We do this, because if we continue decoding without finishing buffer
@@ -1966,15 +2001,16 @@ bool V4L2VideoDecodeAccelerator::IsResolutionChangeNecessary() {
     return true;
   }
   struct v4l2_format format;
+  gfx::Size visible_size;
   bool again = false;
-  bool ret = GetFormatInfo(&format, &again);
+  bool ret = GetFormatInfo(&format, &visible_size, &again);
   if (!ret || again) {
     DVLOG(3) << "IsResolutionChangeNecessary(): GetFormatInfo() failed";
     return false;
   }
-  gfx::Size new_size(base::checked_cast<int>(format.fmt.pix_mp.width),
-                     base::checked_cast<int>(format.fmt.pix_mp.height));
-  if (frame_buffer_size_ != new_size) {
+  gfx::Size new_coded_size(base::checked_cast<int>(format.fmt.pix_mp.width),
+                           base::checked_cast<int>(format.fmt.pix_mp.height));
+  if (coded_size_ != new_coded_size) {
     DVLOG(3) << "IsResolutionChangeNecessary(): Resolution change detected";
     return true;
   }