Skip to content

Commit

Permalink
Currently, Chromium does not listen for error notifications from QTKi…
Browse files Browse the repository at this point in the history
…t, i.e. they are ignored. QTKit device creation can succeed, and then later fail asynchronously via such a message, never producing any frames. If Chromium doesn't listen for the error notifications, then this results in a getUserMedia call that never fires its success or failure callbacks.

For example, with this change, if too many cameras are connected to the same USB hub, then bandwidth limitations may cause some of the cameras to fail to open.  This results in a QTCaptureSessionRuntimeErrorNotification whose localized text reads "The operation couldn't be completed. (OSStatus error -536870164.)".  With this change, that notification results in a getUserMedia failure callback to the javascript (with event.name == 'PERMISSION_DENIED', which is the only currently supported error code).

BUG=265704

Review URL: https://chromiumcodereview.appspot.com/22982008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218892 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
bemasc@chromium.org committed Aug 22, 2013
1 parent 7be9c7d commit 3126881
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 1 deletion.
13 changes: 13 additions & 0 deletions media/video/capture/mac/video_capture_device_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include <string>

#include "base/compiler_specific.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop_proxy.h"
#include "media/video/capture/video_capture_device.h"
#include "media/video/capture/video_capture_types.h"

Expand Down Expand Up @@ -38,6 +41,8 @@ class VideoCaptureDeviceMac : public VideoCaptureDevice {
void ReceiveFrame(const uint8* video_frame, int video_frame_length,
const VideoCaptureCapability& frame_info);

void ReceiveError(const std::string& reason);

private:
void SetErrorState(const std::string& reason);

Expand All @@ -52,8 +57,16 @@ class VideoCaptureDeviceMac : public VideoCaptureDevice {

Name device_name_;
VideoCaptureDevice::EventHandler* observer_;

// Only read and write state_ from inside this loop.
const scoped_refptr<base::MessageLoopProxy> loop_proxy_;
InternalState state_;

// Used with Bind and PostTask to ensure that methods aren't called
// after the VideoCaptureDeviceMac is destroyed.
base::WeakPtrFactory<VideoCaptureDeviceMac> weak_factory_;
base::WeakPtr<VideoCaptureDeviceMac> weak_this_;

VideoCaptureDeviceQTKit* capture_device_;

DISALLOW_COPY_AND_ASSIGN(VideoCaptureDeviceMac);
Expand Down
22 changes: 21 additions & 1 deletion media/video/capture/mac/video_capture_device_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#import <QTKit/QTKit.h>

#include "base/bind.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/time/time.h"
#include "media/video/capture/mac/video_capture_device_qtkit_mac.h"
Expand Down Expand Up @@ -94,17 +96,22 @@ Name name([[capture_devices valueForKey:key] UTF8String],
VideoCaptureDeviceMac::VideoCaptureDeviceMac(const Name& device_name)
: device_name_(device_name),
observer_(NULL),
loop_proxy_(base::MessageLoopProxy::current()),
state_(kNotInitialized),
weak_factory_(this),
weak_this_(weak_factory_.GetWeakPtr()),
capture_device_(nil) {
}

VideoCaptureDeviceMac::~VideoCaptureDeviceMac() {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
[capture_device_ release];
}

void VideoCaptureDeviceMac::Allocate(
const VideoCaptureCapability& capture_format,
EventHandler* observer) {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
if (state_ != kIdle) {
return;
}
Expand Down Expand Up @@ -153,6 +160,7 @@ Name name([[capture_devices valueForKey:key] UTF8String],
}

void VideoCaptureDeviceMac::Start() {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
DCHECK_EQ(state_, kAllocated);
if (![capture_device_ startCapture]) {
SetErrorState("Could not start capture device.");
Expand All @@ -162,12 +170,14 @@ Name name([[capture_devices valueForKey:key] UTF8String],
}

void VideoCaptureDeviceMac::Stop() {
DCHECK_EQ(state_, kCapturing);
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
DCHECK(state_ == kCapturing || state_ == kError) << state_;
[capture_device_ stopCapture];
state_ = kAllocated;
}

void VideoCaptureDeviceMac::DeAllocate() {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
if (state_ != kAllocated && state_ != kCapturing) {
return;
}
Expand All @@ -185,6 +195,7 @@ Name name([[capture_devices valueForKey:key] UTF8String],
}

bool VideoCaptureDeviceMac::Init() {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
DCHECK_EQ(state_, kNotInitialized);

Names device_names;
Expand All @@ -206,11 +217,20 @@ Name name([[capture_devices valueForKey:key] UTF8String],
const uint8* video_frame,
int video_frame_length,
const VideoCaptureCapability& frame_info) {
// This method is safe to call from a device capture thread,
// i.e. any thread controlled by QTKit.
observer_->OnIncomingCapturedFrame(
video_frame, video_frame_length, base::Time::Now(), 0, false, false);
}

void VideoCaptureDeviceMac::ReceiveError(const std::string& reason) {
loop_proxy_->PostTask(FROM_HERE,
base::Bind(&VideoCaptureDeviceMac::SetErrorState, weak_this_,
reason));
}

void VideoCaptureDeviceMac::SetErrorState(const std::string& reason) {
DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
DLOG(ERROR) << reason;
state_ = kError;
observer_->OnError();
Expand Down
3 changes: 3 additions & 0 deletions media/video/capture/mac/video_capture_device_qtkit_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ namespace media {
// Stops video capturing.
- (void)stopCapture;

// Handle any QTCaptureSessionRuntimeErrorNotifications.
- (void)handleNotification:(NSNotification *)errorNotification;

@end

#endif // MEDIA_VIDEO_CAPTURE_MAC_VIDEO_CAPTURE_DEVICE_MAC_QTKIT_H_
15 changes: 15 additions & 0 deletions media/video/capture/mac/video_capture_device_qtkit_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ - (BOOL)setCaptureDevice:(NSString *)deviceId {
}
if ([[captureSession_ outputs] count] > 0) {
// Only one output is set for |captureSession_|.
DCHECK_EQ([[captureSession_ outputs] count], 1u);
id output = [[captureSession_ outputs] objectAtIndex:0];
[output setDelegate:nil];

Expand Down Expand Up @@ -194,6 +195,12 @@ - (BOOL)startCapture {
<< [[error localizedDescription] UTF8String];
return NO;
}
NSNotificationCenter * notificationCenter =
[NSNotificationCenter defaultCenter];
[notificationCenter addObserver:self
selector:@selector(handleNotification:)
name:QTCaptureSessionRuntimeErrorNotification
object:captureSession_];
[captureSession_ startRunning];
}
return YES;
Expand All @@ -204,6 +211,8 @@ - (void)stopCapture {
[captureSession_ removeInput:captureDeviceInput_];
[captureSession_ stopRunning];
}

[[NSNotificationCenter defaultCenter] removeObserver:self];
}

// |captureOutput| is called by the capture device to deliver a new frame.
Expand Down Expand Up @@ -269,4 +278,10 @@ - (void)captureOutput:(QTCaptureOutput *)captureOutput
[lock_ unlock];
}

- (void)handleNotification:(NSNotification *)errorNotification {
NSError * error = (NSError *)[[errorNotification userInfo]
objectForKey:QTCaptureSessionErrorKey];
frameReceiver_->ReceiveError([[error localizedDescription] UTF8String]);
}

@end

0 comments on commit 3126881

Please sign in to comment.