Mac Video Capture: Connect error logging to WebRTC Log

Mac Video Capture: Connect error logging to WebRTC Log.

Connect the scattered DLOG(ERROR) to also send an error message to
WebRTC Log. Also log the type of API in use: QTKit or AVFoundation.

BUG=b/14652504

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273863

[mcasas, 2014-05-30 10:07:41]

perkj@, grunell@ PTAL

[Henrik Grunell, 2014-05-30 11:48:31]

Is there a bug for this?

https://codereview.chromium.org/308813002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/308813002/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/video_capture_controller.cc:493:
MediaStreamManager::SendMessageToNativeLog("video capture: " + message);
Start with capital letter, "Video".

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_avfoundation_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_avfoundation_mac.mm:278: [error
localizedDescription],
Fix indentation.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_mac.mm:382: SendSystemLog("Using
AVFoundation for device: " + device_name_.name());
Just to confirm: device_name_.name() is a user friendly name right? No unique
ID?

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_mac.mm:567: void
VideoCaptureDeviceMac::SendSystemLog(const std::string& message) {
Unclear to me why it's called SendSystemLog. Can you explain or rename.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_qtkit_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:103:
stringWithFormat:@"Could not open video capture device: %@ %@",
Clarify by for example "...: %@, reason: %@". Here and everywhere in the CL.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:327: [error
localizedDescription],
Fix indentation.

[mcasas, 2014-05-30 12:15:27]

PTAL.

Bug is actually b/14652504 , we can create a crbug.com if interesting.

https://codereview.chromium.org/308813002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/308813002/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/video_capture_controller.cc:493:
MediaStreamManager::SendMessageToNativeLog("video capture: " + message);
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Start with capital letter, "Video".

Done.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_avfoundation_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_avfoundation_mac.mm:278: [error
localizedDescription],
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Fix indentation.

Done.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_mac.mm:382: SendSystemLog("Using
AVFoundation for device: " + device_name_.name());
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Just to confirm: device_name_.name() is a user friendly name right? No unique
> ID?

Yes, is the camera user friendly name, with the vendor id and product id
suffixed:

"FaceTime HD Camera (Built-in) (058c:8510)"

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_mac.mm:567: void
VideoCaptureDeviceMac::SendSystemLog(const std::string& message) {
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Unclear to me why it's called SendSystemLog. Can you explain or rename.

Renamed to a more simpler name.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_qtkit_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:103:
stringWithFormat:@"Could not open video capture device: %@ %@",
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Clarify by for example "...: %@, reason: %@". Here and everywhere in the CL.

Used the original "Big error (%@): %@", this should output stuff like:

"Big error (Operation could not be completed): Device is inexistent"

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:327: [error
localizedDescription],
On 2014/05/30 11:48:32, Henrik Grunell wrote:
> Fix indentation.

Done.

[Henrik Grunell, 2014-05-30 12:30:47]

Update the title and description to "WebRTC log" instead of "system log".

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_avfoundation_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_avfoundation_mac.mm:278: [error
localizedDescription],
On 2014/05/30 12:15:27, mcasas wrote:
> On 2014/05/30 11:48:32, Henrik Grunell wrote:
> > Fix indentation.
> 
> Done.

Nit: I don't know how objective-c is usually indented, but it seems above that
the [ usually is under the @. Maybe line break before stringWithFormat and
format like that? I'm fine with either.

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_qtkit_mac.mm (right):

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:103:
stringWithFormat:@"Could not open video capture device: %@ %@",
On 2014/05/30 12:15:27, mcasas wrote:
> On 2014/05/30 11:48:32, Henrik Grunell wrote:
> > Clarify by for example "...: %@, reason: %@". Here and everywhere in the CL.
> 
> Used the original "Big error (%@): %@", this should output stuff like:
> 
> "Big error (Operation could not be completed): Device is inexistent"

SGTM

https://codereview.chromium.org/308813002/diff/20001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:327: [error
localizedDescription],
On 2014/05/30 12:15:27, mcasas wrote:
> On 2014/05/30 11:48:32, Henrik Grunell wrote:
> > Fix indentation.
> 
> Done.

Nit: Same here.

https://codereview.chromium.org/308813002/diff/40001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_qtkit_mac.mm (right):

https://codereview.chromium.org/308813002/diff/40001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:332: [lock_ lock];
DLOG here?

[mcasas, 2014-05-30 12:55:10]

PTAL.

https://codereview.chromium.org/308813002/diff/40001/media/video/capture/mac/...
File media/video/capture/mac/video_capture_device_qtkit_mac.mm (right):

https://codereview.chromium.org/308813002/diff/40001/media/video/capture/mac/...
media/video/capture/mac/video_capture_device_qtkit_mac.mm:332: [lock_ lock];
On 2014/05/30 12:30:48, Henrik Grunell wrote:
> DLOG here?

Done.

[Henrik Grunell, 2014-05-30 13:02:16]

lgtm

[perkj_chrome, 2014-05-30 13:17:16]

lgtm

[mcasas, 2014-05-30 13:29:49]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-05-30 13:31:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mcasas@chromium.org/308813002/60001

[commit-bot: I haz the power, 2014-05-30 17:09:21]

Change committed as 273863