V4L2VDA: do not allocate input buffers in GPU child thread.

media/gpu/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 "media/gpu/v4l2_video_decode_accelerator.h"
 
 #include <dlfcn.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/videodev2.h>
@@ skipping 185 matching lines @@
   // descriptors, mmap() segments, etc.
   DCHECK(input_buffer_map_.empty());
   DCHECK(output_buffer_map_.empty());
 }
 
 bool V4L2VideoDecodeAccelerator::Initialize(const Config& config,
                                             Client* client) {
   DVLOGF(3) << "profile: " << config.profile
             << ", output_mode=" << static_cast<int>(config.output_mode);
   DCHECK(child_task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(decoder_state_, kUninitialized);

[kcwu, 2016/11/28 06:31:14]

Two questions,
1. should we add kInitializing state.
Without this, we cannot detect Initialize() twice before InitializeTask done.

2. should we add lock for decoder_state_?

[wuchengli, 2016/11/28 07:04:49]

I don't think it's very useful to add a state to just detect Initialize() is
called twice before InitializeTask done.
Decoder thread is started in Initialize(). That's why Initialize touches many
variables that should be only used on decode thread (e.g. SetupFormats). If
Initialize is called after decode thread starts, the result is really undefined.

[wuchengli, 2016/11/28 09:03:19]

As discussed, moved decoder_state_ = kInitialized; back to Initialize().

 
   if (config.is_encrypted) {
     NOTREACHED() << "Encrypted streams are not supported for this VDA";
     return false;
   }
 
   if (config.output_mode != Config::OutputMode::ALLOCATE &&
       config.output_mode != Config::OutputMode::IMPORT) {
     NOTREACHED() << "Only ALLOCATE and IMPORT OutputModes are supported";
     return false;
@@ skipping 50 matching lines @@
   IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_QUERYCAP, &caps);
   if ((caps.capabilities & kCapsRequired) != kCapsRequired) {
     LOGF(ERROR) << "ioctl() failed: VIDIOC_QUERYCAP"
                 << ", caps check failed: 0x" << std::hex << caps.capabilities;
     return false;
   }
 
   if (!SetupFormats())
     return false;
 
-  // Subscribe to the resolution change event.
-  struct v4l2_event_subscription sub;
-  memset(&sub, 0, sizeof(sub));
-  sub.type = V4L2_EVENT_SOURCE_CHANGE;
-  IOCTL_OR_ERROR_RETURN_FALSE(VIDIOC_SUBSCRIBE_EVENT, &sub);
-
   if (video_profile_ >= H264PROFILE_MIN && video_profile_ <= H264PROFILE_MAX) {
     decoder_h264_parser_.reset(new H264Parser());
   }
 
-  if (!CreateInputBuffers())
-    return false;
-
-  decoder_cmd_supported_ = IsDecoderCmdSupported();
-
   if (!decoder_thread_.Start()) {
     LOGF(ERROR) << "decoder thread failed to start";
     return false;
   }
 
-  decoder_state_ = kInitialized;
   output_mode_ = config.output_mode;
 
-  // StartDevicePoll will NOTIFY_ERROR on failure, so IgnoreResult is fine here.
+  // InitializeTask will NOTIFY_ERROR on failure.
   decoder_thread_.task_runner()->PostTask(
-      FROM_HERE, base::Bind(base::IgnoreResult(
-                                &V4L2VideoDecodeAccelerator::StartDevicePoll),
+      FROM_HERE, base::Bind(&V4L2VideoDecodeAccelerator::InitializeTask,
                             base::Unretained(this)));
 
   return true;
 }
 
+void V4L2VideoDecodeAccelerator::InitializeTask() {
+  DVLOGF(3);
+  DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());

[kcwu, 2016/11/28 06:31:14]

DCHECK_EQ(decoder_state_, kUninitialized);

[wuchengli, 2016/11/28 09:03:19]

Done.

+
+  // Subscribe to the resolution change event.
+  struct v4l2_event_subscription sub;
+  memset(&sub, 0, sizeof(sub));
+  sub.type = V4L2_EVENT_SOURCE_CHANGE;
+  IOCTL_OR_ERROR_RETURN(VIDIOC_SUBSCRIBE_EVENT, &sub);
+
+  if (!CreateInputBuffers()) {
+    NOTIFY_ERROR(PLATFORM_FAILURE);
+    return;
+  }
+
+  decoder_cmd_supported_ = IsDecoderCmdSupported();
+
+  if (!StartDevicePoll())
+    return;
+
+  decoder_state_ = kInitialized;
+}
+
 void V4L2VideoDecodeAccelerator::Decode(
     const BitstreamBuffer& bitstream_buffer) {
   DVLOGF(1) << "input_id=" << bitstream_buffer.id()
             << ", size=" << bitstream_buffer.size();
   DCHECK(decode_task_runner_->BelongsToCurrentThread());
 
   if (bitstream_buffer.id() < 0) {
     LOGF(ERROR) << "Invalid bitstream_buffer, id: " << bitstream_buffer.id();
     if (base::SharedMemory::IsHandleValid(bitstream_buffer.handle()))
       base::SharedMemory::CloseHandle(bitstream_buffer.handle());
@@ skipping 1803 matching lines @@
     DLOGF(ERROR) << "Unexpected visible rectangle " << rect.ToString()
                  << ", top-left is not origin";
     return coded_size;
   }
 
   return rect.size();
 }
 
 bool V4L2VideoDecodeAccelerator::CreateInputBuffers() {
   DVLOGF(3);
+  DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
   // 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;
   reqbufs.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
   reqbufs.memory = V4L2_MEMORY_MMAP;
@@ skipping 482 matching lines @@
       StartResolutionChange();
   }
 }
 
 void V4L2VideoDecodeAccelerator::ImageProcessorError() {
   LOGF(ERROR) << "Image processor error";
   NOTIFY_ERROR(PLATFORM_FAILURE);
 }
 
 }  // namespace media