V4L2VDA: Fix format initialization order.

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>
@@ skipping 249 matching lines @@
       V4L2_CAP_VIDEO_OUTPUT_MPLANE |
       V4L2_CAP_STREAMING;
   IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_QUERYCAP, &caps);
   if ((caps.capabilities & kCapsRequired) != kCapsRequired) {
     LOG(ERROR) << "Initialize(): ioctl() failed: VIDIOC_QUERYCAP"
         ", caps check failed: 0x" << std::hex << caps.capabilities;
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return false;
   }
 
-  if (!CreateInputBuffers())
-    return false;
-
-  if (!SetupOutputFormat())
+  if (!SetupFormats())
     return false;
 
   // Subscribe to the resolution change event.
   struct v4l2_event_subscription sub;
   memset(&sub, 0, sizeof(sub));
   sub.type = V4L2_EVENT_RESOLUTION_CHANGE;
   IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_SUBSCRIBE_EVENT, &sub);
 
   if (video_profile_ >= media::H264PROFILE_MIN &&
       video_profile_ <= media::H264PROFILE_MAX) {
     decoder_h264_parser_.reset(new media::H264Parser());
   }
 
+  if (!CreateInputBuffers())

[wuchengli, 2015/01/06 08:45:16]

Why this is moved here?

[Pawel Osciak, 2015/01/06 13:23:06]

I wanted to finish all the initialization before creating buffers. Not really a
big difference, but why not.

+    return false;
+
   if (!decoder_thread_.Start()) {
     LOG(ERROR) << "Initialize(): decoder thread failed to start";
     NOTIFY_ERROR(PLATFORM_FAILURE);
     return false;
   }
 
   // StartDevicePoll will NOTIFY_ERROR on failure, so IgnoreResult is fine here.
   decoder_thread_.message_loop()->PostTask(
       FROM_HERE,
       base::Bind(
@@ skipping 770 matching lines @@
       PLOG(ERROR) << "Dequeue(): ioctl() failed: VIDIOC_DQBUF";
       NOTIFY_ERROR(PLATFORM_FAILURE);
       return;
     }
     OutputRecord& output_record = output_buffer_map_[dqbuf.index];
     DCHECK(output_record.at_device);
     DCHECK(!output_record.at_client);
     DCHECK_NE(output_record.egl_image, EGL_NO_IMAGE_KHR);
     DCHECK_NE(output_record.picture_id, -1);
     output_record.at_device = false;
-    if (dqbuf.m.planes[0].bytesused + dqbuf.m.planes[1].bytesused == 0) {
+    if (dqbuf.m.planes[0].bytesused == 0) {

[wuchengli, 2015/01/06 08:45:16]

Is this related to format initialization order?

[Pawel Osciak, 2015/01/06 13:23:06]

No, this should've gone to the other enum formats CL. I had the CLs stacked for
testing, sorry about that. Removing.

       // 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,
@@ skipping 534 matching lines @@
       // EINVAL means we haven't seen sufficient stream to decode the format.
       *again = true;
       return true;
     } else {
       PLOG(ERROR) << __func__ << "(): ioctl() failed: VIDIOC_G_FMT";
       NOTIFY_ERROR(PLATFORM_FAILURE);
       return false;
     }
   }
 
+  // Make sure we are still getting the format we set on initialization.
+  if (format->fmt.pix_mp.pixelformat != output_format_fourcc_) {
+    LOG(ERROR) << "Unexpected format from G_FMT on output";
+    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(
       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::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());
 
-  __u32 pixelformat = V4L2Device::VideoCodecProfileToV4L2PixFmt(video_profile_);
-  if (!pixelformat) {
-    NOTREACHED();
-    return false;
-  }
-
-  struct v4l2_format format;
-  memset(&format, 0, sizeof(format));
-  format.type                              = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
-  format.fmt.pix_mp.pixelformat            = pixelformat;
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kIgnoreResolutionLimitsForAcceleratedVideoDecode))
-    format.fmt.pix_mp.plane_fmt[0].sizeimage = kInputBufferMaxSizeFor4k;
-  else
-    format.fmt.pix_mp.plane_fmt[0].sizeimage = kInputBufferMaxSizeFor1080p;
-  format.fmt.pix_mp.num_planes             = 1;
-  IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_S_FMT, &format);
-
   struct v4l2_requestbuffers reqbufs;
   memset(&reqbufs, 0, sizeof(reqbufs));
   reqbufs.count  = kInputBufferCount;
   reqbufs.type   = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
   reqbufs.memory = V4L2_MEMORY_MMAP;
   IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_REQBUFS, &reqbufs);
   input_buffer_map_.resize(reqbufs.count);
   for (size_t i = 0; i < input_buffer_map_.size(); ++i) {
     free_input_buffers_.push_back(i);
 
@@ skipping 17 matching lines @@
       PLOG(ERROR) << "CreateInputBuffers(): mmap() failed";
       return false;
     }
     input_buffer_map_[i].address = address;
     input_buffer_map_[i].length = buffer.m.planes[0].length;
   }
 
   return true;
 }
 
-bool V4L2VideoDecodeAccelerator::SetupOutputFormat() {
-  // We have to set up the format for output beforehand, because the driver may
-  // not allow changing it once we start streaming; whether it can support our
-  // chosen output format or not may depend on the input format.
+bool V4L2VideoDecodeAccelerator::SetupFormats() {
+  // We always run this as we prepare to initialize.
+  DCHECK_EQ(decoder_state_, kUninitialized);
+  DCHECK(!input_streamon_);
+  DCHECK(!output_streamon_);
+
+  __u32 input_format_fourcc =

[wuchengli, 2015/01/06 08:45:16]

indent

[Pawel Osciak, 2015/01/06 13:23:06]

Sorry where? Perhaps you missed the "__" before "u32"...

+      V4L2Device::VideoCodecProfileToV4L2PixFmt(video_profile_);
+  if (!input_format_fourcc) {
+    NOTREACHED();
+    return false;
+  }
+
+  size_t input_size;
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kIgnoreResolutionLimitsForAcceleratedVideoDecode))
+    input_size = kInputBufferMaxSizeFor4k;
+  else
+    input_size = kInputBufferMaxSizeFor1080p;
+
+  struct v4l2_format format;
+  memset(&format, 0, sizeof(format));
+  format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+  format.fmt.pix_mp.pixelformat = input_format_fourcc;
+  format.fmt.pix_mp.plane_fmt[0].sizeimage = input_size;
+  format.fmt.pix_mp.num_planes = 1;
+  IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_S_FMT, &format);
+
+  // We have to set up the format for output, because the driver may not allow
+  // changing it once we start streaming; whether it can support our chosen
+  // output format or not may depend on the input format.
   struct v4l2_fmtdesc fmtdesc;
   memset(&fmtdesc, 0, sizeof(fmtdesc));
   fmtdesc.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
   while (device_->Ioctl(VIDIOC_ENUM_FMT, &fmtdesc) == 0) {
     if (device_->CanCreateEGLImageFrom(fmtdesc.pixelformat)) {
       output_format_fourcc_ = fmtdesc.pixelformat;
       break;
     }
     ++fmtdesc.index;
   }
 
   if (output_format_fourcc_ == 0) {
     LOG(ERROR) << "Could not find a usable output format";
     return false;
   }
 
   // Just set the fourcc for output; resolution, etc., will come from the
   // driver once it extracts it from the stream.
-  struct v4l2_format format;
   memset(&format, 0, sizeof(format));
   format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
   format.fmt.pix_mp.pixelformat = output_format_fourcc_;
   IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_S_FMT, &format);
 
   return true;
 }
 
 bool V4L2VideoDecodeAccelerator::CreateOutputBuffers() {
   DVLOG(3) << "CreateOutputBuffers()";
@@ skipping 211 matching lines @@
   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;
   }
   return false;
 }
 
 }  // namespace content