From Video Capture, abolish OnFrameInfo and enable resolution changes

content/browser/renderer_host/media/web_contents_video_capture_device.cc

Index: content/browser/renderer_host/media/web_contents_video_capture_device.cc
diff --git a/content/browser/renderer_host/media/web_contents_video_capture_device.cc b/content/browser/renderer_host/media/web_contents_video_capture_device.cc
index 7aca8df58809a3f1ee994bd96bc5f9b650e1cdca..6537661b0332f05ae0eead497411dac90f129665 100644
--- a/content/browser/renderer_host/media/web_contents_video_capture_device.cc
+++ b/content/browser/renderer_host/media/web_contents_video_capture_device.cc
@@ -139,7 +139,8 @@ class ThreadSafeCaptureOracle
     : public base::RefCountedThreadSafe<ThreadSafeCaptureOracle> {
  public:
   ThreadSafeCaptureOracle(scoped_ptr<media::VideoCaptureDevice::Client> client,
-                          scoped_ptr<VideoCaptureOracle> oracle);
+                          scoped_ptr<VideoCaptureOracle> oracle,
+                          const gfx::Size& capture_size);
 
   bool ObserveEventAndDecideCapture(
       VideoCaptureOracle::Event event,
@@ -174,6 +175,9 @@ class ThreadSafeCaptureOracle
 
   // Makes the decision to capture a frame.
   const scoped_ptr<VideoCaptureOracle> oracle_;
+
+  // The resolution at which we're capturing.
+  const gfx::Size capture_size_;
 };
 
 // FrameSubscriber is a proxy to the ThreadSafeCaptureOracle that's compatible
@@ -396,8 +400,12 @@ class VideoFrameDeliveryLog {
 
 ThreadSafeCaptureOracle::ThreadSafeCaptureOracle(
     scoped_ptr<media::VideoCaptureDevice::Client> client,
-    scoped_ptr<VideoCaptureOracle> oracle)
-    : client_(client.Pass()), oracle_(oracle.Pass()) {}
+    scoped_ptr<VideoCaptureOracle> oracle,
+    const gfx::Size& capture_size)
+    : client_(client.Pass()),
+      oracle_(oracle.Pass()),
+      capture_size_(capture_size) {
+}
 
 bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     VideoCaptureOracle::Event event,
@@ -410,7 +418,7 @@ bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     return false;  // Capture is stopped.
 
   scoped_refptr<media::VideoFrame> output_buffer =
-      client_->ReserveOutputBuffer();
+      client_->ReserveOutputBuffer(capture_size_);
   const bool should_capture =
       oracle_->ObserveEventAndDecideCapture(event, event_time);
   const bool content_is_dirty =
@@ -1028,27 +1036,20 @@ void WebContentsVideoCaptureDevice::Impl::AllocateAndStart(
     return;
   }
 
-  // Initialize capture settings which will be consistent for the
-  // duration of the capture.
+  // Need to call OnFrameInfo just to set the frame rate.

[Ami GONE FROM CHROMIUM, 2013/10/04 00:24:15]

Time for a special-purpose setter?

[ncarter (slow), 2013/10/16 02:08:40]

I'm not quite sure what to do about frame_rate. It kind of stands alone. In
practice, only flash seems to depend on it (it doesn't seem to be hooked up to
the webrtc consumer).

Options are: 
 1. Special purpose setter, as you suggest.
 2. Pass as argument (similar to timestamp) to frame delivery /
OnIncomingCapturedVideoFrame.
 3. Keep using OnFrameInfo just for this.

I was leaning toward (3) -- though that's probably a bias towards not having
this CL introduce yet another tweak to the video_capture_device / client
interface.

[Ami GONE FROM CHROMIUM, 2013/10/17 20:31:45]

I like #1 best, but I agree fixing this doesn't need to be part of this CL. 
File a bug, point to it with a TODO here, and call it a day?

[ncarter (slow), 2013/10/22 01:06:20]

Done.

+  // The other parameters of this struct are ignored.
   media::VideoCaptureCapability settings;
-
-  settings.width = width;
-  settings.height = height;
   settings.frame_rate = frame_rate;
-  // Note: the value of |settings.color| doesn't matter if we use only the
-  // VideoFrame based methods on |client|.
-  settings.color = media::PIXEL_FORMAT_I420;
-  settings.expected_capture_delay = 0;
-  settings.interlaced = false;
+  client->OnFrameInfo(settings);
 
   base::TimeDelta capture_period = base::TimeDelta::FromMicroseconds(
-      1000000.0 / settings.frame_rate + 0.5);
+      1000000.0 / frame_rate + 0.5);
 
-  client->OnFrameInfo(settings);
   scoped_ptr<VideoCaptureOracle> oracle(
       new VideoCaptureOracle(capture_period,
                              kAcceleratedSubscriberIsSupported));
-  oracle_proxy_ = new ThreadSafeCaptureOracle(client.Pass(), oracle.Pass());
+  oracle_proxy_ = new ThreadSafeCaptureOracle(
+      client.Pass(), oracle.Pass(), gfx::Size(width, height));
 
   // Allocates the CaptureMachine. The CaptureMachine will be tracking render
   // view swapping over its lifetime, and we don't want to lose our reference to