[Mojo Video Capture] Decouple VCController from VCBufferPool and VCDeviceClient

content/browser/renderer_host/media/video_capture_manager.cc

Index: content/browser/renderer_host/media/video_capture_manager.cc
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index e1830eed9fa09a7704246e7d7ac534ed97e5d574..2532acf786544f34f279cf235bc00394b874f218 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -26,12 +26,17 @@
 #include "content/browser/media/media_internals.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
+#include "content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h"
+#include "content/browser/renderer_host/media/video_frame_receiver_on_io_thread.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/desktop_media_id.h"
 #include "content/public/common/media_stream_request.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
+#include "media/capture/video/video_capture_buffer_pool_impl.h"
+#include "media/capture/video/video_capture_buffer_tracker_factory_impl.h"
 #include "media/capture/video/video_capture_device.h"
+#include "media/capture/video/video_capture_device_client.h"
 #include "media/capture/video/video_capture_device_factory.h"
 
 #if defined(ENABLE_SCREEN_CAPTURE) && !defined(OS_ANDROID)
@@ -113,8 +118,11 @@ void ConsolidateCaptureFormats(media::VideoCaptureFormats* formats) {
   }
 }
 
-// The maximum number of buffers in the capture pipeline. See
-// VideoCaptureController ctor comments for more details.
+// The maximum number of video frame buffers in-flight at any one time. This
+// value should be based on the logical capacity of the capture pipeline, and
+// not on hardware performance.  For example, tab capture requires more buffers
+// than webcam capture because the pipeline is longer (it includes read-backs
+// pending in the GPU pipeline).
 const int kMaxNumberOfBuffers = 3;
 // TODO(miu): The value for tab capture should be determined programmatically.
 // http://crbug.com/460318
@@ -143,6 +151,11 @@ static int g_device_start_id = 0;
 
 const media::VideoCaptureSessionId kFakeSessionId = -1;
 
+std::unique_ptr<media::VideoCaptureJpegDecoder> CreateGpuJpegDecoder(
+    const media::VideoCaptureJpegDecoder::DecodeDoneCB& decode_done_cb) {
+  return base::MakeUnique<content::VideoCaptureGpuJpegDecoder>(decode_done_cb);
+}
+
 }  // namespace
 
 namespace content {
@@ -153,36 +166,28 @@ namespace content {
 // is no device present.
 // Phase 2: When a request to "start" the entry comes in (via
 // HandleQueuedStartRequest()), a VideoCaptureDevice::Client is created
-// via video_capture_controller()->NewDeviceClient() and is used to schedule the
+// via CreateDeviceClient() and is used to schedule the
 // creation and start of a VideoCaptureDevice on the Device Thread.
 // Phase 3: As soon as the creation of the VideoCaptureDevice is complete, this
 // newly created VideoCaptureDevice instance is connected to the
 // VideoCaptureController via SetConsumerFeedbackObserver().
-class VideoCaptureManager::DeviceEntry {
+struct VideoCaptureManager::DeviceEntry {
  public:
   DeviceEntry(MediaStreamType stream_type,
               const std::string& id,
               std::unique_ptr<VideoCaptureController> controller,
               const media::VideoCaptureParams& params);
   ~DeviceEntry();
+  std::unique_ptr<media::VideoCaptureDevice::Client> CreateDeviceClient();
+  std::unique_ptr<media::FrameBufferHost> CreateFrameBufferHost();
 
   const int serial_id;
   const MediaStreamType stream_type;
   const std::string id;
   const media::VideoCaptureParams parameters;
-
-  VideoCaptureController* video_capture_controller() const;
-  media::VideoCaptureDevice* video_capture_device() const;
-
-  void SetVideoCaptureDevice(std::unique_ptr<VideoCaptureDevice> device);
-  std::unique_ptr<VideoCaptureDevice> ReleaseVideoCaptureDevice();
-
- private:
-  const std::unique_ptr<VideoCaptureController> video_capture_controller_;
-
-  std::unique_ptr<VideoCaptureDevice> video_capture_device_;
-
-  base::ThreadChecker thread_checker_;
+  const std::unique_ptr<VideoCaptureController> video_capture_controller;
+  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool;
+  std::unique_ptr<media::VideoCaptureDevice> video_capture_device;
 };
 
 // Bundles a media::VideoCaptureDeviceDescriptor with corresponding supported
@@ -198,6 +203,28 @@ struct VideoCaptureManager::DeviceInfo {
   media::VideoCaptureFormats supported_formats;
 };
 
+class BufferPoolFrameBufferHost : public media::FrameBufferHost {
+ public:
+  BufferPoolFrameBufferHost(
+      scoped_refptr<media::VideoCaptureBufferPool> buffer_pool)
+      : buffer_pool_(std::move(buffer_pool)) {}
+
+  void SetBufferHold(int buffer_id) override {
+    buffer_pool_->HoldForConsumers(buffer_id, 1);
+  }
+
+  void ReleaseBufferHold(int buffer_id) override {
+    buffer_pool_->RelinquishConsumerHold(buffer_id, 1);
+  }
+
+  mojo::ScopedSharedBufferHandle GetHandleForTransit(int buffer_id) override {
+    return buffer_pool_->GetHandleForTransit(buffer_id);
+  }
+
+ private:
+  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_;

[mcasas, 2016/12/06 20:02:24]

media::FrameBufferHost (and BufferPoolFrameBufferHost) 
are not ref counted whereas media::VideoCaptureBufferPool
is.  This forces you to use naked pointers in VCC::BufferState,
is it because you envision the mojo-based interface to not
be ref-counted...?

[chfremer, 2016/12/06 21:07:41]

The reason is that I am trying to avoid shared ownership in 
favor of clear ownership rules. Since shared ownership is not 
needed here, I opted for one unique_ptr<> for the owner and 
raw pointers for non-owning users.

+};
+
 // Class used for queuing request for starting a device.
 class VideoCaptureManager::CaptureDeviceStartRequest {
  public:
@@ -231,38 +258,41 @@ VideoCaptureManager::DeviceEntry::DeviceEntry(
       stream_type(stream_type),
       id(id),
       parameters(params),
-      video_capture_controller_(std::move(controller)) {}
+      video_capture_controller(std::move(controller)) {}
 
 VideoCaptureManager::DeviceEntry::~DeviceEntry() {
-  DCHECK(thread_checker_.CalledOnValidThread());
   // DCHECK that this DeviceEntry does not still own a
   // media::VideoCaptureDevice. media::VideoCaptureDevice must be deleted on
   // the device thread.
-  DCHECK(video_capture_device_ == nullptr);
-}
-
-void VideoCaptureManager::DeviceEntry::SetVideoCaptureDevice(
-    std::unique_ptr<VideoCaptureDevice> device) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  video_capture_device_.swap(device);
+  DCHECK(video_capture_device == nullptr);
 }
 
-std::unique_ptr<media::VideoCaptureDevice>
-VideoCaptureManager::DeviceEntry::ReleaseVideoCaptureDevice() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return std::move(video_capture_device_);
-}
+std::unique_ptr<media::VideoCaptureDevice::Client>
+VideoCaptureManager::DeviceEntry::CreateDeviceClient() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-VideoCaptureController*
-VideoCaptureManager::DeviceEntry::video_capture_controller() const {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return video_capture_controller_.get();
+  const int max_buffers = stream_type == MEDIA_TAB_VIDEO_CAPTURE
+                              ? kMaxNumberOfBuffersForTabCapture
+                              : kMaxNumberOfBuffers;
+  buffer_pool = new media::VideoCaptureBufferPoolImpl(
+      base::MakeUnique<media::VideoCaptureBufferTrackerFactoryImpl>(),
+      max_buffers);
+
+  return base::MakeUnique<media::VideoCaptureDeviceClient>(
+      base::MakeUnique<VideoFrameReceiverOnIOThread>(
+          video_capture_controller->GetWeakPtrForIOThread()),
+      buffer_pool,
+      base::Bind(
+          &CreateGpuJpegDecoder,
+          base::Bind(&media::VideoFrameReceiver::OnIncomingCapturedVideoFrame,
+                     video_capture_controller->GetWeakPtrForIOThread())));
 }
 
-media::VideoCaptureDevice*
-VideoCaptureManager::DeviceEntry::video_capture_device() const {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return video_capture_device_.get();
+std::unique_ptr<media::FrameBufferHost>
+VideoCaptureManager::DeviceEntry::CreateFrameBufferHost() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(buffer_pool);
+  return base::MakeUnique<BufferPoolFrameBufferHost>(buffer_pool);
 }
 
 VideoCaptureManager::DeviceInfo::DeviceInfo() = default;
@@ -384,8 +414,7 @@ void VideoCaptureManager::Close(int capture_session_id) {
   if (existing_device) {
     // Remove any client that is still using the session. This is safe to call
     // even if there are no clients using the session.
-    existing_device->video_capture_controller()
-        ->StopSession(capture_session_id);
+    existing_device->video_capture_controller->StopSession(capture_session_id);
 
     // StopSession() may have removed the last client, so we might need to
     // close the device.
@@ -433,17 +462,18 @@ void VideoCaptureManager::DoStopDevice(DeviceEntry* entry) {
 
   DVLOG(3) << "DoStopDevice. Send stop request for device = " << entry->id
            << " serial_id = " << entry->serial_id << ".";
-  entry->video_capture_controller()->OnLog(
+  entry->video_capture_controller->OnLog(
       base::StringPrintf("Stopping device: id: %s", entry->id.c_str()));
-  entry->video_capture_controller()->SetConsumerFeedbackObserver(nullptr);
+  entry->video_capture_controller->SetConsumerFeedbackObserver(nullptr);
+  entry->video_capture_controller->SetFrameBufferHost(nullptr);
 
   // |entry->video_capture_device| can be null if creating the device has
   // failed.
-  if (entry->video_capture_device()) {
+  if (entry->video_capture_device) {
     device_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread, this,
-                   base::Passed(entry->ReleaseVideoCaptureDevice())));
+                   base::Passed(&entry->video_capture_device)));

[mcasas, 2016/12/06 20:02:24]

std::move(entry->video_capture_device())

[chfremer, 2016/12/06 21:07:41]

Hmm, I don't fully understand the documentation of base::Bind, but when trying
to use std::move() here, I get the following compile error:

../../base/bind_internal.h:214:31: error: call to deleted constructor of
'std::unique_ptr<media::VideoCaptureDevice,
std::default_delete<media::VideoCaptureDevice> >'
    return (receiver.*method)(std::forward<RunArgs>(args)...);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~

   }
 }
 
@@ -465,6 +495,11 @@ void VideoCaptureManager::HandleQueuedStartRequest() {
   DVLOG(3) << "HandleQueuedStartRequest, Post start to device thread, device = "
            << entry->id << " start id = " << entry->serial_id;
 
+  std::unique_ptr<media::VideoCaptureDevice::Client> device_client =
+      entry->CreateDeviceClient();
+  std::unique_ptr<media::FrameBufferHost> frame_buffer_host =
+      entry->CreateFrameBufferHost();
+
   base::Callback<std::unique_ptr<VideoCaptureDevice>(void)>
       start_capture_function;
 
@@ -475,16 +510,16 @@ void VideoCaptureManager::HandleQueuedStartRequest() {
       // held in the browser-side VideoCaptureDevice::Name structure.
       const DeviceInfo* found = GetDeviceInfoById(entry->id);
       if (found) {
-        entry->video_capture_controller()->OnLog(
+        entry->video_capture_controller->OnLog(
             base::StringPrintf("Starting device: id: %s, name: %s, api: %s",
                                found->descriptor.device_id.c_str(),
                                found->descriptor.GetNameAndModel().c_str(),
                                found->descriptor.GetCaptureApiTypeString()));
 
-        start_capture_function = base::Bind(
-            &VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread, this,
-            found->descriptor, request->params(),
-            base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+        start_capture_function =
+            base::Bind(&VideoCaptureManager::DoStartDeviceCaptureOnDeviceThread,
+                       this, found->descriptor, request->params(),
+                       base::Passed(std::move(device_client)));
       } else {
         // Errors from DoStartDeviceCaptureOnDeviceThread go via
         // VideoCaptureDeviceClient::OnError, which needs some thread
@@ -495,8 +530,8 @@ void VideoCaptureManager::HandleQueuedStartRequest() {
             "Error on %s:%d: device %s unknown. Maybe recently disconnected?",
             __FILE__, __LINE__, entry->id.c_str());
         DLOG(ERROR) << log_message;
-        entry->video_capture_controller()->OnLog(log_message);
-        entry->video_capture_controller()->OnError();
+        entry->video_capture_controller->OnLog(log_message);
+        entry->video_capture_controller->OnError();
         // Drop the failed start request.
         device_start_queue_.pop_front();
 
@@ -507,15 +542,13 @@ void VideoCaptureManager::HandleQueuedStartRequest() {
     case MEDIA_TAB_VIDEO_CAPTURE:
       start_capture_function = base::Bind(
           &VideoCaptureManager::DoStartTabCaptureOnDeviceThread, this,
-          entry->id, request->params(),
-          base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+          entry->id, request->params(), base::Passed(std::move(device_client)));
       break;
 
     case MEDIA_DESKTOP_VIDEO_CAPTURE:
       start_capture_function = base::Bind(
           &VideoCaptureManager::DoStartDesktopCaptureOnDeviceThread, this,
-          entry->id, request->params(),
-          base::Passed(entry->video_capture_controller()->NewDeviceClient()));
+          entry->id, request->params(), base::Passed(std::move(device_client)));
       break;
 
     default: {
@@ -523,14 +556,17 @@ void VideoCaptureManager::HandleQueuedStartRequest() {
       return;
     }
   }
+  // Passing raw pointer |device_client_ptr| is safe, because it will
+  // be owned by the device returned by |start_capture_function|.
   base::PostTaskAndReplyWithResult(
       device_task_runner_.get(), FROM_HERE, start_capture_function,
       base::Bind(&VideoCaptureManager::OnDeviceStarted, this,
-                 request->serial_id()));
+                 request->serial_id(), base::Passed(&frame_buffer_host)));
 }
 
 void VideoCaptureManager::OnDeviceStarted(
     int serial_id,
+    std::unique_ptr<media::FrameBufferHost> frame_buffer_host,
     std::unique_ptr<VideoCaptureDevice> device) {
   DVLOG(3) << __func__;
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -551,15 +587,19 @@ void VideoCaptureManager::OnDeviceStarted(
   } else {
     DeviceEntry* const entry = GetDeviceEntryBySerialId(serial_id);
     DCHECK(entry);
-    DCHECK(!entry->video_capture_device());
-    // Passing raw pointer |device.get()| to the controller is safe,
-    // because we transfer ownership of it to |entry|. We are calling
-    // SetConsumerFeedbackObserver(nullptr) before releasing
-    // |entry->video_capture_device_| on the |device_task_runner_|.
-    entry->video_capture_controller()->SetConsumerFeedbackObserver(
-        base::MakeUnique<VideoFrameConsumerFeedbackObserverOnTaskRunner>(
-            device.get(), device_task_runner_));
-    entry->SetVideoCaptureDevice(std::move(device));
+    DCHECK(!entry->video_capture_device);
+    if (device) {
+      entry->video_capture_controller->SetFrameBufferHost(
+          std::move(frame_buffer_host));
+      // Passing raw pointer |device.get()| to the controller is safe,
+      // because we transfer ownership of it to |entry|. We are calling
+      // SetConsumerFeedbackObserver(nullptr) before releasing
+      // |entry->video_capture_device_| on the |device_task_runner_|.
+      entry->video_capture_controller->SetConsumerFeedbackObserver(
+          base::MakeUnique<VideoFrameConsumerFeedbackObserverOnTaskRunner>(
+              device.get(), device_task_runner_));
+    }
+    entry->video_capture_device = std::move(device);
 
     if (entry->stream_type == MEDIA_DESKTOP_VIDEO_CAPTURE) {
       const media::VideoCaptureSessionId session_id =
@@ -572,8 +612,8 @@ void VideoCaptureManager::OnDeviceStarted(
     while (it != photo_request_queue_.end()) {
       auto request = it++;
       DeviceEntry* maybe_entry = GetDeviceEntryBySessionId(request->first);
-      if (maybe_entry && maybe_entry->video_capture_device()) {
-        request->second.Run(maybe_entry->video_capture_device());
+      if (maybe_entry && maybe_entry->video_capture_device) {
+        request->second.Run(maybe_entry->video_capture_device.get());
         photo_request_queue_.erase(request);
       }
     }
@@ -686,21 +726,21 @@ void VideoCaptureManager::StartCaptureForClient(
     return;
   }
 
-  DCHECK(entry->video_capture_controller());
+  DCHECK(entry->video_capture_controller);
 
   LogVideoCaptureEvent(VIDEO_CAPTURE_START_CAPTURE);
 
   // First client starts the device.
-  if (!entry->video_capture_controller()->HasActiveClient() &&
-      !entry->video_capture_controller()->HasPausedClient()) {
+  if (!entry->video_capture_controller->HasActiveClient() &&
+      !entry->video_capture_controller->HasPausedClient()) {
     DVLOG(1) << "VideoCaptureManager starting device (type = "
              << entry->stream_type << ", id = " << entry->id << ")";
     QueueStartDevice(session_id, entry, params);
   }
   // Run the callback first, as AddClient() may trigger OnFrameInfo().
-  done_cb.Run(entry->video_capture_controller()->GetWeakPtrForIOThread());
-  entry->video_capture_controller()->AddClient(
-      client_id, client_handler, session_id, params);
+  done_cb.Run(entry->video_capture_controller->GetWeakPtrForIOThread());
+  entry->video_capture_controller->AddClient(client_id, client_handler,
+                                             session_id, params);
 }
 
 void VideoCaptureManager::StopCaptureForClient(
@@ -765,7 +805,7 @@ void VideoCaptureManager::PauseCaptureForClient(
   controller->PauseClient(client_id, client_handler);
   if (!had_active_client || controller->HasActiveClient())
     return;
-  if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
+  if (media::VideoCaptureDevice* device = entry->video_capture_device.get()) {
     device_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&VideoCaptureDevice::MaybeSuspend,
@@ -795,7 +835,7 @@ void VideoCaptureManager::ResumeCaptureForClient(
   controller->ResumeClient(client_id, client_handler);
   if (had_active_client || !controller->HasActiveClient())
     return;
-  if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
+  if (media::VideoCaptureDevice* device = entry->video_capture_device.get()) {
     device_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&VideoCaptureDevice::Resume,
@@ -812,7 +852,7 @@ void VideoCaptureManager::RequestRefreshFrameForClient(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (DeviceEntry* entry = GetDeviceEntryByController(controller)) {
-    if (media::VideoCaptureDevice* device = entry->video_capture_device()) {
+    if (media::VideoCaptureDevice* device = entry->video_capture_device.get()) {
       device_task_runner_->PostTask(
           FROM_HERE,
           base::Bind(&VideoCaptureDevice::RequestRefreshFrame,
@@ -860,7 +900,7 @@ bool VideoCaptureManager::GetDeviceFormatsInUse(
   if (device_in_use) {
     // Currently only one format-in-use is supported at the VCC level.
     formats_in_use->push_back(
-        device_in_use->video_capture_controller()->GetVideoCaptureFormat());
+        device_in_use->video_capture_controller->GetVideoCaptureFormat());
   }
   return true;
 }
@@ -888,7 +928,7 @@ void VideoCaptureManager::MaybePostDesktopCaptureWindowId(
     return;
   }
 
-  if (!existing_device->video_capture_device()) {
+  if (!existing_device->video_capture_device) {
     DVLOG(2) << "Screen capture device not yet started.";
     return;
   }
@@ -910,8 +950,7 @@ void VideoCaptureManager::MaybePostDesktopCaptureWindowId(
   device_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&VideoCaptureManager::SetDesktopCaptureWindowIdOnDeviceThread,
-                 this,
-                 existing_device->video_capture_device(),
+                 this, existing_device->video_capture_device.get(),
                  window_id_it->second));
 
   notification_window_ids_.erase(window_id_it);
@@ -925,7 +964,7 @@ void VideoCaptureManager::GetPhotoCapabilities(
   const DeviceEntry* entry = GetDeviceEntryBySessionId(session_id);
   if (!entry)
     return;
-  VideoCaptureDevice* device = entry->video_capture_device();
+  VideoCaptureDevice* device = entry->video_capture_device.get();
   if (device) {
     VideoCaptureManager::DoGetPhotoCapabilities(std::move(callback), device);
     return;
@@ -945,7 +984,7 @@ void VideoCaptureManager::SetPhotoOptions(
   const DeviceEntry* entry = GetDeviceEntryBySessionId(session_id);
   if (!entry)
     return;
-  VideoCaptureDevice* device = entry->video_capture_device();
+  VideoCaptureDevice* device = entry->video_capture_device.get();
   if (device) {
     VideoCaptureManager::DoSetPhotoOptions(std::move(callback),
                                            std::move(settings), device);
@@ -965,7 +1004,7 @@ void VideoCaptureManager::TakePhoto(
   const DeviceEntry* entry = GetDeviceEntryBySessionId(session_id);
   if (!entry)
     return;
-  VideoCaptureDevice* device = entry->video_capture_device();
+  VideoCaptureDevice* device = entry->video_capture_device.get();
   if (device) {
     VideoCaptureManager::DoTakePhoto(std::move(callback), device);
     return;
@@ -1073,8 +1112,8 @@ void VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread(
 void VideoCaptureManager::DestroyDeviceEntryIfNoClients(DeviceEntry* entry) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // Removal of the last client stops the device.
-  if (!entry->video_capture_controller()->HasActiveClient() &&
-      !entry->video_capture_controller()->HasPausedClient()) {
+  if (!entry->video_capture_controller->HasActiveClient() &&
+      !entry->video_capture_controller->HasPausedClient()) {
     DVLOG(1) << "VideoCaptureManager stopping device (type = "
              << entry->stream_type << ", id = " << entry->id << ")";
 
@@ -1124,7 +1163,7 @@ VideoCaptureManager::GetDeviceEntryByController(
 
   // Look up |controller| in |devices_|.
   for (const std::unique_ptr<DeviceEntry>& device : devices_) {
-    if (device->video_capture_controller() == controller)
+    if (device->video_capture_controller.get() == controller)
       return device.get();
   }
   return nullptr;
@@ -1169,15 +1208,12 @@ VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
     return existing_device;
   }
 
-  const int max_buffers = device_info.type == MEDIA_TAB_VIDEO_CAPTURE ?
-      kMaxNumberOfBuffersForTabCapture : kMaxNumberOfBuffers;
-  std::unique_ptr<VideoCaptureController> video_capture_controller(
-      new VideoCaptureController(max_buffers));
-  DeviceEntry* new_device =
+  auto video_capture_controller = base::MakeUnique<VideoCaptureController>();
+  auto new_device_entry =
       new DeviceEntry(device_info.type, device_info.id,
                       std::move(video_capture_controller), params);
-  devices_.emplace_back(new_device);
-  return new_device;
+  devices_.emplace_back(new_device_entry);
+  return new_device_entry;
 }
 
 void VideoCaptureManager::SetDesktopCaptureWindowIdOnDeviceThread(
@@ -1263,7 +1299,7 @@ void VideoCaptureManager::ResumeDevices() {
     // Do not resume Content Video Capture devices, e.g. Tab or Screen capture.
     // Do not try to restart already running devices.
     if (entry->stream_type != MEDIA_DEVICE_VIDEO_CAPTURE ||
-        entry->video_capture_device())
+        entry->video_capture_device)
       continue;
 
     // Check if the device is already in the start queue.