Commit f21fba7d authored by Christian Fremerey's avatar Christian Fremerey Committed by Commit Bot

[Video/Image Capture] Put Windows image capture controls behind feature flag

The drivers for several devices on Windows appear to not properly handle
querying for controls used in the context of Image Capture. These cases lead to
video capture being broken and users seeing only a blank image.

This CL adds a feature flag kImageCaptureControls Image Capture on Windows. 
For now, we disable this feature by default. The feature can be enabled by 
adding --enable-features=ImageCaptureControls to the command-line.

Bug: 722038
Test: Manual tested on Windows using iSpy virtual device.
Change-Id: I654ff72772af10c2d5356d5131ed7ed98e300c1f
Reviewed-on: https://chromium-review.googlesource.com/530076Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Reviewed-by: default avatarEmircan Uysaler <emircan@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480999}
parent e7d3f68a
......@@ -22,6 +22,10 @@ namespace content {
#if defined(OS_WIN)
// These tests are flaky on WebRTC Windows bots: https://crbug.com/633242.
// As a mitigation for https://crbug.com/722038, access to the image capture
// controls has been put behind a feature flag kImageCaptureControls, which is
// disabled by default on Windows. In order to re-activate these tests on
// Windows, this feature must be enabled.
#define MAYBE_GetPhotoCapabilities DISABLED_GetPhotoCapabilities
#define MAYBE_GetPhotoSettings DISABLED_GetPhotoSettings
#define MAYBE_TakePhoto DISABLED_TakePhoto
......
......@@ -288,6 +288,15 @@ const base::Feature kD3D11VideoDecoding{"D3D11VideoDecoding",
const base::Feature kDelayCopyNV12Textures{"DelayCopyNV12Textures",
base::FEATURE_ENABLED_BY_DEFAULT};
// Enables code paths accessing controls exposed by video capture devices in the
// context of capturing still images. Note that several webcam drivers have
// shown issues when accessing these controls, resulting in symptoms such as
// video capture outputting blank images or images with incorrect settings for
// things like zoom, white balance, contrast, focus, etc, see
// https://crbug.com/722038.
const base::Feature kImageCaptureControls{"ImageCaptureControls",
base::FEATURE_DISABLED_BY_DEFAULT};
// Enables H264 HW encode acceleration using Media Foundation for Windows.
const base::Feature kMediaFoundationH264Encoding{
"MediaFoundationH264Encoding", base::FEATURE_ENABLED_BY_DEFAULT};
......
......@@ -129,6 +129,7 @@ MEDIA_EXPORT extern const base::Feature kMediaDrmPersistentLicense;
#if defined(OS_WIN)
MEDIA_EXPORT extern const base::Feature kD3D11VideoDecoding;
MEDIA_EXPORT extern const base::Feature kDelayCopyNV12Textures;
MEDIA_EXPORT extern const base::Feature kImageCaptureControls;
MEDIA_EXPORT extern const base::Feature kMediaFoundationH264Encoding;
#endif // defined(OS_WIN)
......
......@@ -426,7 +426,9 @@ std::unique_ptr<VideoCaptureDevice> VideoCaptureDeviceFactoryWin::CreateDevice(
device.reset();
} else if (device_descriptor.capture_api ==
VideoCaptureApi::WIN_DIRECT_SHOW) {
device.reset(new VideoCaptureDeviceWin(device_descriptor));
device.reset(new VideoCaptureDeviceWin(
device_descriptor,
base::FeatureList::IsEnabled(kImageCaptureControls)));
DVLOG(1) << " DirectShow Device: " << device_descriptor.display_name;
if (!static_cast<VideoCaptureDeviceWin*>(device.get())->Init())
device.reset();
......
......@@ -264,8 +264,10 @@ void VideoCaptureDeviceWin::ScopedMediaType::DeleteMediaType(
}
VideoCaptureDeviceWin::VideoCaptureDeviceWin(
const VideoCaptureDeviceDescriptor& device_descriptor)
const VideoCaptureDeviceDescriptor& device_descriptor,
bool allow_image_capture_controls)
: device_descriptor_(device_descriptor),
allow_image_capture_controls_(allow_image_capture_controls),
state_(kIdle),
white_balance_mode_manual_(false),
exposure_mode_manual_(false) {
......@@ -466,8 +468,13 @@ void VideoCaptureDeviceWin::AllocateAndStart(
client_->OnStarted();
state_ = kCapturing;
if (allow_image_capture_controls_)
InitializeVideoAndCameraControls();
}
void VideoCaptureDeviceWin::InitializeVideoAndCameraControls() {
base::win::ScopedComPtr<IKsTopologyInfo> info;
hr = capture_filter_.CopyTo(info.GetAddressOf());
HRESULT hr = capture_filter_.CopyTo(info.GetAddressOf());
if (FAILED(hr)) {
SetErrorState(FROM_HERE, "Failed to obtain the topology info.", hr);
return;
......@@ -525,6 +532,10 @@ void VideoCaptureDeviceWin::StopAndDeAllocate() {
void VideoCaptureDeviceWin::TakePhoto(TakePhotoCallback callback) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!allow_image_capture_controls_)
return;
// DirectShow has other means of capturing still pictures, e.g. connecting a
// SampleGrabber filter to a PIN_CATEGORY_STILL of |capture_filter_|. This
// way, however, is not widespread and proves too cumbersome, so we just grab
......@@ -535,7 +546,7 @@ void VideoCaptureDeviceWin::TakePhoto(TakePhotoCallback callback) {
void VideoCaptureDeviceWin::GetPhotoState(GetPhotoStateCallback callback) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!camera_control_ || !video_control_)
if (!camera_control_ || !video_control_ || !allow_image_capture_controls_)
return;
auto photo_capabilities = mojom::PhotoState::New();
......@@ -627,7 +638,7 @@ void VideoCaptureDeviceWin::SetPhotoOptions(
VideoCaptureDevice::SetPhotoOptionsCallback callback) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!camera_control_ || !video_control_)
if (!camera_control_ || !video_control_ || !allow_image_capture_controls_)
return;
HRESULT hr;
......
......@@ -65,8 +65,8 @@ class VideoCaptureDeviceWin : public VideoCaptureDevice,
static VideoPixelFormat TranslateMediaSubtypeToPixelFormat(
const GUID& sub_type);
explicit VideoCaptureDeviceWin(
const VideoCaptureDeviceDescriptor& device_descriptor);
VideoCaptureDeviceWin(const VideoCaptureDeviceDescriptor& device_descriptor,
bool allow_image_capture_controls);
~VideoCaptureDeviceWin() override;
// Opens the device driver for this device.
bool Init();
......@@ -89,6 +89,8 @@ class VideoCaptureDeviceWin : public VideoCaptureDevice,
// User needs to recover by destroying the object.
};
void InitializeVideoAndCameraControls();
// Implements SinkFilterObserver.
void FrameReceived(const uint8_t* buffer,
int length,
......@@ -102,6 +104,7 @@ class VideoCaptureDeviceWin : public VideoCaptureDevice,
HRESULT hr);
const VideoCaptureDeviceDescriptor device_descriptor_;
const bool allow_image_capture_controls_;
InternalState state_;
std::unique_ptr<VideoCaptureDevice::Client> client_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment