Implement all resolution change policies for desktop and tab capture.

content/browser/media/capture/desktop_capture_device_unittest.cc

 // Copyright (c) 2013 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.
 
 #include "content/browser/media/capture/desktop_capture_device.h"
 
+#include <algorithm>
 #include <string>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/test/test_timeouts.h"
 #include "base/time/time.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_geometry.h"
 #include "third_party/webrtc/modules/desktop_capture/screen_capturer.h"
 
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::DoAll;
 using ::testing::Expectation;
+using ::testing::Invoke;
 using ::testing::InvokeWithoutArgs;
 using ::testing::SaveArg;
+using ::testing::WithArg;
 
 namespace content {
 
 namespace {
 
 MATCHER_P2(EqualsCaptureCapability, width, height, "") {
   return arg.width == width && arg.height == height;
 }
 
-const int kTestFrameWidth1 = 100;
-const int kTestFrameHeight1 = 100;
-const int kTestFrameWidth2 = 200;
-const int kTestFrameHeight2 = 150;
+const int kTestFrameWidth1 = 500;
+const int kTestFrameHeight1 = 500;
+const int kTestFrameWidth2 = 400;
+const int kTestFrameHeight2 = 300;
 
 const int kFrameRate = 30;
 
 // The value of the padding bytes in unpacked frames.
 const uint8_t kFramePaddingValue = 0;
 
 // Use a special value for frame pixels to tell pixel bytes apart from the
 // padding bytes in the unpacked frame test.
 const uint8_t kFakePixelValue = 1;
 
@@ skipping 236 matching lines @@
   EXPECT_CALL(*client, OnIncomingCapturedData(_, _, _, _, _)).WillRepeatedly(
       DoAll(SaveArg<1>(&frame_size),
             SaveArg<2>(&format),
             InvokeWithoutArgs(&done_event, &base::WaitableEvent::Signal)));
 
   media::VideoCaptureParams capture_params;
   capture_params.requested_format.frame_size.SetSize(kTestFrameWidth1,
                                                      kTestFrameHeight1);
   capture_params.requested_format.frame_rate = kFrameRate;
   capture_params.requested_format.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_params.resolution_change_policy =
+      media::RESOLUTION_POLICY_FIXED_RESOLUTION;
 
   capture_device_->AllocateAndStart(capture_params, client.Pass());
 
   // Capture at least two frames, to ensure that the source frame size has
   // changed while capturing.
   EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
   done_event.Reset();
   EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
 
   capture_device_->StopAndDeAllocate();
 
   EXPECT_EQ(kTestFrameWidth1, format.frame_size.width());
   EXPECT_EQ(kTestFrameHeight1, format.frame_size.height());
   EXPECT_EQ(kFrameRate, format.frame_rate);
   EXPECT_EQ(media::PIXEL_FORMAT_ARGB, format.pixel_format);
 
   EXPECT_EQ(format.frame_size.GetArea() * 4, frame_size);
 }
 
+// Test that screen capturer behaves correctly if the source frame size changes,
+// where the video frames sent the the client vary in resolution but maintain
+// the same aspect ratio.
+TEST_F(DesktopCaptureDeviceTest, ScreenResolutionChangeFixedAspectRatio) {
+  FakeScreenCapturer* mock_capturer = new FakeScreenCapturer();
+
+  CreateScreenCaptureDevice(scoped_ptr<webrtc::DesktopCapturer>(mock_capturer));
+
+  media::VideoCaptureFormats formats;
+  base::WaitableEvent done_event(false, false);
+
+  scoped_ptr<MockDeviceClient> client(new MockDeviceClient());
+  EXPECT_CALL(*client, OnError(_)).Times(0);
+  void (media::VideoCaptureFormats::*push_back_method)(
+      const media::VideoCaptureFormat&) =
+      &media::VideoCaptureFormats::push_back;
+  EXPECT_CALL(*client, OnIncomingCapturedData(_, _, _, _, _)).WillRepeatedly(
+      DoAll(WithArg<2>(Invoke(&formats, push_back_method)),
+            InvokeWithoutArgs(&done_event, &base::WaitableEvent::Signal)));
+
+  media::VideoCaptureParams capture_params;
+  const gfx::Size high_def_16_by_9(1920, 1080);
+  ASSERT_GE(high_def_16_by_9.width(),
+            std::max(kTestFrameWidth1, kTestFrameWidth2));
+  ASSERT_GE(high_def_16_by_9.height(),
+            std::max(kTestFrameHeight1, kTestFrameHeight2));
+  capture_params.requested_format.frame_size = high_def_16_by_9;
+  capture_params.requested_format.frame_rate = kFrameRate;
+  capture_params.requested_format.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_params.resolution_change_policy =
+      media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO;
+
+  capture_device_->AllocateAndStart(
+      capture_params, client.Pass());
+
+  // Capture at least three frames, to ensure that the source frame size has
+  // changed at least twice while capturing.

[Wez, 2015/05/14 01:44:55]

nit: Why is this not a loop?

Why do we care that the source frame size has changed at least twice, versus
just once?

If the requirement is that it has changed at least twice, could we simply
compare received frame sizes between OnIncomingCapturedData() invocations?

[miu, 2015/05/14 21:12:26]

I originally wrote this test by forking the existing one
(ScreenResolutionChangeVariableResolution).  I've fixed all the tests to use a
loop.
 
Elaborated in code comments.
Yes.  Thinking about this a bit, I've ripped out the existing "vector of sizes"
approach, and replaced it with a simple FrameChecker helper class/method.  I've
done this for the three tests that correspond to each of the three possible
resolution change policies.

+  EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
+  done_event.Reset();
+  EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
+  done_event.Reset();
+  EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
+
+  capture_device_->StopAndDeAllocate();
+
+  std::vector<gfx::Size> distinct_sizes_seen;
+  for (const media::VideoCaptureFormat& f : formats) {

[Wez, 2015/05/14 01:44:55]

nit: Could this be:

for (auto format : formats)

?

[miu, 2015/05/14 21:12:26]

Reply 1: It could, but the style guide suggests only to use it where it would
help readability (e.g., huge-string template class types, like iterators).  IMO,
it's more readable to provide the exact, known type here.

Reply 2: No longer applicable since I've replaced this code.

+    ASSERT_FALSE(f.frame_size.IsEmpty());
+    if (std::find(distinct_sizes_seen.begin(),
+                  distinct_sizes_seen.end(),
+                  f.frame_size) == distinct_sizes_seen.end()) {
+      distinct_sizes_seen.push_back(f.frame_size);
+    }
+    EXPECT_EQ(kFrameRate, f.frame_rate);
+    EXPECT_EQ(media::PIXEL_FORMAT_ARGB, f.pixel_format);
+  }
+  EXPECT_EQ(2u, distinct_sizes_seen.size());

[Wez, 2015/05/14 01:44:55]

Is there something about the mock capturer that causes it to output two specific
sets of dimensions?

[miu, 2015/05/14 21:12:26]

Done.  Elaborated on this in code comments.

+  for (const gfx::Size& s : distinct_sizes_seen)

[Wez, 2015/05/14 01:44:55]

nit: auto here?

[miu, 2015/05/14 21:12:26]

No longer applicable since I've replaced this code.

+    EXPECT_NEAR(16.0 / 9.0, static_cast<double>(s.width()) / s.height(), 0.01);
+}
+
 // Test that screen capturer behaves correctly if the source frame size changes
 // and the caller can cope with variable resolution output.
 TEST_F(DesktopCaptureDeviceTest, ScreenResolutionChangeVariableResolution) {
   FakeScreenCapturer* mock_capturer = new FakeScreenCapturer();
 
   CreateScreenCaptureDevice(scoped_ptr<webrtc::DesktopCapturer>(mock_capturer));
 
-  media::VideoCaptureFormat format;
+  media::VideoCaptureFormats formats;
   base::WaitableEvent done_event(false, false);
 
   scoped_ptr<MockDeviceClient> client(new MockDeviceClient());
   EXPECT_CALL(*client, OnError(_)).Times(0);
+  void (media::VideoCaptureFormats::*push_back_method)(
+      const media::VideoCaptureFormat&) =
+      &media::VideoCaptureFormats::push_back;
   EXPECT_CALL(*client, OnIncomingCapturedData(_, _, _, _, _)).WillRepeatedly(
-      DoAll(SaveArg<2>(&format),
+      DoAll(WithArg<2>(Invoke(&formats, push_back_method)),
             InvokeWithoutArgs(&done_event, &base::WaitableEvent::Signal)));
 
   media::VideoCaptureParams capture_params;
-  capture_params.requested_format.frame_size.SetSize(kTestFrameWidth2,
-                                                     kTestFrameHeight2);
+  const gfx::Size high_def_16_by_9(1920, 1080);
+  ASSERT_GE(high_def_16_by_9.width(),
+            std::max(kTestFrameWidth1, kTestFrameWidth2));
+  ASSERT_GE(high_def_16_by_9.height(),
+            std::max(kTestFrameHeight1, kTestFrameHeight2));
+  capture_params.requested_format.frame_size = high_def_16_by_9;
   capture_params.requested_format.frame_rate = kFrameRate;
   capture_params.requested_format.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_params.resolution_change_policy =
+      media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT;
 
   capture_device_->AllocateAndStart(
       capture_params, client.Pass());
 
   // Capture at least three frames, to ensure that the source frame size has
   // changed at least twice while capturing.
   EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
   done_event.Reset();
   EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
   done_event.Reset();
   EXPECT_TRUE(done_event.TimedWait(TestTimeouts::action_max_timeout()));
 
   capture_device_->StopAndDeAllocate();
 
-  EXPECT_EQ(kTestFrameWidth1, format.frame_size.width());
-  EXPECT_EQ(kTestFrameHeight1, format.frame_size.height());
-  EXPECT_EQ(kFrameRate, format.frame_rate);
-  EXPECT_EQ(media::PIXEL_FORMAT_ARGB, format.pixel_format);
+  bool have_seen_frame_size_1 = false;
+  bool have_seen_frame_size_2 = false;
+  bool have_seen_other_frame_size = false;
+  for (const media::VideoCaptureFormat& f : formats) {
+    if (f.frame_size == gfx::Size(kTestFrameWidth1, kTestFrameHeight1))
+      have_seen_frame_size_1 = true;
+    else if (f.frame_size == gfx::Size(kTestFrameWidth2, kTestFrameHeight2))
+      have_seen_frame_size_2 = true;
+    else
+      have_seen_other_frame_size = true;
+    EXPECT_EQ(kFrameRate, f.frame_rate);
+    EXPECT_EQ(media::PIXEL_FORMAT_ARGB, f.pixel_format);
+  }
+  EXPECT_TRUE(have_seen_frame_size_1);
+  EXPECT_TRUE(have_seen_frame_size_2);
+  EXPECT_FALSE(have_seen_other_frame_size);

[Wez, 2015/05/14 01:44:55]

nit: The fixed aspect-ratio test collates a list of the sizes seen and checks
e.g. that there are exactly two of them - you could do the same here, plus test
that the two test frame sizes are contained in the list, and the resulting code
looks like it'd be more concise?

Either way, it seems that using the same basic approach between the two tests
would be valid; the fixed-aspect test should still have predictable output
values, so could use a similar approach to this test.

[miu, 2015/05/14 21:12:26]

Done.

 }
 
 // This test verifies that an unpacked frame is converted to a packed frame.
 TEST_F(DesktopCaptureDeviceTest, UnpackedFrame) {
   FakeScreenCapturer* mock_capturer = new FakeScreenCapturer();
   mock_capturer->set_generate_cropped_frames(true);
   CreateScreenCaptureDevice(scoped_ptr<webrtc::DesktopCapturer>(mock_capturer));
 
   media::VideoCaptureFormat format;
   base::WaitableEvent done_event(false, false);
@@ skipping 72 matching lines @@
             frame_size);
   for (int i = 0; i < output_frame_->size().height(); ++i) {
     EXPECT_EQ(0,
         memcmp(inverted_frame->data() + i * inverted_frame->stride(),
                output_frame_->data() + i * output_frame_->stride(),
                output_frame_->stride()));
   }
 }
 
 }  // namespace content