Remove kIgnoreResolutionLimitsForAcceleratedVideoDecode flag

content/common/gpu/media/v4l2_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 <libdrm/drm_fourcc.h>
 #include <linux/videodev2.h>
 
 #include "base/numerics/safe_conversions.h"
 #include "content/common/gpu/media/generic_v4l2_device.h"
 #if defined(ARCH_CPU_ARMEL)
@@ skipping 179 matching lines @@
 
   // Sanity checks. Calculated coded size has to contain given visible size
   // and fulfill buffer byte size requirements.
   DCHECK(gfx::Rect(coded_size).Contains(gfx::Rect(visible_size)));
   DCHECK_LE(sizeimage,
             media::VideoFrame::AllocationSize(frame_format, coded_size));
 
   return coded_size;
 }
 
+gfx::Size V4L2Device::GetMaxSupportedResolution(unsigned int pixelformat) {
+  gfx::Size max_resolution;
+  v4l2_frmsizeenum frame_size;
+  memset(&frame_size, 0, sizeof(frame_size));
+  frame_size.pixel_format = pixelformat;
+  for (; Ioctl(VIDIOC_ENUM_FRAMESIZES, &frame_size) == 0; ++frame_size.index) {
+    if (frame_size.type == V4L2_FRMIVAL_TYPE_DISCRETE) {

[Pawel Osciak, 2015/04/22 08:50:55]

This define is for framerate enumeration. The correct define for frame sizes is
V4L2_FRMSIZE_TYPE_DISCRETE. Same for other defines.

[henryhsu, 2015/04/23 03:56:42]

Done.

+      if (frame_size.discrete.width >

[Pawel Osciak, 2015/04/22 08:50:55]

Perhaps merge into the if above?

[henryhsu, 2015/04/23 03:56:42]

Done.

+              static_cast<unsigned int>(max_resolution.width()) &&

[Pawel Osciak, 2015/04/22 08:50:55]

I'd prefer base::checked_cast on frame_size.discrete.width to int.

[henryhsu, 2015/04/23 03:56:42]

Done.

+          frame_size.discrete.height >
+              static_cast<unsigned int>(max_resolution.height())) {
+        max_resolution.SetSize(frame_size.discrete.width,

[Pawel Osciak, 2015/04/22 08:50:55]

If you get say 640x360 and next 640x480, I think this will ignore 640x480? Do we
need s/&&/||/ ?

[Pawel Osciak, 2015/04/22 10:01:38]

Sorry, not this, but probably s/>/>=/ ?

[henryhsu, 2015/04/23 03:56:42]

Use || may have the case: first is 640x480, second is 1080x360.
Is the case possible?

I'd prefer to use ">=".

+                               frame_size.discrete.height);
+      }
+    } else if (frame_size.type == V4L2_FRMIVAL_TYPE_STEPWISE ||
+               frame_size.type == V4L2_FRMIVAL_TYPE_CONTINUOUS) {
+      max_resolution.SetSize(frame_size.stepwise.max_width,
+                             frame_size.stepwise.max_height);

[Pawel Osciak, 2015/04/22 08:50:55]

You could break here to avoid an additional ioctl call.

[henryhsu, 2015/04/23 03:56:42]

Done.

+    }
+  }
+  if (max_resolution.IsEmpty()) {
+    LOG(ERROR) << "GetMaxSupportedResolution failed for format "

[Pawel Osciak, 2015/04/22 08:50:55]

s/format/fourcc/

[henryhsu, 2015/04/23 03:56:42]

Done.

+               << std::hex << pixelformat;
+    max_resolution.SetSize(1920, 1088);

[Pawel Osciak, 2015/04/22 08:50:55]

Please also say that we are falling back to 1088p in the log message.

[henryhsu, 2015/04/23 03:56:42]

Done.

+  }
+  return max_resolution;
+}
+
 }  //  namespace content