Video capture Android: enable unittests for Camera2 API

media/capture/video/video_capture_device_unittest.cc

Index: media/capture/video/video_capture_device_unittest.cc
diff --git a/media/capture/video/video_capture_device_unittest.cc b/media/capture/video/video_capture_device_unittest.cc
index a684a3b7e5b3e8c5bcee5af06b32e0fe8823709f..8e59a526d9251d0e73539d99deb4dab848062eb7 100644
--- a/media/capture/video/video_capture_device_unittest.cc
+++ b/media/capture/video/video_capture_device_unittest.cc
@@ -39,6 +39,7 @@
 #if defined(OS_ANDROID)
 #include "base/android/jni_android.h"
 #include "media/capture/video/android/video_capture_device_android.h"
+#include "media/capture/video/android/video_capture_device_factory_android.h"
 #endif
 
 #if defined(OS_MACOSX)
@@ -52,14 +53,8 @@
 #define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto DISABLED_TakePhoto
 #elif defined(OS_ANDROID)
-// TODO(wjia): enable those tests on Android.
-// On Android, native camera (JAVA) delivers frames on UI thread which is the
-// main thread for tests. This results in no frame received by
-// VideoCaptureAndroid.
-#define MAYBE_AllocateBadSize DISABLED_AllocateBadSize
-#define DeAllocateCameraWhileRunning DISABLED_DeAllocateCameraWhileRunning
-#define DeAllocateCameraWhileRunning DISABLED_DeAllocateCameraWhileRunning
-#define MAYBE_CaptureMjpeg DISABLED_CaptureMjpeg
+#define MAYBE_AllocateBadSize AllocateBadSize
+#define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto DISABLED_TakePhoto
 #elif defined(OS_LINUX)
 // AllocateBadSize will hang when a real camera is attached and if more than one
@@ -104,7 +99,7 @@ class MockVideoCaptureClient : public VideoCaptureDevice::Client {
                               base::TimeTicks reference_time,
                               base::TimeDelta timestamp) override {
     ASSERT_GT(length, 0);
-    ASSERT_TRUE(data != NULL);
+    ASSERT_TRUE(!!data);

[jbudorick, 2016/09/01 18:46:41]

nit: what's with this change? !! appears to be very uncommon in chromium
tests...

[mcasas, 2016/09/01 20:52:37]

Yeah, my bad; this comes from msvc compiler
having troubles with if (bla) where bla is
a pointer. If the bots are happy, so am I.

     main_thread_->PostTask(FROM_HERE, base::Bind(frame_cb_, format));
   }
 
@@ -168,12 +163,12 @@ class MockImageCaptureClient : public base::RefCounted<MockImageCaptureClient> {
 class DeviceEnumerationListener
     : public base::RefCounted<DeviceEnumerationListener> {
  public:
-  MOCK_METHOD1(OnEnumerateDeviceDescriptorsCallbackPtr,
+  MOCK_METHOD1(DoOnEnumerateDeviceDescriptors,
                void(VideoCaptureDeviceDescriptors* device_descriptors));
   // GMock doesn't support move-only arguments, so we use this forward method.
-  void OnEnumerateDeviceDescriptorsCallback(
+  void OnEnumerateDeviceDescriptors(
       std::unique_ptr<VideoCaptureDeviceDescriptors> device_descriptors) {
-    OnEnumerateDeviceDescriptorsCallbackPtr(device_descriptors.release());
+    DoOnEnumerateDeviceDescriptors(device_descriptors.release());
   }
 
  private:
@@ -201,6 +196,10 @@ class VideoCaptureDeviceTest : public testing::TestWithParam<gfx::Size> {
 #if defined(OS_ANDROID)
     VideoCaptureDeviceAndroid::RegisterVideoCaptureDevice(
         base::android::AttachCurrentThread());
+
+    static_cast<VideoCaptureDeviceFactoryAndroid*>(
+        video_capture_device_factory_.get())
+        ->ConfigureForTesting();
 #endif
 #if defined(OS_MACOSX)
     AVFoundationGlue::InitializeAVFoundation();
@@ -226,17 +225,35 @@ class VideoCaptureDeviceTest : public testing::TestWithParam<gfx::Size> {
     run_loop_->Run();
   }
 
-  std::unique_ptr<VideoCaptureDeviceDescriptors> EnumerateDeviceDescriptors() {
-    VideoCaptureDeviceDescriptors* device_descriptors;
+  bool EnumerateDeviceDescriptors() {

[jbudorick, 2016/09/01 18:46:41]

Why did you switch this to return a bool rather than just returning a null
unique_ptr or an empty instance of VideoCaptureDeviceDescriptors if any of the
devices are legacy or deprecated? Having device_descriptors_ assigned in here
seems more confusing than the previous implementation.

[mcasas, 2016/09/01 20:52:37]

It didn't make a lot of sense to have a member 
method returning the list just to force the 
individual call sites to update a member 
variable; the member variable had to be tested 
then individually at those call sites, and that
was verbose and cluttered the test intentions.  

Actually, I'll even bring more code into this 
method and return true if there are usable 
|device_desciptors_|, and rename accordingly.

+    VideoCaptureDeviceDescriptors* descriptors = nullptr;
     EXPECT_CALL(*device_enumeration_listener_.get(),
-                OnEnumerateDeviceDescriptorsCallbackPtr(_))
-        .WillOnce(SaveArg<0>(&device_descriptors));
+                DoOnEnumerateDeviceDescriptors(_))
+        .WillOnce(SaveArg<0>(&descriptors));
 
-    video_capture_device_factory_->EnumerateDeviceDescriptors(base::Bind(
-        &DeviceEnumerationListener::OnEnumerateDeviceDescriptorsCallback,
-        device_enumeration_listener_));
+    video_capture_device_factory_->EnumerateDeviceDescriptors(
+        base::Bind(&DeviceEnumerationListener::OnEnumerateDeviceDescriptors,
+                   device_enumeration_listener_));
     base::RunLoop().RunUntilIdle();
-    return std::unique_ptr<VideoCaptureDeviceDescriptors>(device_descriptors);
+
+    if (!descriptors || descriptors->empty())
+      return false;
+    device_descriptors_.reset(descriptors);
+
+#if defined(OS_ANDROID)
+    // Android deprecated/legacy devices capture on a single thread, which is
+    // occupied by the tests, so nothing gets actually delivered.
+    // TODO(mcasas): use those devices' test mode to deliver frames in a
+    // background thread, https://crbug.com/626857
+    for (const auto& descriptor : *descriptors) {
+      if (VideoCaptureDeviceFactoryAndroid::IsLegacyOrDeprecatedDevice(
+              descriptor.device_id)) {
+        return false;
+      }
+    }
+#endif
+
+    return true;
   }
 
   const VideoCaptureFormat& last_format() const { return last_format_; }
@@ -244,25 +261,22 @@ class VideoCaptureDeviceTest : public testing::TestWithParam<gfx::Size> {
   std::unique_ptr<VideoCaptureDeviceDescriptor>
   GetFirstDeviceDescriptorSupportingPixelFormat(
       const VideoPixelFormat& pixel_format) {
-    device_descriptors_ = EnumerateDeviceDescriptors();
-    if (device_descriptors_->empty()) {
-      DVLOG(1) << "No camera available.";
+    if (!EnumerateDeviceDescriptors() || device_descriptors_->empty())
       return std::unique_ptr<VideoCaptureDeviceDescriptor>();
-    }
-    for (const auto& descriptors_iterator : *device_descriptors_) {
+
+    for (const auto& descriptor : *device_descriptors_) {
       VideoCaptureFormats supported_formats;
-      video_capture_device_factory_->GetSupportedFormats(descriptors_iterator,
+      video_capture_device_factory_->GetSupportedFormats(descriptor,
                                                          &supported_formats);
       for (const auto& formats_iterator : supported_formats) {
         if (formats_iterator.pixel_format == pixel_format) {
           return std::unique_ptr<VideoCaptureDeviceDescriptor>(
-              new VideoCaptureDeviceDescriptor(descriptors_iterator));
+              new VideoCaptureDeviceDescriptor(descriptor));
         }
       }
     }
     DVLOG_IF(1, pixel_format != PIXEL_FORMAT_MAX)
-        << "No camera can capture the"
-        << " format: " << VideoPixelFormatToString(pixel_format);
+        << VideoPixelFormatToString(pixel_format);
     return std::unique_ptr<VideoCaptureDeviceDescriptor>();
   }
 
@@ -301,7 +315,7 @@ class VideoCaptureDeviceTest : public testing::TestWithParam<gfx::Size> {
 #else
 #define MAYBE_OpenInvalidDevice OpenInvalidDevice
 #endif
-
+// Tries to allocate an invalid device and verifies it doesn't work.
 TEST_F(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
   VideoCaptureDeviceDescriptor invalid_descriptor;
   invalid_descriptor.device_id = "jibberish";
@@ -318,7 +332,7 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
       video_capture_device_factory_->CreateDevice(invalid_descriptor);
 
 #if !defined(OS_MACOSX)
-  EXPECT_TRUE(device == NULL);
+  EXPECT_FALSE(device);
 #else
   // The presence of the actual device is only checked on AllocateAndStart()
   // and not on creation.
@@ -333,12 +347,10 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
 #endif
 }
 
+// Allocates the first enumerated device, and expects a frame.
 TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
-  device_descriptors_ = EnumerateDeviceDescriptors();
-  if (device_descriptors_->empty()) {
-    VLOG(1) << "No camera available. Exiting test.";
+  if (!EnumerateDeviceDescriptors() || device_descriptors_->empty())
     return;
-  }
 
   const gfx::Size& size = GetParam();
   if (!IsCaptureSizeSupported(device_descriptors_->front(), size))
@@ -350,7 +362,6 @@ TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
       video_capture_device_factory_->CreateDevice(
           device_descriptors_->front()));
   ASSERT_TRUE(device);
-  DVLOG(1) << device_descriptors_->front().device_id;
 
   EXPECT_CALL(*video_capture_client_, OnError(_, _)).Times(0);
 
@@ -359,29 +370,29 @@ TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
   capture_params.requested_format.frame_rate = 30.0f;
   capture_params.requested_format.pixel_format = PIXEL_FORMAT_I420;
   device->AllocateAndStart(capture_params, std::move(video_capture_client_));
-  // Get captured video frames.
+
   WaitForCapturedFrame();
   EXPECT_EQ(last_format().frame_size.width(), width);
   EXPECT_EQ(last_format().frame_size.height(), height);
   if (last_format().pixel_format != PIXEL_FORMAT_MJPEG)
     EXPECT_EQ(size.GetArea(), last_format().frame_size.GetArea());
+  EXPECT_EQ(last_format().frame_rate, 30);
   device->StopAndDeAllocate();
 }
 
-#if !defined(OS_ANDROID)
 const gfx::Size kCaptureSizes[] = {gfx::Size(640, 480), gfx::Size(1280, 720)};
 
 INSTANTIATE_TEST_CASE_P(VideoCaptureDeviceTests,
                         VideoCaptureDeviceTest,
                         testing::ValuesIn(kCaptureSizes));
-#endif
 
+// Allocates a device with an uncommon resolution and verifies frames are
+// captured
+// in a close, much more typical one.
 TEST_F(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
-  device_descriptors_ = EnumerateDeviceDescriptors();
-  if (device_descriptors_->empty()) {
-    VLOG(1) << "No camera available. Exiting test.";
+  if (!EnumerateDeviceDescriptors() || device_descriptors_->empty())
     return;
-  }
+
   std::unique_ptr<VideoCaptureDevice> device(
       video_capture_device_factory_->CreateDevice(
           device_descriptors_->front()));
@@ -403,13 +414,10 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
     EXPECT_EQ(input_size.GetArea(), last_format().frame_size.GetArea());
 }
 
-// Cause hangs on Windows, Linux. Fails Android. http://crbug.com/417824
+// Cause hangs on Windows, Linux. Fails Android. https://crbug.com/417824
 TEST_F(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
-  device_descriptors_ = EnumerateDeviceDescriptors();
-  if (device_descriptors_->empty()) {
-    VLOG(1) << "No camera available. Exiting test.";
+  if (!EnumerateDeviceDescriptors() || device_descriptors_->empty())
     return;
-  }
 
   // First, do a number of very fast device start/stops.
   for (int i = 0; i <= 5; i++) {
@@ -418,11 +426,11 @@ TEST_F(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
         video_capture_device_factory_->CreateDevice(
             device_descriptors_->front()));
     gfx::Size resolution;
-    if (i % 2) {
+    if (i % 2)
       resolution = gfx::Size(640, 480);
-    } else {
+    else
       resolution = gfx::Size(1280, 1024);
-    }
+
     VideoCaptureParams capture_params;
     capture_params.requested_format.frame_size = resolution;
     capture_params.requested_format.frame_rate = 30;
@@ -450,45 +458,19 @@ TEST_F(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
   EXPECT_EQ(last_format().frame_size.height(), 240);
 }
 
-TEST_F(VideoCaptureDeviceTest, DeAllocateCameraWhileRunning) {

[jbudorick, 2016/09/01 18:46:41]

?

[mcasas, 2016/09/01 20:52:37]

Like it says in the CL description...

"Removes DeAllocateCameraWhileRunning because it was
 equivalent to CaptureWithSize test case."

-  device_descriptors_ = EnumerateDeviceDescriptors();
-  if (device_descriptors_->empty()) {
-    VLOG(1) << "No camera available. Exiting test.";
-    return;
-  }
-  std::unique_ptr<VideoCaptureDevice> device(
-      video_capture_device_factory_->CreateDevice(
-          device_descriptors_->front()));
-  ASSERT_TRUE(device);
-
-  EXPECT_CALL(*video_capture_client_, OnError(_, _)).Times(0);
-
-  VideoCaptureParams capture_params;
-  capture_params.requested_format.frame_size.SetSize(640, 480);
-  capture_params.requested_format.frame_rate = 30;
-  capture_params.requested_format.pixel_format = PIXEL_FORMAT_I420;
-  device->AllocateAndStart(capture_params, std::move(video_capture_client_));
-  // Get captured video frames.
-  WaitForCapturedFrame();
-  EXPECT_EQ(last_format().frame_size.width(), 640);
-  EXPECT_EQ(last_format().frame_size.height(), 480);
-  EXPECT_EQ(last_format().frame_rate, 30);
-  device->StopAndDeAllocate();
-}
-
-// Start the camera in 720p to capture MJPEG instead of a raw format.
+// Starts the camera in 720p to try and capture MJPEG format.
 TEST_F(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
   std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
       GetFirstDeviceDescriptorSupportingPixelFormat(PIXEL_FORMAT_MJPEG);
   if (!device_descriptor) {
-    VLOG(1) << "No camera supports MJPEG format. Exiting test.";
+    DVLOG(1) << "No camera supports MJPEG format. Exiting test.";
     return;
   }
 #if defined(OS_WIN)
   base::win::Version version = base::win::GetVersion();
-  VLOG(1) << "Windows version: " << (int)version;
   if (version >= base::win::VERSION_WIN10) {
-    VLOG(1) << "Skipped on Win10: http://crbug.com/570604.";
+    VLOG(1) << "Skipped on Win10: http://crbug.com/570604, current: "
+            << static_cast<int>(version);
     return;
   }
 #endif
@@ -503,7 +485,7 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
   capture_params.requested_format.frame_rate = 30;
   capture_params.requested_format.pixel_format = PIXEL_FORMAT_MJPEG;
   device->AllocateAndStart(capture_params, std::move(video_capture_client_));
-  // Get captured video frames.
+
   WaitForCapturedFrame();
   // Verify we get MJPEG from the device. Not all devices can capture 1280x720
   // @ 30 fps, so we don't care about the exact resolution we get.
@@ -513,7 +495,7 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
   device->StopAndDeAllocate();
 }
 
-TEST_F(VideoCaptureDeviceTest, GetDeviceSupportedFormats) {
+TEST_F(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
   // Use PIXEL_FORMAT_MAX to iterate all device names for testing
   // GetDeviceSupportedFormats().
   std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
@@ -523,13 +505,11 @@ TEST_F(VideoCaptureDeviceTest, GetDeviceSupportedFormats) {
   ASSERT_FALSE(device_descriptor);
 }
 
-// Start the camera and take a photo.
+// Starts the camera and take a photo.
 TEST_F(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
-  device_descriptors_ = EnumerateDeviceDescriptors();
-  if (device_descriptors_->empty()) {
-    VLOG(1) << "No camera available. Exiting test.";
+  if (!EnumerateDeviceDescriptors() || device_descriptors_->empty())
     return;
-  }
+
   std::unique_ptr<VideoCaptureDevice> device(
       video_capture_device_factory_->CreateDevice(
           device_descriptors_->front()));