Fix DCHECKs in V4l2VideoDevice.

content/common/gpu/media/v4l2_video_device.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 <linux/videodev2.h>
 
 #include "base/numerics/safe_conversions.h"
 #include "content/common/gpu/media/exynos_v4l2_video_device.h"
 #include "content/common/gpu/media/tegra_v4l2_video_device.h"
 
@@ skipping 63 matching lines @@
   } else {
     LOG(FATAL) << "Add more cases as needed";
     return 0;
   }
 }
 
 // static
 gfx::Size V4L2Device::CodedSizeFromV4L2Format(struct v4l2_format format) {
   gfx::Size coded_size;
   gfx::Size visible_size;
   media::VideoFrame::Format frame_format = media::VideoFrame::UNKNOWN;

[scherkus (not reviewing), 2014/08/20 17:13:53]

are we using actual VideoFrames in V4L2 code?

it seems like we're adding functions to VideoFrame such as RowBytes() and
PlaneHorizontalBitsPerPixel() out of convenience, but I wonder whether we'd be
better off having V4L2 code do the calculations themselves instead of having a
dependency on VideoFrame code

[wuchengli, 2014/08/21 01:03:57]

V4L2 code use actual VideoFrames like v4l2_image_process.cc and
v4l2_video_encode_accelerator.cc. Other files in content/common/gpu/media also
use actual VideoFrame. So the dependency already exists.

RowBytes() uses the code of existing row_bytes(), which is used by several
different places. row_bytes() is not a new function. Duplicating it here is not
so good.

As for PlaneHorizontalBitsPerPixel, its implementation is like
VideoFrame::PlaneAllocationSize, which is used by several other places. Putting
it in VideoFrame looks right.

[scherkus (not reviewing), 2014/08/21 21:37:54]

Thanks for the pointers.

I'm a little wary of making VideoFrame a big dumping ground of utility functions
... it might be worth thinking about splitting things into a specific utility
file vs. the class. We don't have to do this now, though.

   int bytesperline = 0;
   int sizeimage = 0;
 
   if (V4L2_TYPE_IS_MULTIPLANAR(format.type) &&
       format.fmt.pix_mp.num_planes > 0) {
     bytesperline =
         base::checked_cast<int>(format.fmt.pix_mp.plane_fmt[0].bytesperline);
     sizeimage =
         base::checked_cast<int>(format.fmt.pix_mp.plane_fmt[0].sizeimage);
     visible_size.SetSize(base::checked_cast<int>(format.fmt.pix_mp.width),
                          base::checked_cast<int>(format.fmt.pix_mp.height));
     frame_format =
         V4L2Device::V4L2PixFmtToVideoFrameFormat(format.fmt.pix_mp.pixelformat);
   } else {
     bytesperline = base::checked_cast<int>(format.fmt.pix.bytesperline);
     sizeimage = base::checked_cast<int>(format.fmt.pix.sizeimage);
     visible_size.SetSize(base::checked_cast<int>(format.fmt.pix.width),
                          base::checked_cast<int>(format.fmt.pix.height));
     frame_format =
         V4L2Device::V4L2PixFmtToVideoFrameFormat(format.fmt.pix.pixelformat);
   }
 
   int horiz_bpp =
       media::VideoFrame::PlaneHorizontalBitsPerPixel(frame_format, 0);
+  DVLOG(3) << __func__ << ": bytesperline=" << bytesperline
+           << ", sizeimage=" << sizeimage
+           << ", visible_size=" << visible_size.ToString() << ", frame_format="
+           << media::VideoFrame::FormatToString(frame_format)
+           << ", horiz_bpp=" << horiz_bpp;
   if (sizeimage == 0 || bytesperline == 0 || horiz_bpp == 0 ||
       (bytesperline * 8) % horiz_bpp != 0) {
     DLOG(ERROR) << "Invalid format provided";
     return coded_size;
   }
 
   // Round up sizeimage to full bytesperlines. sizeimage does not have to be
   // a multiple of bytesperline, as in V4L2 terms it's just a byte size of
   // the buffer, unrelated to its payload.
   sizeimage = ((sizeimage + bytesperline - 1) / bytesperline) * bytesperline;
 
   coded_size.SetSize(bytesperline * 8 / horiz_bpp, sizeimage / bytesperline);
+  DVLOG(3) << "coded_size=" << coded_size.ToString();
 
   // Sanity checks. Calculated coded size has to contain given visible size
   // and fulfill buffer byte size requirements for each plane.
   DCHECK(gfx::Rect(coded_size).Contains(gfx::Rect(visible_size)));
 
   if (V4L2_TYPE_IS_MULTIPLANAR(format.type)) {
     for (size_t i = 0; i < format.fmt.pix_mp.num_planes; ++i) {
-      DCHECK_LE(
-          format.fmt.pix_mp.plane_fmt[i].sizeimage,
-          media::VideoFrame::PlaneAllocationSize(frame_format, i, coded_size));
-      DCHECK_LE(format.fmt.pix_mp.plane_fmt[i].bytesperline,
-                base::checked_cast<__u32>(coded_size.width()));
+      DCHECK_EQ(format.fmt.pix_mp.plane_fmt[i].bytesperline,
+                base::checked_cast<__u32>(media::VideoFrame::RowBytes(
+                    i, frame_format, coded_size.width())));
     }
   }
 
   return coded_size;
 }
 
 }  //  namespace content