Eliminate video capture thread in renderer

content/renderer/media/video_capture_impl_manager.cc

Index: content/renderer/media/video_capture_impl_manager.cc
diff --git a/content/renderer/media/video_capture_impl_manager.cc b/content/renderer/media/video_capture_impl_manager.cc
index 672fbd403c3374b0ed97a645119027e29808e7a9..099c4bab83d326ad7a5934c0b052165e97f61893 100644
--- a/content/renderer/media/video_capture_impl_manager.cc
+++ b/content/renderer/media/video_capture_impl_manager.cc
@@ -5,84 +5,96 @@
 #include "content/renderer/media/video_capture_impl_manager.h"
 
 #include "base/bind.h"
-#include "base/stl_util.h"
+#include "base/bind_helpers.h"
+#include "content/public/renderer/render_thread.h"
 #include "content/renderer/media/video_capture_impl.h"
 #include "content/renderer/media/video_capture_message_filter.h"
+#include "media/base/bind_to_loop.h"
 
 namespace content {
 
-VideoCaptureImplManager::VideoCaptureImplManager()
-    : thread_("VC manager") {
-  thread_.Start();
-  message_loop_proxy_ = thread_.message_loop_proxy();
-  filter_ = new VideoCaptureMessageFilter();
+VideoCaptureHandle::VideoCaptureHandle(
+    media::VideoCapture* impl, base::Closure destruction_cb)
+    : impl_(impl), destruction_cb_(destruction_cb) {
 }
 
-media::VideoCapture* VideoCaptureImplManager::AddDevice(
-    media::VideoCaptureSessionId id,
-    media::VideoCapture::EventHandler* handler) {
-  DCHECK(handler);
+VideoCaptureHandle::~VideoCaptureHandle() {
+  destruction_cb_.Run();
+}
 
-  base::AutoLock auto_lock(lock_);
-  Devices::iterator it = devices_.find(id);
-  if (it == devices_.end()) {
-    VideoCaptureImpl* vc =
-        new VideoCaptureImpl(id, message_loop_proxy_.get(), filter_.get());
-    devices_[id] = new Device(vc, handler);
-    vc->Init();
-    return vc;
-  }
+void VideoCaptureHandle::StartCapture(
+    EventHandler* handler,
+    const media::VideoCaptureParams& params) {
+  impl_->StartCapture(handler, params);
+}
 
-  devices_[id]->clients.push_front(handler);
-  return it->second->vc;
+void VideoCaptureHandle::StopCapture(EventHandler* handler) {
+  impl_->StopCapture(handler);
 }
 
-void VideoCaptureImplManager::SuspendDevices(bool suspend) {
-  base::AutoLock auto_lock(lock_);
-  for (Devices::iterator it = devices_.begin(); it != devices_.end(); ++it)
-    it->second->vc->SuspendCapture(suspend);
+bool VideoCaptureHandle::CaptureStarted() {
+  return impl_->CaptureStarted();
 }
 
-void VideoCaptureImplManager::RemoveDevice(
-    media::VideoCaptureSessionId id,
-    media::VideoCapture::EventHandler* handler) {
-  DCHECK(handler);
+int VideoCaptureHandle::CaptureFrameRate() {
+  return impl_->CaptureFrameRate();
+}
 
-  base::AutoLock auto_lock(lock_);
-  Devices::iterator it = devices_.find(id);
-  if (it == devices_.end())
-    return;
+VideoCaptureImplManager::VideoCaptureImplManager()
+    : filter_(new VideoCaptureMessageFilter()),
+      weak_factory_(this) {
+}
 
-  size_t size = it->second->clients.size();
-  it->second->clients.remove(handler);
+VideoCaptureImplManager::~VideoCaptureImplManager() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
 
-  if (size == it->second->clients.size() || size > 1)
-    return;
+scoped_ptr<VideoCaptureHandle> VideoCaptureImplManager::UseDevice(
+    media::VideoCaptureSessionId id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
 
-  devices_[id]->vc->DeInit(base::Bind(&VideoCaptureImplManager::FreeDevice,
-                                      this, devices_[id]->vc));
-  delete devices_[id];
-  devices_.erase(id);
-}
+  VideoCaptureImpl* video_capture_device = NULL;
+  VideoCaptureDeviceMap::iterator it = devices_.find(id);
+  if (it == devices_.end()) {
+    video_capture_device = new VideoCaptureImpl(id, filter_.get());
+    devices_[id] =
+        std::make_pair(1, linked_ptr<VideoCaptureImpl>(video_capture_device));
+    video_capture_device->Init();
+  } else {
+    ++it->second.first;
+    video_capture_device = it->second.second.get();
+  }
 
-void VideoCaptureImplManager::FreeDevice(VideoCaptureImpl* vc) {
-  delete vc;
+  // This callback ensures UnrefDevice() happens on the render thread.
+  return scoped_ptr<VideoCaptureHandle>(
+      new VideoCaptureHandle(
+          video_capture_device,
+          media::BindToCurrentLoop(
+              base::Bind(
+                  &VideoCaptureImplManager::UnrefDevice,
+                  weak_factory_.GetWeakPtr(),
+                  id))));
 }
 
-VideoCaptureImplManager::~VideoCaptureImplManager() {
-  thread_.Stop();
-  // TODO(wjia): uncomment the line below after collecting enough info for
-  // crbug.com/152418.
-  // STLDeleteContainerPairSecondPointers(devices_.begin(), devices_.end());
+void VideoCaptureImplManager::UnrefDevice(
+    media::VideoCaptureSessionId id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  VideoCaptureDeviceMap::iterator it = devices_.find(id);
+  DCHECK(it != devices_.end());
+
+  DCHECK(it->second.first);
+  if (--it->second.first) {

[Ami GONE FROM CHROMIUM, 2014/01/08 01:43:58]

is this test backwards?

[Alpha Left Google, 2014/01/08 22:30:50]

Good catch. I've separated it into two lines (just like before) to be clear.

+    VideoCaptureImpl* impl = it->second.second.release();
+    devices_.erase(id);
+    impl->DeInit(base::Bind(&base::DeletePointer<VideoCaptureImpl>, impl));

[Ami GONE FROM CHROMIUM, 2014/01/08 01:43:58]

Why that trailing ",impl"??

[Alpha Left Google, 2014/01/08 22:30:50]

I see what you mean now. But I would need a method to receive this scoped_ptr,
e.g.:

void DisposeVideoCaptureImpl(scoped_ptr<VideoCaptureImpl>);
impl->DeInit(base::Bind(&DisposeVideoCaptureImpl, base::Owned(impl)));

Using DeletePointer seems cleaner to me.

+  }
 }
 
-VideoCaptureImplManager::Device::Device(
-    VideoCaptureImpl* device,
-    media::VideoCapture::EventHandler* handler)
-    : vc(device) {
-  clients.push_front(handler);
+void VideoCaptureImplManager::SuspendDevices(bool suspend) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  for (VideoCaptureDeviceMap::iterator it = devices_.begin();
+       it != devices_.end(); ++it)
+    it->second.second->SuspendCapture(suspend);
 }
 
-VideoCaptureImplManager::Device::~Device() {}
-
 }  // namespace content