Break tight coupling of VideoCaptureDeviceClient to renderer_host

content/browser/renderer_host/media/video_capture_controller.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/media/video_capture_controller.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/stl_util.h"
 #include "build/build_config.h"
 #include "components/display_compositor/gl_helper.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/renderer_host/media/video_capture_buffer_pool.h"
 #include "content/browser/renderer_host/media/video_capture_device_client.h"
+#include "content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "media/base/video_frame.h"
 
 #if !defined(OS_ANDROID)
 #include "content/browser/compositor/image_transport_factory.h"
 #endif
 
 using media::VideoCaptureFormat;
@@ skipping 38 matching lines @@
   // UpdateReleaseSyncToken() creates a new sync_token using |gl_helper|, so
   // wait the given |sync_token| using |gl_helper|.
   if (gl_helper) {
     gl_helper->WaitSyncToken(sync_token);
     SyncTokenClientImpl client(gl_helper);
     video_frame->UpdateReleaseSyncToken(&client);
   }
 #endif
 }
 
+class VideoCaptureGpuJpegDecoderFactory
+    : public VideoCaptureJpegDecoderFactory {
+ public:
+  VideoCaptureGpuJpegDecoderFactory(
+      const VideoCaptureJpegDecoder::DecodeDoneCB& decode_done_cb)
+      : decode_done_cb_(decode_done_cb) {}
+
+  std::unique_ptr<VideoCaptureJpegDecoder> CreateJpegDecoder() override {
+    return base::MakeUnique<VideoCaptureGpuJpegDecoder>(decode_done_cb_);
+  }
+
+ private:
+  VideoCaptureJpegDecoder::DecodeDoneCB decode_done_cb_;
+};
+
+// Decorator for VideoFrameReceiver that forwards all incoming calls
+// to the Browser IO thread.
+class PostOnBrowserIoThreadDecorator : public VideoFrameReceiver {

[emircan, 2016/09/08 19:57:56]

VideoFrameReceiverOnIOThread?

[mcasas, 2016/09/08 20:25:45]

IOThread is written with IO in uppercase.

(grep -rn "IoThread" content/ | wc -l  says 0)

[chfremer, 2016/09/09 00:55:38]

Thanks for both of the suggestions.
I hope you are okay with "PostOnIOThreadVideoFrameReceiver", since I try to be
consistent in putting the specialization aspect of a name in front of the thing
it specializes.

+ public:
+  PostOnBrowserIoThreadDecorator(
+      const base::WeakPtr<VideoFrameReceiver>& receiver)
+      : receiver_(receiver) {}
+
+  void OnIncomingCapturedVideoFrame(
+      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
+      const scoped_refptr<media::VideoFrame>& frame) override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&VideoFrameReceiver::OnIncomingCapturedVideoFrame, receiver_,
+                   base::Passed(&buffer), frame));
+  }
+
+  void OnError() override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&VideoFrameReceiver::OnError, receiver_));
+  }
+
+  void OnLog(const std::string& message) override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&VideoFrameReceiver::OnLog, receiver_, message));
+  }
+
+  void OnBufferDestroyed(int buffer_id_to_drop) override {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&VideoFrameReceiver::OnBufferDestroyed,
+                                       receiver_, buffer_id_to_drop));
+  }
+
+ private:
+  base::WeakPtr<VideoFrameReceiver> receiver_;
+};
+
 }  // anonymous namespace
 
 struct VideoCaptureController::ControllerClient {
   ControllerClient(VideoCaptureControllerID id,
                    VideoCaptureControllerEventHandler* handler,
                    base::ProcessHandle render_process,
                    media::VideoCaptureSessionId session_id,
                    const media::VideoCaptureParams& params)
       : controller_id(id),
         event_handler(handler),
@@ skipping 49 matching lines @@
 
 base::WeakPtr<VideoCaptureController>
 VideoCaptureController::GetWeakPtrForIOThread() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
 std::unique_ptr<media::VideoCaptureDevice::Client>
 VideoCaptureController::NewDeviceClient() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return base::MakeUnique<VideoCaptureDeviceClient>(
-      this->GetWeakPtrForIOThread(), buffer_pool_);
+      base::MakeUnique<PostOnBrowserIoThreadDecorator>(
+          this->GetWeakPtrForIOThread()),
+      buffer_pool_,
+      base::MakeUnique<VideoCaptureGpuJpegDecoderFactory>(
+          base::Bind(&VideoFrameReceiver::OnIncomingCapturedVideoFrame,
+                     this->GetWeakPtrForIOThread())),

[emircan, 2016/09/08 19:57:56]

Passing a callback would be easier to follow here than passing the factory
class. I would rather avoid another layer of class hierarchy for readability
here.

[chfremer, 2016/09/09 00:55:38]

Interesting. I feel the exact opposite way about the readability.
With mcasas@ already having expressed the same feeling as you, I'll go ahead and
make the change :-).

+      base::Bind([]() { DCHECK_CURRENTLY_ON(BrowserThread::IO); }));
 }
 
 void VideoCaptureController::AddClient(
     VideoCaptureControllerID id,
     VideoCaptureControllerEventHandler* event_handler,
     base::ProcessHandle render_process,
     media::VideoCaptureSessionId session_id,
     const media::VideoCaptureParams& params) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(1) << "VideoCaptureController::AddClient() -- id=" << id
@@ skipping 185 matching lines @@
 VideoCaptureController::GetVideoCaptureFormat() const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return video_capture_format_;
 }
 
 VideoCaptureController::~VideoCaptureController() {
   base::STLDeleteContainerPointers(controller_clients_.begin(),
                                    controller_clients_.end());
 }
 
-void VideoCaptureController::DoIncomingCapturedVideoFrameOnIOThread(
+void VideoCaptureController::OnIncomingCapturedVideoFrame(
     std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> buffer,
     const scoped_refptr<VideoFrame>& frame) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   const int buffer_id = buffer->id();
   DCHECK_NE(buffer_id, VideoCaptureBufferPool::kInvalidId);
 
   int count = 0;
   if (state_ == VIDEO_CAPTURE_STATE_STARTED) {
     if (!frame->metadata()->HasKey(VideoFrameMetadata::FRAME_RATE)) {
       frame->metadata()->SetDouble(VideoFrameMetadata::FRAME_RATE,
@@ skipping 50 matching lines @@
                                       &frame_rate)) {
       frame_rate = video_capture_format_.frame_rate;
     }
     UMA_HISTOGRAM_COUNTS("Media.VideoCapture.FrameRate", frame_rate);
     has_received_frames_ = true;
   }
 
   buffer_pool_->HoldForConsumers(buffer_id, count);
 }
 
-void VideoCaptureController::DoErrorOnIOThread() {
+void VideoCaptureController::OnError() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   state_ = VIDEO_CAPTURE_STATE_ERROR;
 
   for (const auto* client : controller_clients_) {
     if (client->session_closed)
        continue;
     client->event_handler->OnError(client->controller_id);
   }
 }
 
-void VideoCaptureController::DoLogOnIOThread(const std::string& message) {
+void VideoCaptureController::OnLog(const std::string& message) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   MediaStreamManager::SendMessageToNativeLog("Video capture: " + message);
 }
 
-void VideoCaptureController::DoBufferDestroyedOnIOThread(
-    int buffer_id_to_drop) {
+void VideoCaptureController::OnBufferDestroyed(int buffer_id_to_drop) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   for (auto* client : controller_clients_) {
     if (client->session_closed)
       continue;
 
     if (client->known_buffers.erase(buffer_id_to_drop)) {
       client->event_handler->OnBufferDestroyed(client->controller_id,
                                                buffer_id_to_drop);
     }
@@ skipping 51 matching lines @@
     int session_id,
     const ControllerClients& clients) {
   for (auto* client : clients) {
     if (client->session_id == session_id)
       return client;
   }
   return NULL;
 }
 
 }  // namespace content