Adding VideoCaptureDevice for Mac.

media/video/capture/mac/video_capture_device_mac.mm

+// 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 "media/video/capture/mac/video_capture_device_mac.h"
+
+#import <QTKit/QTKit.h>
+
+#include "base/logging.h"
+#include "base/time.h"
+#include "media/video/capture/mac/video_capture_device_mac_qtkit.h"
+
+namespace media {
+
+void VideoCaptureDevice::GetDeviceNames(Names* device_names) {
+  // Loop through all available devices and add to |device_names|.
+  device_names->clear();
+  // TODO(mflodman) Return name and id as NSArray* instead of QTCaptureDevice*.
+  for (QTCaptureDevice* device in [VideoCaptureDeviceMacQTKit deviceNames]) {
+    Name name;
+    NSString* qt_device_name = [device localizedDisplayName];
+    name.device_name = [qt_device_name UTF8String];
+    NSString* qt_unique_id = [device uniqueID];
+    name.unique_id = [qt_unique_id UTF8String];
+    device_names->push_back(name);
+  }
+}
+
+VideoCaptureDevice* VideoCaptureDevice::Create(const Name& device_name) {
+  VideoCaptureDeviceMac* capture_device =
+    new VideoCaptureDeviceMac(device_name);

[scherkus (not reviewing), 2011/10/07 17:17:55]

indent by 2 more spaces

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Done.

+  if (!capture_device) {

[scherkus (not reviewing), 2011/10/07 17:17:55]

is this checking for a failed memory allocation?

these types of checks aren't typically useful, especially since Chrome uses
base::EnableTerminationOnOutOfMemory() (i.e., this code will never execute)

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Ok, thanks for the info. Removed.

It was done like this in the other VideoCaptureDevice implementations, so we
should remove them there as well.

+    LOG(ERROR) << "Could not create VideoCaptureDevice instance.";
+    return NULL;
+  }
+  if (!capture_device->Init()) {
+    LOG(ERROR) << "Could not initialize VideoCaptureDevice.";
+    delete capture_device;
+    return NULL;
+  }
+  return capture_device;
+}
+
+VideoCaptureDeviceMac::VideoCaptureDeviceMac(const Name& device_name)

[scherkus (not reviewing), 2011/10/07 17:17:55]

could you re-order the functions to match header file order?

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

The only disorder I see is the static VideoCaptureDevice:: functions I put at
top, since they are not part of the header file.

+    : device_name_(device_name),
+      observer_(NULL),
+      state_(kNotInitialized),
+      capture_device_(NULL) {
+}
+
+VideoCaptureDeviceMac::~VideoCaptureDeviceMac() {
+  [capture_device_ release];

[scherkus (not reviewing), 2011/10/07 17:17:55]

I'm an ObjC noob, but is it OK to call release on a NULL object?

[dmac, 2011/10/07 17:38:55]

Technically it's a "nil" object, and yes it is ;-)

+}
+
+void VideoCaptureDeviceMac::Allocate(int width, int height, int frame_rate,
+                                     EventHandler* observer) {
+  if (state_ != kIdle) {
+    return;

[scherkus (not reviewing), 2011/10/07 17:17:55]

is it an error to call Allocate if we're not idle? should we add a DCHECK?

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Linux and Win returns, instead of DCHECK, and it is tested that
observer_->OnFrameInfo is not called for the second call. That test is disabled
for Mac right now, but I'll keep the code to get the same behavior.

+  }
+
+  observer_ = observer;
+  NSString* deviceId =
+      [NSString stringWithUTF8String:device_name_.unique_id.c_str()];

[mflodman_chromium_OOO, 2011/10/07 15:15:29]

I'll change this to make it autorelease instead.

[dmac, 2011/10/07 17:38:55]

I'm confused, as this is already autoreleased

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

And you're probably confused since I was confused about the objective-c memory
terms. Got it now, I hope...

+
+  if (![capture_device_ setCaptureDevice:deviceId]) {
+    SetErrorState("Could not open capture device.");
+    return;
+  }
+  if ([capture_device_ setCaptureCapabilities:height:width:frame_rate] == NO) {
+    SetErrorState("Could not configure capture device.");
+    return;
+  }
+
+  state_ = kAllocated;
+  Capability current_settings;
+  current_settings.color = kARGB;
+  current_settings.width = width;
+  current_settings.height = height;
+  current_settings.frame_rate = frame_rate;
+
+  observer_->OnFrameInfo(current_settings);
+}
+
+void VideoCaptureDeviceMac::Start() {
+  if (state_ != kAllocated) {

[scherkus (not reviewing), 2011/10/07 17:17:55]

ditto for error / DCHECK-worthiness

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Done.

+    return;
+  }
+  if (![capture_device_ startCapture]) {
+    SetErrorState("Could not start capture device.");
+    return;
+  }
+  state_ = kCapturing;
+}
+
+void VideoCaptureDeviceMac::Stop() {
+  if (state_ != kCapturing) {

[scherkus (not reviewing), 2011/10/07 17:17:55]

ditto for error / DCHECK-worthiness

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Done.

+    return;
+  }
+  [capture_device_ stopCapture];
+  state_ = kAllocated;
+}
+
+void VideoCaptureDeviceMac::DeAllocate() {
+  if (state_ != kAllocated && state_ != kCapturing) {

[scherkus (not reviewing), 2011/10/07 17:17:55]

ditto for error / DCHECK-worthiness

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

DeAllocate will actually be called twice in most cases, so it's a bit different
than the other API calls. (Won't go into details, but this is since a device can
be closed directly by MediaStreamManager code).

Maybe we can change this in some way, but then we need to change for all three
platforms in the same time.

+    return;
+  }
+  if (state_ == kCapturing) {
+    [capture_device_ stopCapture];
+  }
+  [capture_device_ setCaptureDevice:nil];
+  state_ = kIdle;
+}
+
+const VideoCaptureDevice::Name& VideoCaptureDeviceMac::device_name() {
+  return device_name_;
+}
+
+bool VideoCaptureDeviceMac::Init() {

[dmac, 2011/10/07 17:38:55]

From an API point of view, does it make sense to have this init function? The
only thing that is really going to fail here is the allocation of the
capture_device_. Should this not be done in Allocate? Then DeAllocate could
release the capture_device potentially freeing up system resources? This would
also allow you to simplify the interface for VideoCaptureDeviceMatQTKit by
passing in the init all of the values you are currently calling set on which
would make VideoCaptureDeviceMatQTKit basically an immutable object. I would
have it something like -(id)initWithDeviceID:(NSString*)id height:(int)height
width:(int)width frameRate:(int)frameRate
frameReceiver:(media::VideoCaptureDeviceMac *)frameReceiver. It would certainly
cut down on a lot of code.

Also I wonder if this would let you drop the state_ member var here, and keep
track of the state by querying the capture_device_. It doesn't seem right to
have state external to the capture_device_ to me.

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

The QTKit code is written right now to match Linux and Win in regards to when
things are done. I agree the code would be a lot nicer if everything would be
done as you suggest, but it wouldn't match Win and Linux.

+  DCHECK(state_ == kNotInitialized);

[scherkus (not reviewing), 2011/10/07 17:17:55]

DCHECK_EQ

[mflodman_chromium_OOO, 2011/10/10 12:56:52]

Done.

+  capture_device_ = [[VideoCaptureDeviceMacQTKit alloc] init];
+  if (capture_device_ == NULL) {
+    return false;
+  }
+  [capture_device_ setFrameReceiver:this];
+  state_ = kIdle;
+  return true;
+}
+
+int VideoCaptureDeviceMac::IncomingFrame(void* video_frame,
+                                         int video_frame_length,
+                                         const Capability& frame_info) {
+  observer_->OnIncomingCapturedFrame(static_cast<uint8*>(video_frame),
+                                     video_frame_length,
+                                     base::Time::Now());
+ return 0;
+}
+
+void VideoCaptureDeviceMac::SetErrorState(const std::string& reason) {
+  DLOG(ERROR) << reason;
+  state_ = kError;
+  observer_->OnError();
+}
+
+} // namespace media