Adding VideoCaptureDevice for Mac.

media/video/capture/mac/video_capture_device_mac_qtkit.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.
+
+#import "media/video/capture/mac/video_capture_device_mac_qtkit.h"
+
+#import <QTKit/QTKit.h>
+
+@implementation VideoCaptureDeviceMacQTKit
+
+- (id)init {
+  if (self = [super init]) {
+    pool_ = [[NSAutoreleasePool alloc]init];

[dmac, 2011/10/06 23:36:22]

space between ] and init

[dmac, 2011/10/06 23:36:22]

do you really need an autoreleasepool? please explain why

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Removed. One of my object-c misunderstandings.

+    frameRate_ = 0;
+    frameWidth_ = 0;

[dmac, 2011/10/06 23:36:22]

no need to set things to zero/nil. This happens automagically in objc

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+    frameHeight_ = 0;
+    captureSession_ = nil;
+    captureDecompressedOutput_ = nil;
+    captureDeviceInput_ = nil;
+  }
+  return self;
+}
+
+- (void)dealloc {
+  if(captureSession_) {

[dmac, 2011/10/06 23:36:22]

no need to check if these are nil

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+    [captureSession_ release];
+    captureSession_ = nil;
+  }
+  if (captureDecompressedOutput_) {
+    [captureDecompressedOutput_ release];
+    captureDecompressedOutput_ = nil;
+  }
+  if (captureDeviceInput_) {
+    [captureDeviceInput_ release];
+    captureDeviceInput_ = nil;
+  }
+  [pool_ drain];
+  [super dealloc];
+}
+
+#pragma mark Class methods
+
++ (void) deviceNames:(NSArray *) deviceList {

[dmac, 2011/10/06 23:36:22]

no space between ) and device (in both cases)

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Modified.

+  [deviceList initWithArray:[QTCaptureDevice

[dmac, 2011/10/06 23:36:22]

This does not do what you think it does. My guess it you just want to return
[QTCaptureDevice inputDevicesWithMediaType:QTMediaTypeVideo]

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+                              inputDevicesWithMediaType:QTMediaTypeVideo]];
+}
+
+#pragma mark Public methods
+
+- (void)registerReceiver:(media::VideoCaptureDeviceMac *)frameReceiver {

[dmac, 2011/10/06 23:36:22]

should this be "setFrameReceiver" since you can only have one of them?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Good point, changed.

+  frameReceiver_ = frameReceiver;
+}
+
+- (BOOL)setCaptureDevice:(const char *)name {

[dmac, 2011/10/06 23:36:22]

so on one side of the interface you take an NSString and convert it to a char *,
and then you rebuild it into an NSString? how about just passing an NSString...

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+  if (captureDeviceInput_) {
+    // A device is already set.
+    return NO;
+  }
+
+  // Find the requested device, open it and set as input.
+  NSArray* captureDevices;
+  captureDevices = [[[NSArray alloc] initWithArray:

[dmac, 2011/10/06 23:36:22]

There's a couple of bugs here, but can I suggest an alternative?

NSArray *captureDevices =
[QTCaptureDeviceinputDevicesWithMediaType:QTMediaTypeVideo];
NSArray *captureDevicesNames = [captureDevices
valueForKey:@"localizedDisplayName"];
NSUInteger index = [captureDevicesNames indexOfObject:name]; // This assumes
name is passed in as a NSString * as suggested above
if (index == NSNotFound) {
  // Possibly log here?
  return NO;
}
QTCaptureDevice *device = [captureDevices objectAtIndex:index];
NSError *error = nil;
if (![captureDevice open:&error]) {
  // Log error
  return NO;
}
captureDeviceInput_ = [QTCaptureDeviceInput alloc] initWithDevice:device];
....

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Changed to the suggested code, almost... I changed to use the uniqueId instead
of name to distinguish between different devices with the same name. 

+      [QTCaptureDevice inputDevicesWithMediaType:QTMediaTypeVideo]]
+      autorelease];
+
+  if ([captureDevices count] == 0) {
+    return NO;
+  }
+
+  for(NSUInteger index = 0; index < captureDevices.count; ++index) {
+    NSError* error;
+    QTCaptureDevice* captureDevice =
+        (QTCaptureDevice*)[captureDevices objectAtIndex:index];
+
+    char captureDeviceName[1024];
+    [[captureDevice localizedDisplayName] getCString:captureDeviceName
+                                           maxLength:1024
+                                            encoding:NSUTF8StringEncoding];
+    if (!strcmp(captureDeviceName, name)) {
+      if (![captureDevice open:&error]) {
+        return NO;
+      }
+      captureDeviceInput_ = [QTCaptureDeviceInput alloc];
+      [captureDeviceInput_ initWithDevice:captureDevice];
+      captureSession_ = [[QTCaptureSession alloc] init];
+      captureDecompressedOutput_ =
+          [[QTCaptureDecompressedVideoOutput alloc] init];
+      [captureDecompressedOutput_ setDelegate:self];
+      return YES;
+    }
+  }
+  return NO;
+}
+
+- (void)removeCaptureDevice {
+  if ([[captureSession_ inputs] count] > 0) {
+    // The device is still running.
+    [self stopCapture];
+  }
+  if (captureDecompressedOutput_) {

[dmac, 2011/10/06 23:36:22]

no need to check. Just call release on it.

Same with captrueSession_ and captureDeviceInput_ etc.

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+    [captureDecompressedOutput_ release];
+    captureDecompressedOutput_ = nil;
+  }
+  if(captureSession_) {
+    [captureSession_ release];
+    captureSession_ = nil;
+  }
+  if (captureDeviceInput_) {
+    [captureDeviceInput_ release];
+    captureDeviceInput_ = nil;
+  }
+}
+
+- (BOOL)setCaptureHeight:(int)height
+                AndWidth:(int)width
+            AndFrameRate:(int)frameRate {
+  // Verify an input device has been set.
+  if (!captureDeviceInput_) {
+    return NO;
+  }
+
+  frameWidth_ = width;
+  frameHeight_ = height;
+  frameRate_ = frameRate;
+
+  // Set up desired output properties.
+  NSDictionary* captureDictionary =
+      [NSDictionary dictionaryWithObjectsAndKeys:
+          [NSNumber numberWithDouble:frameWidth_],
+          (id)kCVPixelBufferWidthKey,
+          [NSNumber numberWithDouble:frameHeight_],
+          (id)kCVPixelBufferHeightKey,
+          [NSNumber numberWithUnsignedInt:kCVPixelFormatType_32BGRA],
+          (id)kCVPixelBufferPixelFormatTypeKey,
+          nil];
+  [captureDecompressedOutput_ setPixelBufferAttributes:captureDictionary];
+
+  // Connect the output with the session.
+  NSError* error;
+  if (![captureSession_ addOutput:captureDecompressedOutput_ error:&error]) {
+    return NO;

[dmac, 2011/10/06 23:36:22]

log the error?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+  }
+  return YES;
+}
+
+- (BOOL)startCapture {
+  if ([[captureSession_ outputs] count] == 0) {
+    // Capture properties not set.
+    return NO;
+  }
+  if ([[captureSession_ inputs] count] == 0) {
+    NSError* error;
+    if (![captureSession_ addInput:captureDeviceInput_ error:&error]) {
+      return NO;

[dmac, 2011/10/06 23:36:22]

log the error?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Done.

+    }
+    [[NSRunLoop currentRunLoop] runUntilDate:[NSDate distantFuture]];

[dmac, 2011/10/06 23:36:22]

what breaks out of this runloop?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

The runloop was originally from sample, which I probably had misunderstood.

+    [captureSession_ startRunning];
+  }
+  return YES;
+}
+
+- (void)stopCapture {
+  if ([[captureSession_ inputs] count] > 0) {
+    [captureSession_ removeInput:captureDeviceInput_];
+    [captureSession_ stopRunning];
+  }
+}
+
+// |captureOutput| is called by the capture device to deliver a new frame.
+- (void)captureOutput:(QTCaptureOutput *)captureOutput
+  didOutputVideoFrame:(CVImageBufferRef)videoFrame
+     withSampleBuffer:(QTSampleBuffer *)sampleBuffer
+       fromConnection:(QTCaptureConnection *)connection {
+
+  // Lock the frame and calculate frame size.
+  const int LOCK_FLAGS = 0;
+  CVPixelBufferLockBaseAddress(videoFrame, LOCK_FLAGS);
+  void* baseAddress = CVPixelBufferGetBaseAddress(videoFrame);
+  size_t bytesPerRow = CVPixelBufferGetBytesPerRow(videoFrame);
+  int frameHeight = CVPixelBufferGetHeight(videoFrame);
+
+  if(frameReceiver_) {

[dmac, 2011/10/06 23:36:22]

do you want to check frameReceiver_ before doing any of the above?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

Good point, changed.

+    int frameSize = bytesPerRow * frameHeight;
+    CVBufferRetain(videoFrame);

[dmac, 2011/10/06 23:36:22]

why do feel you need to retain and release the videoFrame in here?

[mflodman_chromium_OOO, 2011/10/07 13:03:51]

It was to make sure videoFrame was valid during the entire call and that is the
way it is done in the samples I've seen. I couldn't find any documentation to
back this up and it seems reasonable that CVPixelBufferLockBaseAddress should be
enough.

Removed.

+    media::VideoCaptureDevice::Capability captureCapability;
+    captureCapability.width = frameWidth_;
+    captureCapability.height = frameHeight_;
+    captureCapability.frame_rate = frameRate_;
+    captureCapability.color = media::VideoCaptureDevice::kARGB;
+
+    frameReceiver_->IncomingFrame(baseAddress, frameSize, captureCapability);
+
+    CVBufferRelease(videoFrame);
+  }
+  CVPixelBufferUnlockBaseAddress(videoFrame, LOCK_FLAGS);
+}
+
+@end