Fix visible size for V4L2 VDA

content/common/gpu/media/v4l2_video_decode_accelerator.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <dlfcn.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/videodev2.h>
 #include <poll.h>
 #include <sys/eventfd.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/memory/shared_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/numerics/safe_conversions.h"
 #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)                        \
   do {                                         \
     LOG(ERROR) << "Setting error state:" << x; \
     SetErrorState(x);                          \
   } while (0)
 
 #define IOCTL_OR_ERROR_RETURN_VALUE(type, arg, value)              \
   do {                                                             \
@@ skipping 704 matching lines @@
     *endpos = size;
     return true;
   }
 
   // Run this initialization only on first startup.
   if (decoder_state_ == kInitialized) {
     DVLOG(3) << "DecodeBufferInitial(): running initialization";
     // Success! Setup our parameters.
     if (!CreateBuffersForFormat(format))
       return false;
+    if (!GetCropSize(frame_buffer_size_, &frame_visible_size_))
+      frame_visible_size_ = frame_buffer_size_;
 
     // We expect to process the initial buffer once during stream init to
     // configure stream parameters, but will not consume the steam data on that
     // iteration.  Subsequent iterations (including after reset) do not require
     // the stream init step.
     *endpos = 0;
   } else {
     *endpos = size;
   }
 
@@ skipping 319 matching lines @@
     output_record.at_device = false;
     if (dqbuf.m.planes[0].bytesused == 0) {
       // This is an empty output buffer returned as part of a flush.
       free_output_buffers_.push(dqbuf.index);
     } else {
       DCHECK_GE(dqbuf.timestamp.tv_sec, 0);
       output_record.at_client = true;
       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(frame_visible_size_));
       pending_picture_ready_.push(
           PictureRecord(output_record.cleared, picture));
       SendPictureReady();
       output_record.cleared = true;
       decoder_frames_at_client_++;
     }
     output_buffer_queued_count_--;
   }
 
   NotifyFlushDoneIfNeeded();
@@ skipping 439 matching lines @@
     LOG(ERROR) << "Couldn't get format information after resolution change";
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return;
   }
 
   if (!CreateBuffersForFormat(format)) {
     LOG(ERROR) << "Couldn't reallocate buffers after resolution change";
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return;
   }
+  if (!GetCropSize(frame_buffer_size_, &frame_visible_size_))

[Pawel Osciak, 2015/01/29 05:02:04]

Could we call this from GetFormatInfo()?

[kcwu, 2015/01/29 09:33:11]

Done.

+    frame_visible_size_ = frame_buffer_size_;
 
   decoder_state_ = kDecoding;
 
   if (resolution_change_reset_pending_) {
     resolution_change_reset_pending_ = false;
     ResetTask();
     return;
   }
 
   if (!StartDevicePoll())
@@ skipping 81 matching lines @@
     return false;
   }
 
   return true;
 }
 
 bool V4L2VideoDecodeAccelerator::CreateBuffersForFormat(
     const struct v4l2_format& format) {
   DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
   output_planes_count_ = format.fmt.pix_mp.num_planes;
   frame_buffer_size_.SetSize(

[Owen Lin, 2015/01/29 02:42:09]

Since we set |frame_buffer_size_| here, how about getting the
|frame_visible_size_| at the same time?

[kcwu, 2015/01/29 02:58:38]

I'm not sure ... since this function is named "CreateBuffersForFormat".
Getting crop size here seems weird.

[Owen Lin, 2015/01/29 03:40:11]

But we have already set |frame_buffer_size_| inside the function. How about
considering both |frame_buffer_size_| and |frame_visible_size_| as attributes of
the OutputBuffers. So we set them when we create the buffers.

[kcwu, 2015/01/29 09:33:10]

Moved to GetFormatInfo()

       format.fmt.pix_mp.width, format.fmt.pix_mp.height);
   DVLOG(3) << "CreateBuffersForFormat(): new resolution: "
            << frame_buffer_size_.ToString();
 
   if (!CreateOutputBuffers())
     return false;
 
   return true;
 }
 
+bool V4L2VideoDecodeAccelerator::GetCropSize(const gfx::Size& buffer_size,
+                                             gfx::Size* crop_size) {
+  DCHECK_EQ(decoder_thread_.message_loop(), base::MessageLoop::current());
+
+  struct v4l2_crop crop_arg;

[Pawel Osciak, 2015/01/29 05:02:04]

memset to 0 please.

[kcwu, 2015/01/29 09:33:10]

Done.

+  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) << "Crop rectangle is " << rect.ToString();

[Pawel Osciak, 2015/01/29 05:02:04]

Let's stop calling this crop and say visible size everywhere instead please.

[kcwu, 2015/01/29 09:33:11]

Done.

+  if (!gfx::Rect(buffer_size).Contains(rect)) {

[Pawel Osciak, 2015/01/29 05:02:04]

Please also check if it's !Empty()

[kcwu, 2015/01/29 09:33:10]

Done.

+    DLOG(ERROR) << "crop rectangle " << rect.ToString()
+                << " is not inside buffer size " << buffer_size.ToString();
+    return false;
+  }
+
+  if (!rect.origin().IsOrigin()) {

[Pawel Osciak, 2015/01/29 05:02:04]

Please add a comment why.

[kcwu, 2015/01/29 09:33:10]

Done.

+    DLOG(ERROR) << "Unexpected crop rectangle " << rect.ToString()
+                << ", top-left is not origin";
+    return false;
+  }
+  *crop_size = rect.size();
+
+  return true;
+}
+
 bool V4L2VideoDecodeAccelerator::CreateInputBuffers() {
   DVLOG(3) << "CreateInputBuffers()";
   // We always run this as we prepare to initialize.
   DCHECK_EQ(decoder_state_, kUninitialized);
   DCHECK(!input_streamon_);
   DCHECK(input_buffer_map_.empty());
 
   struct v4l2_requestbuffers reqbufs;
   memset(&reqbufs, 0, sizeof(reqbufs));
   reqbufs.count  = kInputBufferCount;
@@ skipping 300 matching lines @@
   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) {
     DVLOG(3) << "IsResolutionChangeNecessary(): Resolution change detected";
     return true;
   }
+
+  gfx::Size new_crop_size;
+  if (GetCropSize(new_size, &new_crop_size) &&
+      new_crop_size != frame_visible_size_) {
+    DVLOG(3) << "IsResolutionChangeNecessary(): crop size change detected";

[Pawel Osciak, 2015/01/29 05:02:04]

Crop size does not influence resolution change. If coded size didn't change, a
new crop size does not require reallocating buffers.

[kcwu, 2015/01/29 09:33:11]

Done.

+    return true;
+  }
+
   return false;
 }
 
 }  // namespace content