FakeVideoCaptureDevice: Y16 testing support.

media/capture/video/fake_video_capture_device_factory.cc

Index: media/capture/video/fake_video_capture_device_factory.cc
diff --git a/media/capture/video/fake_video_capture_device_factory.cc b/media/capture/video/fake_video_capture_device_factory.cc
index 38ef6054ecb237593f6ec916050fb036b80aa2b0..1db858fea94aa1813e0e63c012212d38894b297f 100644
--- a/media/capture/video/fake_video_capture_device_factory.cc
+++ b/media/capture/video/fake_video_capture_device_factory.cc
@@ -13,6 +13,16 @@
 #include "build/build_config.h"
 #include "media/base/media_switches.h"
 
+namespace {
+
+media::VideoPixelFormat get_pixelformat(const std::string& device_id) {
+  if (device_id == "/dev/video1")
+    return media::PIXEL_FORMAT_Y16;
+  return media::PIXEL_FORMAT_I420;

[mcasas, 2016/10/25 22:13:12]

Suggestion: send the device index (|n| in l.49

media::VideoPixelFormat GetPixelFormat(int device_index) {
  return device_index == 1 ? media::PIXEL_FORMAT_Y16 : media::PIXEL_FORMAT_I420;
}

Note also the CamelCase naming for methods/functions.
This will also affect l.37.


A larger comment is that if #devices is 2, then we get an 
I420 and an Y16; but with the current code, if we select 
|device-count = 4| then we don't get two "couples" I420-Y16,
but more I420s.  Is this intended/unintended/don't care ... ?

[aleksandar.stojiljkovic, 2016/10/25 23:20:40]

Changed the name but kept the parameter - the reason is usage in l.53 where I
wanted to avoid string to int conversion.
Comment fixed - moved it to GetPixelFormat:
// Factory has one device by default; I420. If there are more, the second device
// is of Y16 format while the rest are I420.
media::VideoPixelFormat GetPixelFormat(const std::string& device_id) {

[mcasas, 2016/10/26 02:03:15]

I meant calling the function with the parameter |n|
of the for loop in l.49, then you don't need to back-convert
string to int, and the comparison in l.21 is trivial and can
be done using the ternary operator in one line.

[aleksandar.stojiljkovic, 2016/10/26 14:46:00]

Sorry, my previous explanation was wrong.
l.53/l.94 - the reason is usage in l.94 where I wanted to avoid string to int
conversion.
Otherwise, ternary operator makes sense here so it is used in new patch.
Previous code was this at because the plan is to add Y8 later (for. e.g.
infrared 8 bit greyscale camera), but let's change that later.

+}
+
+}
+
 namespace media {
 
 // Cap the frame rate command line input to reasonable values.
@@ -20,7 +30,11 @@ static const float kFakeCaptureMinFrameRate = 5.0f;
 static const float kFakeCaptureMaxFrameRate = 60.0f;
 // Default rate if none is specified as part of the command line.
 static const float kFakeCaptureDefaultFrameRate = 20.0f;
+// Cap the device count command line input to reasonable values.
+static const int kFakeCaptureMinDeviceCount = 1;
+static const int kFakeCaptureMaxDeviceCount = 10;
 
+// Factory has two devices by default; I420 and Y16. See also get_pixelformat().
 FakeVideoCaptureDeviceFactory::FakeVideoCaptureDeviceFactory()
     : number_of_devices_(1),
       fake_vcd_ownership_(FakeVideoCaptureDevice::BufferOwnership::OWN_BUFFERS),
@@ -35,8 +49,8 @@ std::unique_ptr<VideoCaptureDevice> FakeVideoCaptureDeviceFactory::CreateDevice(
   for (int n = 0; n < number_of_devices_; ++n) {
     std::string possible_id = base::StringPrintf("/dev/video%d", n);
     if (device_descriptor.device_id.compare(possible_id) == 0) {
-      return std::unique_ptr<VideoCaptureDevice>(
-          new FakeVideoCaptureDevice(fake_vcd_ownership_, frame_rate_));
+      return std::unique_ptr<VideoCaptureDevice>(new FakeVideoCaptureDevice(
+          fake_vcd_ownership_, frame_rate_, get_pixelformat(possible_id)));
     }
   }
   return std::unique_ptr<VideoCaptureDevice>();
@@ -46,6 +60,9 @@ void FakeVideoCaptureDeviceFactory::GetDeviceDescriptors(
     VideoCaptureDeviceDescriptors* device_descriptors) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(device_descriptors->empty());
+
+  parse_command_line();
+
   for (int n = 0; n < number_of_devices_; ++n) {
     device_descriptors->emplace_back(base::StringPrintf("fake_device_%d", n),
                                      base::StringPrintf("/dev/video%d", n),
@@ -66,21 +83,25 @@ void FakeVideoCaptureDeviceFactory::GetSupportedFormats(
     const VideoCaptureDeviceDescriptor& device_descriptor,
     VideoCaptureFormats* supported_formats) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  const gfx::Size supported_sizes[] = {gfx::Size(320, 240),
+  const gfx::Size supported_sizes[] = {gfx::Size(96, 96),  // To test cubemap.
+                                       gfx::Size(320, 240),
                                        gfx::Size(640, 480),
                                        gfx::Size(1280, 720),
                                        gfx::Size(1920, 1080)};
   supported_formats->clear();
   for (const auto& size : supported_sizes) {
-    supported_formats->push_back(
-        VideoCaptureFormat(size, frame_rate_, media::PIXEL_FORMAT_I420));
+    supported_formats->push_back(VideoCaptureFormat(
+        size, frame_rate_, get_pixelformat(device_descriptor.device_id)));
   }
 }
 
 // Optional comma delimited parameters to the command line can specify buffer
-// ownership, buffer planarity, and the fake video device FPS.
-// Examples: "ownership=client, planarity=triplanar, fps=60" "fps=30"
+// ownership, device count, and the fake video devices FPS.
+// Examples: "ownership=client, device-count=2, fps=60" "fps=30"
 void FakeVideoCaptureDeviceFactory::parse_command_line() {
+  if (command_line_parsed_)
+    return;
+  command_line_parsed_ = true;
   const std::string option =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kUseFakeDeviceForMediaStream);
@@ -109,6 +130,15 @@ void FakeVideoCaptureDeviceFactory::parse_command_line() {
             std::max(kFakeCaptureMinFrameRate, static_cast<float>(fps));
         frame_rate_ = std::min(kFakeCaptureMaxFrameRate, frame_rate_);
       }
+    } else if (base::EqualsCaseInsensitiveASCII(param.front(),
+                                                "device-count")) {
+      unsigned count = 0;

[mcasas, 2016/10/25 22:13:12]

unsigned int? I think it's uncommon to see just |unsigned| 
in Cr codebase.

[aleksandar.stojiljkovic, 2016/10/25 23:20:40]

Done.

+      if (base::StringToUint(param.back(), &count)) {
+        number_of_devices_ =
+            std::min(kFakeCaptureMaxDeviceCount, static_cast<int>(count));
+        number_of_devices_ = std::max(kFakeCaptureMinDeviceCount,
+                                      static_cast<int>(number_of_devices_));

[mcasas, 2016/10/25 22:13:12]

One line less if we do:

    number_of_devices_ =
        std::min(kFakeCaptureMaxDeviceCount,
                 std::max(kFakeCaptureMinDeviceCount, static_cast<int>(count)));

[aleksandar.stojiljkovic, 2016/10/25 23:20:40]

Done.

+      }
     }
   }
 }