VideoCaptureManager opens/closes, starts/stops and enumerates video capture devices. VideoCapture...

content/browser/media_stream/video_capture_manager.cc

Index: content/browser/media_stream/video_capture_manager.cc
===================================================================
--- content/browser/media_stream/video_capture_manager.cc	(revision 0)
+++ content/browser/media_stream/video_capture_manager.cc	(revision 0)
@@ -0,0 +1,442 @@
+// Copyright (c) 2011 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/media_stream/video_capture_manager.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "content/browser/browser_thread.h"
+#include "media/video/capture/fake_video_capture_device.h"
+#include "media/video/capture/video_capture_device.h"
+
+namespace media_stream {
+
+// Starting id for the first capture session
+enum { kFirstSessionId = 2};
+
+static ::base::LazyInstance<VideoCaptureManager> g_video_capture_manager(
+    base::LINKER_INITIALIZED);
+
+bool VideoCaptureManager::use_fake_device_ = false;

[John Knottenbelt, 2011/05/11 11:36:35]

Question: Can you make use_fake_device_ non-static (i.e. an ordinary member)?

[mflodman1, 2011/05/15 20:24:00]

Done.

+
+VideoCaptureManager* VideoCaptureManager::Get() {
+  return g_video_capture_manager.Pointer();
+}
+
+void VideoCaptureManager::CreateTestManager() {

[John Knottenbelt, 2011/05/11 11:36:35]

Suggestion: Remove this method and make UseFakeDevice a public method, perhaps
called UseFakeDeviceForTests.

Test code could then use VideoCaptureManager::Get()->UseFakeDeviceForTests()

[mflodman1, 2011/05/15 20:24:00]

Done.

+  VideoCaptureManager* vcm = g_video_capture_manager.Pointer();
+  vcm->UseFakeDevice();
+}
+
+VideoCaptureManager::VideoCaptureManager()
+  : vc_device_thread_("VideoCaptureManagerThread"),
+    listener_(NULL),
+    new_capture_session_id_(kFirstSessionId),
+    devices_() {
+  vc_device_thread_.Start();
+}
+
+VideoCaptureManager::~VideoCaptureManager() {
+  vc_device_thread_.Stop();
+  devices_.clear();

[John Knottenbelt, 2011/05/11 11:36:35]

devices_ will be cleared by virtue of its destruction, so no need to call it
explicitly here.

The same goes for vc_device_thread.Stop(), but I think it is a good idea to call
it explicitly here because you don't want the thread to be running after the
destructor's body exits and the member fields are destroyed.

[mflodman1, 2011/05/15 20:24:00]

Done.

+}
+
+bool VideoCaptureManager::Register(MediaStreamType service_type,
+                                  MediaStreamProviderListener* listener) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (service_type != kVideoCapture) {

[John Knottenbelt, 2011/05/11 11:36:35]

Nit (here, and elsewhere): Single line body should remove { }

[mflodman1, 2011/05/15 20:24:00]

Earlier code review (for VideoCaptureDeviceLinux) gave feedback to use one of
the styles, but not mix styles in the same cc-file. After that we've used {} in
all our code for all if-statements, so I'll keep it that way if you don't
disagree and have a strong opinion about it.

+    // Not a valid service type
+    return false;
+  }
+  if (listener_) {

[John Knottenbelt, 2011/05/11 11:36:35]

Question (here, and elsewhere): Is it a logic error to Register when there is
already a listener? If so add a DCHECK:

DCHECK(!listener_);

Same question for the service_type checks.

[mflodman1, 2011/05/15 20:24:00]

Done.

+    // Already registered, unregister first.
+    return false;
+  }
+  listener_ = listener;
+  return true;
+}
+
+void VideoCaptureManager::Unregister(MediaStreamType service_type,
+                                     MediaStreamProviderListener* listener) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (service_type != kVideoCapture) {
+    return;
+  }
+  listener_ = NULL;
+  return;

[John Knottenbelt, 2011/05/11 11:36:35]

Nit (here and elsewhere): Unnecessary final return statement.

[mflodman1, 2011/05/15 20:24:00]

Done.

+}
+
+void VideoCaptureManager::EnumerateDevices(MediaStreamType service_type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    return;
+  }
+  if (service_type != kVideoCapture) {
+    PostOnError(-1, kInvalidMediaStreamType);

[John Knottenbelt, 2011/05/11 11:36:35]

Suggestion: Introduce a constant, e.g. kInvalidMediaCaptureSessionId instead of
using -1 directly.

[mflodman1, 2011/05/15 20:24:00]

servive_type removed, but invalid id added as return valur from Open().

+    return;
+  }
+
+  vc_device_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &VideoCaptureManager::OnEnumerateDevices));
+  return;
+}
+
+MediaCaptureSessionId VideoCaptureManager::Open(
+    MediaStreamType service_type, const MediaCaptureDeviceInfo& device) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    return -1;
+  }
+  if (service_type != kVideoCapture) {
+      PostOnError(-1, kInvalidMediaStreamType);
+      return -1;
+  }
+
+  // Generate a new id for this device
+  MediaCaptureSessionId video_capture_session_id = new_capture_session_id_++;
+
+  vc_device_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &VideoCaptureManager::OnOpen,
+                        video_capture_session_id,
+                        device));
+
+  return video_capture_session_id;
+}
+
+void VideoCaptureManager::Close(MediaStreamType service_type,
+                                MediaCaptureSessionId capture_session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    return;
+  }
+  if (service_type != kVideoCapture) {
+    PostOnError(-1, kInvalidMediaStreamType);
+    return;
+  }
+
+  vc_device_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &VideoCaptureManager::OnClose,
+                        capture_session_id));
+  return;
+}
+
+void VideoCaptureManager::Start(
+    const media::VideoCaptureParams& capture_params,
+    media::VideoCaptureDevice::EventHandler* video_capture_receiver) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  vc_device_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &VideoCaptureManager::OnStart,
+                        capture_params,
+                        video_capture_receiver));
+}
+
+void VideoCaptureManager::Stop(
+    const media::VideoCaptureSessionId capture_session_id, Task* stopped_task) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  vc_device_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableMethod(this,
+                        &VideoCaptureManager::OnStop,
+                        capture_session_id,
+                        stopped_task));
+}
+
+void VideoCaptureManager::OnEnumerateDevices() {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  scoped_ptr<media::VideoCaptureDevice::Names> device_names(
+      new media::VideoCaptureDevice::Names());
+  GetAvailableDevices(device_names.get());
+
+  MediaCaptureDevices devices;
+  for (media::VideoCaptureDevice::Names::iterator it =
+      device_names.get()->begin(); it != device_names.get()->end(); ++it) {
+    bool opened = DeviceOpened(*it);
+    devices.push_back(MediaCaptureDeviceInfo(kVideoCapture, it->device_name,
+                                             it->unique_id, opened));
+  }
+
+  // Let the listener know the result
+  PostOnDevicesEnumerated(devices);
+
+  // Clean-up
+  devices.clear();
+  device_names.get()->clear();
+
+  return;
+}
+
+void VideoCaptureManager::OnOpen(MediaCaptureSessionId capture_session_id,
+                                 const MediaCaptureDeviceInfo device) {
+  DCHECK(IsOnCaptureDeviceThread());
+  if (devices_.find(capture_session_id) != devices_.end()) {

[John Knottenbelt, 2011/05/11 11:36:35]

Question: Is it appropriate to add more DCHECKs into the error checking here?

[mflodman1, 2011/05/15 20:24:00]

Yes, changed.

+    // id already exists!
+    PostOnError(capture_session_id, kInvalidSession);
+    return;
+  }
+
+  // Check if another session has already opened this device, only one user per
+  // device is supported.
+  if (DeviceOpened(device) == true) {

[John Knottenbelt, 2011/05/11 11:36:35]

Nit: Remove == true

[mflodman1, 2011/05/15 20:24:00]

Done.

+    PostOnError(capture_session_id, kDeviceAlreadyInUse);
+    return;
+  }
+
+  // Open the device
+  media::VideoCaptureDevice::Name vc_device_name;
+  vc_device_name.device_name = device.name;
+  vc_device_name.unique_id = device.device_id;
+
+  media::VideoCaptureDevice* video_capture_device = NULL;
+  if (!use_fake_device_) {
+    video_capture_device = media::VideoCaptureDevice::Create(vc_device_name);
+  } else {
+    video_capture_device =
+        media::FakeVideoCaptureDevice::Create(vc_device_name);
+  }
+  if (video_capture_device == NULL) {
+    PostOnError(capture_session_id, kDeviceNotAvailable);
+    return;
+  }
+
+  devices_[capture_session_id] = video_capture_device;
+  PostOnOpened(capture_session_id);
+  return;
+}
+
+void VideoCaptureManager::OnClose(
+    MediaCaptureSessionId capture_session_id) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  VideoCaptureDevices::iterator it = devices_.find(capture_session_id);
+  if (it != devices_.end()) {
+    // Deallocate (if not done already) and delete the device
+    media::VideoCaptureDevice* video_capture_device = it->second;
+    video_capture_device->DeAllocate();
+    delete video_capture_device;
+    devices_.erase(it);
+  }
+
+  PostOnClosed(capture_session_id);
+  return;
+}
+
+void VideoCaptureManager::OnStart(
+    const media::VideoCaptureParams capture_params,
+    media::VideoCaptureDevice::EventHandler* video_capture_receiver) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  // Solution for not using MediaStreamManager
+  // This session id won't be returned by Open()
+  if (capture_params.session_id == kStartOpenSessionId) {

[John Knottenbelt, 2011/05/11 11:36:35]

Question: This seems like a convenience feature. If it was possible to move it
outside of the implementation of VideoCaptureManager, I think it make it easier
to check the correctness of this file.

Where is this convenience used?

[mflodman1, 2011/05/15 20:24:00]

It is used to be able to open capture devices in Chrome without using
MediaStreamManager. This will be used until MediaStreamManager is committed and
also depending on Pepper and their use case.

I see the point of the comment, but I think this is the easiest solution in
Chrome and moving this outside of this class will probably make the rest of the
code harder to check than this is. Moving this to another class, which would be
VideoCaptureHost, will also require changes to error checks and other functions
in this class.

+    // Start() is called without using Open(), we need to open a device
+    scoped_ptr<media::VideoCaptureDevice::Names> device_names(
+        new media::VideoCaptureDevice::Names());
+    GetAvailableDevices(device_names.get());
+
+    MediaCaptureDeviceInfo device(kVideoCapture,
+                                  device_names.get()->front().device_name,
+                                  device_names.get()->front().unique_id, false);
+
+    // Call OnOpen to open using the first device in the list
+    OnOpen(capture_params.session_id, device);
+  }
+
+  VideoCaptureDevices::iterator it = devices_.find(capture_params.session_id);
+  if (it == devices_.end()) {
+    // Invalid session id
+    video_capture_receiver->OnError();
+    return;
+  }
+  media::VideoCaptureDevice* video_capture_device = it->second;
+
+  // Possible errors are signaled to video_capture_receiver by
+  // video_capture_device. video_capture_receiver to perform actions.
+  video_capture_device->Allocate(capture_params.width, capture_params.height,
+                                 capture_params.frame_per_second,
+                                 video_capture_receiver);
+  video_capture_device->Start();
+  return;
+}
+
+void VideoCaptureManager::OnStop(
+    const media::VideoCaptureSessionId capture_session_id,
+    Task* stopped_task) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  VideoCaptureDevices::iterator it = devices_.find(capture_session_id);
+  if (it != devices_.end()) {
+    media::VideoCaptureDevice* video_capture_device = it->second;
+    // Possible errors are signaled to video_capture_receiver by
+    // video_capture_device. video_capture_receiver to perform actions.
+    video_capture_device->Stop();
+    video_capture_device->DeAllocate();
+  }
+
+  if (stopped_task) {
+    stopped_task->Run();
+    delete stopped_task;
+  }
+
+  if (capture_session_id == kStartOpenSessionId) {
+    // This device was opened from Start(), not Open(). Close it!
+    OnClose(capture_session_id);
+  }
+  return;
+}
+
+void VideoCaptureManager::OnOpened(
+    MediaCaptureSessionId capture_session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    // Listener has been removed
+    return;
+  }
+  listener_->Opened(kVideoCapture, capture_session_id);
+  return;
+}
+
+void VideoCaptureManager::OnClosed(
+    MediaCaptureSessionId capture_session_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    // Listener has been removed
+    return;
+  }
+  listener_->Closed(kVideoCapture, capture_session_id);
+  return;
+}
+
+void VideoCaptureManager::OnDevicesEnumerated(
+    const MediaCaptureDevices& devices) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    // Listener has been removed
+    return;
+  }
+  listener_->DevicesEnumerated(kVideoCapture, devices);
+  return;
+}
+
+void VideoCaptureManager::OnError(MediaCaptureSessionId capture_session_id,
+                                  MediaStreamProviderError error) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (listener_ == NULL) {
+    // Listener has been removed
+    return;
+  }
+  listener_->Error(kVideoCapture, capture_session_id, error);
+  return;
+}
+
+void VideoCaptureManager::PostOnOpened(
+    MediaCaptureSessionId capture_session_id) {
+  DCHECK(IsOnCaptureDeviceThread());
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          NewRunnableMethod(this,
+                                            &VideoCaptureManager::OnOpened,
+                                            capture_session_id));
+  return;
+}
+
+void VideoCaptureManager::PostOnClosed(
+    MediaCaptureSessionId capture_session_id) {
+  DCHECK(IsOnCaptureDeviceThread());
+  BrowserThread::PostTask(BrowserThread::IO,
+                            FROM_HERE,
+                            NewRunnableMethod(this,
+                                              &VideoCaptureManager::OnClosed,
+                                              capture_session_id));
+  return;
+}
+
+void VideoCaptureManager::PostOnDevicesEnumerated(MediaCaptureDevices devices) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          NewRunnableMethod(
+                              this,
+                              &VideoCaptureManager::OnDevicesEnumerated,
+                              devices));
+  return;
+}
+
+void VideoCaptureManager::PostOnError(MediaCaptureSessionId capture_session_id,
+                                      MediaStreamProviderError error) {
+  // Don't check thread here, can be called from both IO thread and device
+  // thread.
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          NewRunnableMethod(this,
+                                            &VideoCaptureManager::OnError,
+                                            capture_session_id,
+                                              error));

[John Knottenbelt, 2011/05/11 11:36:35]

Nit: indentation

[mflodman1, 2011/05/15 20:24:00]

Done.

+  return;
+}
+
+bool VideoCaptureManager::IsOnCaptureDeviceThread() const {
+  return MessageLoop::current() == vc_device_thread_.message_loop();
+}
+
+void VideoCaptureManager::GetAvailableDevices(
+    media::VideoCaptureDevice::Names* device_names) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  if (!use_fake_device_) {
+     media::VideoCaptureDevice::GetDeviceNames(device_names);
+  } else {
+    media::FakeVideoCaptureDevice::GetDeviceNames(device_names);
+  }
+  return;
+}
+
+bool VideoCaptureManager::DeviceOpened(
+    const media::VideoCaptureDevice::Name& device_name) {
+  DCHECK(IsOnCaptureDeviceThread());
+
+  for (VideoCaptureDevices::iterator it = devices_.begin();
+      it != devices_.end();
+      ++it) {
+    if (device_name.unique_id.compare(it->second->device_name().unique_id) ==

[John Knottenbelt, 2011/05/11 11:36:35]

I think we can use a directy comparison (==) instead of .compare? 

Same below.

[mflodman1, 2011/05/15 20:24:00]

Done.

+        0) {
+      // We've found the device!
+      return true;
+    }
+  }
+  return false;
+}
+
+bool VideoCaptureManager::DeviceOpened(

[John Knottenbelt, 2011/05/11 11:36:35]

Add DCHECK to ensure/document what thread this method should be running on.

[mflodman1, 2011/05/15 20:24:00]

Done.

+    const MediaCaptureDeviceInfo& device_info) {
+  for (VideoCaptureDevices::iterator it = devices_.begin();
+      it != devices_.end();
+      it++) {
+    if (device_info.device_id.compare(it->second->device_name().unique_id) ==
+        0) {
+      return true;
+    }
+  }
+  return false;
+}
+
+void VideoCaptureManager::UseFakeDevice() {
+  use_fake_device_ = true;
+}
+
+MessageLoop* VideoCaptureManager::GetMessageLoop() {
+  return vc_device_thread_.message_loop();
+}
+
+}  // namespace media