VideoCapturerDelegate: Retrieve supported/in-use format(s) for constraint negotiation

content/renderer/media/media_stream_video_capturer_source.cc

Index: content/renderer/media/media_stream_video_capturer_source.cc
diff --git a/content/renderer/media/media_stream_video_capturer_source.cc b/content/renderer/media/media_stream_video_capturer_source.cc
index 722844bdbdf806429d3903c34cbcf552a7296ac5..dfaee99aea2869c1ba2826a391b5d91eaf4f25c0 100644
--- a/content/renderer/media/media_stream_video_capturer_source.cc
+++ b/content/renderer/media/media_stream_video_capturer_source.cc
@@ -41,7 +41,8 @@ VideoCapturerDelegate::VideoCapturerDelegate(
               ->UseDevice(device_info.session_id)),
       is_screen_cast_(device_info.device.type == MEDIA_TAB_VIDEO_CAPTURE ||
                       device_info.device.type == MEDIA_DESKTOP_VIDEO_CAPTURE),
-      got_first_frame_(false) {
+      got_first_frame_(false),
+      source_formats_callback_() {

[perkj_chrome, 2014/03/12 19:55:48]

no need to initialize this here.

[mcasas, 2014/03/13 08:04:56]

Done.

   DVLOG(3) << "VideoCapturerDelegate::ctor";
   DCHECK(capture_engine_);
 }
@@ -74,18 +75,12 @@ void VideoCapturerDelegate::GetCurrentSupportedFormats(
     return;
   }
 
-  // This delegate implementation doesn't support capability enumeration.
-  // We need to guess what it supports.
-  media::VideoCaptureFormats formats;
-  for (size_t i = 0; i < arraysize(kVideoFormats); ++i) {
-    formats.push_back(
-        media::VideoCaptureFormat(
-            gfx::Size(kVideoFormats[i].width,
-                      kVideoFormats[i].height),
-            kVideoFormats[i].frame_rate,
-            media::PIXEL_FORMAT_I420));
-  }
-  callback.Run(formats);
+  DCHECK(source_formats_callback_.is_null());
+  source_formats_callback_ = callback;
+  RenderThreadImpl::current()->video_capture_impl_manager()

[perkj_chrome, 2014/03/12 19:55:48]

No... Do
capture_engine_->GetDeviceFormatInUse(base::Bind(&VideoCapturerDelegate::OnDeviceFormatsInUseReceived,
this);

Ie, do not use unretained, unretained is a promise that VideoCapturerDelegate
will not be deleted while waiting for the callback. If you only use this,
base::Bind will add a ref to VideoCaptureDelegate and thus ensure that the
object will not be deleted.

Since we already have capture_engine_, use it.

[mcasas, 2014/03/13 08:04:56]

Done.

+      ->UseDevice(session_id_)->GetDeviceFormatsInUse(
+          base::Bind(&VideoCapturerDelegate::OnDeviceFormatsInUseReceived,
+                     base::Unretained(this)));
 }
 
 void VideoCapturerDelegate::StartDeliver(
@@ -113,6 +108,40 @@ void VideoCapturerDelegate::StopDeliver() {
   started_callback_.Reset();

[perkj_chrome, 2014/03/12 19:55:48]

Do source_formats_callback_.Reset() here to make sure source_formats_callback_
is not triggered after StopDeliver is called. Please see the comment in
MediaStreamVideoSource.h on StopSourceImpl.

[mcasas, 2014/03/13 08:04:56]

Done.

 }
 
+void VideoCapturerDelegate::OnDeviceFormatsInUseReceived(
+    const media::VideoCaptureFormats& formats_in_use) {
+  DVLOG(3) << "OnDeviceFormatsInUseReceived: " << formats_in_use.size();
+  if(!formats_in_use.empty()) {
+    source_formats_callback_.Run(formats_in_use);

[perkj_chrome, 2014/03/12 19:55:48]

check for isnull.

[mcasas, 2014/03/13 08:04:56]

Done.

+  }else {
+    // If there are no formats in use, try to retrieve the whole list of them.
+    RenderThreadImpl::current()->video_capture_impl_manager()

[perkj_chrome, 2014/03/12 19:55:48]

Same as above.

[mcasas, 2014/03/13 08:04:56]

Done.

+        ->UseDevice(session_id_)->GetDeviceSupportedFormats(base::Bind(
+            &VideoCapturerDelegate::OnDeviceSupportedFormatsEnumerated,
+            base::Unretained(this)));
+  }
+}
+
+void VideoCapturerDelegate::OnDeviceSupportedFormatsEnumerated(
+    const media::VideoCaptureFormats& formats) {
+  DVLOG(3) << "OnDeviceSupportedFormatsEnumerated: " << formats.size()
+           << " received";
+  if(formats.size()) {
+    source_formats_callback_.Run(formats);

[perkj_chrome, 2014/03/12 19:55:48]

check for isnull.

[mcasas, 2014/03/13 08:04:56]

Done.

+  } else {
+    // The capture device doesn't seem to support capability enumeration,
+    // compose a fallback list of capabilities.
+    media::VideoCaptureFormats default_formats;
+    for (size_t i = 0; i < arraysize(kVideoFormats); ++i) {
+      default_formats.push_back(media::VideoCaptureFormat(
+          gfx::Size(kVideoFormats[i].width, kVideoFormats[i].height),
+          kVideoFormats[i].frame_rate,
+          media::PIXEL_FORMAT_I420));
+    }
+    source_formats_callback_.Run(default_formats);
+  }
+}
+
 void VideoCapturerDelegate::OnStarted(media::VideoCapture* capture) {
   DVLOG(3) << "VideoCapturerDelegate::OnStarted";
 }