Add MJPEG support to FakeVideoCaptureDevice

media/capture/video/fake_video_capture_device_factory.h

 // Copyright 2014 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.
 
 #ifndef MEDIA_CAPTURE_VIDEO_FAKE_VIDEO_CAPTURE_DEVICE_FACTORY_H_
 #define MEDIA_CAPTURE_VIDEO_FAKE_VIDEO_CAPTURE_DEVICE_FACTORY_H_
 
 #include "media/capture/video/fake_video_capture_device.h"
 #include "media/capture/video/video_capture_device_factory.h"
 
 namespace media {
 
 // Implementation of VideoCaptureDeviceFactory that creates fake devices
 // that generate test output frames.
 // By default, the factory has one device outputting I420.
 // When increasing the number of devices using set_number_of_devices(), the
 // second device will use Y16, and any devices beyond that will use I420.
 // By default, the delivery mode of all devices is USE_OWN_BUFFERS.
 // This, as well as other parameters, can be changed using command-line flags.
 // See implementation of method ParseCommandLine() for details.
 class CAPTURE_EXPORT FakeVideoCaptureDeviceFactory
     : public VideoCaptureDeviceFactory {
  public:
   FakeVideoCaptureDeviceFactory();
   ~FakeVideoCaptureDeviceFactory() override {}
 
+  static media::VideoPixelFormat GetPixelFormatFromDeviceId(

[mcasas, 2017/02/17 18:35:47]

Should be private, but better than that just keep
it static in fake_video_capture_device_factory.cc
as it was.

[chfremer, 2017/02/22 17:28:42]

The reason I am exposing this here is because the test
FakeVideoCaptureDeviceTest.GetDeviceSupportedFormats needs this to know what
pixel format to expect. If we are not exposing this here, we would have to
hard-code the expectation in the test, which would mean duplication.

[mcasas, 2017/02/22 20:26:32]

Duplication for a test is OK, otherwise we'd be comparing
the implementation against itself, and it would always
work :-)
Also, I'd like tests to fail when someone comes along and
modifies e.g. the pixel format of a given device entry.  That
way they'll have to update it.

[chfremer, 2017/02/23 01:14:27]

Duplication for a test can be problematic. We want to avoid testing
implementations by providing and verifying against an equivalent (duplicate)
second implementation. In this case, the duplicated implementation would be this
simple mapping from device id strings to VideoPixelFormat. As you pointed out,
by exposing this static method and using it in the test, we exclude it from test
coverage. And this is okay, because it is so basic that the only way to test it
would be to compare it to a duplicate.

Why is it an advantage to have tests fail when someone wants to modify the pixel
format of a given device entry? The way I see it, the only thing this does is
making it slightly harder for the person making the change by requiring the same
change in two locations (a bit like a confirmation dialog asking "are you
sure?").

Anyway, since the cost of the duplication is not high in this case, I followed
your suggestion in the latest patch set.

Done.

[mcasas, 2017/02/23 21:46:18]

I like this "are you sure" :-)  and I'm not the only one since
such dialogs are pervasive... think that this code is 
probably going to be modified down the line by other
developers, and seeing tests failing, and forcing
them to update those accordingly, provides an extra
check (a CL with no tests updated means... that there
was no test coverage?).

+      const std::string& device_id);
+
   std::unique_ptr<VideoCaptureDevice> CreateDevice(
       const VideoCaptureDeviceDescriptor& device_descriptor) override;
   void GetDeviceDescriptors(
       VideoCaptureDeviceDescriptors* device_descriptors) override;
   void GetSupportedFormats(
       const VideoCaptureDeviceDescriptor& device_descriptor,
       VideoCaptureFormats* supported_formats) override;
 
   void set_number_of_devices(int number_of_devices) {
     DCHECK(thread_checker_.CalledOnValidThread());
     number_of_devices_ = number_of_devices;
   }
   int number_of_devices() {
     DCHECK(thread_checker_.CalledOnValidThread());
     return number_of_devices_;
   }
 
  private:
   void ParseCommandLine();
 
   int number_of_devices_;
   FakeVideoCaptureDeviceMaker::DeliveryMode delivery_mode_;
   float frame_rate_;
   bool command_line_parsed_ = false;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_CAPTURE_VIDEO_FAKE_VIDEO_CAPTURE_DEVICE_FACTORY_H_