Skip to content

Commit

Permalink
Revert "Reland: Win video capture: use IMFCaptureEngine for Media Fou…
Browse files Browse the repository at this point in the history 
…ndation"

This reverts commit 7c1a115.

Reason for revert: I think this CL is still breaking Win8 and Win10:
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win10%20Tester/10071
https://ci.chromium.org/buildbot/chromium.webrtc.fyi/Win8%20Tester/3085

Original change's description:
> Reland: Win video capture: use IMFCaptureEngine for Media Foundation
> 
> Fixes for reland:
> - "Win8 Tester" browser_tests_functional failure
> - "Win8 Tester" capture_unittests failure
> 
> Original description:
> - Full rewrite of the MediaFoundation implementation video part to use
> IMFCaptureEngine
> - Implementation of takePhoto, setPhotoOptions and getPhotoCapabilities
> - takePhoto triggers a still image capture with the highest available
> resolution without stopping the video stream thanks to IMFCaptureEngine
> 
> TEST=adapted video_capture_device_unittest.cc and
> webrtc_image_capture_browsertest.cc; launch Chrome with
> --force-mediafoundation on Win8+ and capture video using
> e.g. https://webrtc.github.io/samples/src/content/getusermedia/gum/
> 
> R=​mcasas@chromium.org
> 
> Bug: 730068
> Change-Id: I3835a48ca8516fc66a6a8394b3709d4dea862f89
> Reviewed-on: https://chromium-review.googlesource.com/734042
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Miguel Casas <mcasas@chromium.org>
> Reviewed-by: Christian Fremerey <chfremer@chromium.org>
> Cr-Original-Commit-Position: refs/heads/master@{#521435}
> Reviewed-on: https://chromium-review.googlesource.com/810766
> Cr-Commit-Position: refs/heads/master@{#524417}

TBR=mcasas@chromium.org,chfremer@chromium.org,alaoui.rda@gmail.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 730068
Change-Id: I04e276e3b9f42e7f8ec3a60fd8b9439788ad994f
Reviewed-on: https://chromium-review.googlesource.com/831588
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524686}
  • Loading branch information
Mirko Bonadei authored and Commit Bot committed Dec 18, 2017
1 parent ab49b2a commit c7e93d2
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 869 deletions.
2 changes: 0 additions & 2 deletions AUTHORS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,6 @@ Raveendra Karu <r.karu@samsung.com>
Ravi Nanjundappa <nravi.n@samsung.com>
Ravi Phaneendra Kasibhatla <r.kasibhatla@samsung.com>
Ravi Phaneendra Kasibhatla <ravi.kasibhatla@motorola.com>
Réda Housni Alaoui <alaoui.rda@gmail.com>
Refael Ackermann <refack@gmail.com>
Renata Hodovan <rhodovan.u-szeged@partner.samsung.com>
Rene Bolldorf <rb@radix.io>
Expand Down Expand Up @@ -914,7 +913,6 @@ BlackBerry Limited <*@blackberry.com>
Canonical Limited <*@canonical.com>
Code Aurora Forum <*@codeaurora.org>
Comodo CA Limited
Cosium <*@cosium.com>
Endless Mobile, Inc. <*@endlessm.com>
Facebook, Inc. <*@fb.com>
Facebook, Inc. <*@oculus.com>
Expand Down
43 changes: 9 additions & 34 deletions content/browser/webrtc/webrtc_image_capture_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,26 +46,13 @@ namespace {

static const char kImageCaptureHtmlFile[] = "/media/image_capture_test.html";

enum class Camera {
FAKE,
DEFAULT,
#if defined(OS_WIN)
// Media Foundation is only available in Windows versions >= 7, below that the
// following flag has no effect
WIN_MEDIA_FOUNDATION
#endif
};

// TODO(mcasas): enable real-camera tests by disabling the Fake Device for
// platforms where the ImageCaptureCode is landed, https://crbug.com/656810
static struct TargetCamera {
Camera camera;
} const kTargetCameras[] = {{Camera::FAKE},
bool use_fake;
} const kTargetCameras[] = {{true},
#if defined(OS_LINUX) || defined(OS_MACOSX) || defined(OS_ANDROID)
{Camera::DEFAULT},
#endif
#if defined(OS_WIN)
{Camera::WIN_MEDIA_FOUNDATION}
{false}
#endif
};

Expand Down Expand Up @@ -158,31 +145,19 @@ class WebRtcImageCaptureSucceedsBrowserTest
void SetUpCommandLine(base::CommandLine* command_line) override {
WebRtcImageCaptureBrowserTestBase::SetUpCommandLine(command_line);

switch (std::get<0>(GetParam()).camera) {
case Camera::FAKE:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kUseFakeDeviceForMediaStream);
ASSERT_TRUE(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kUseFakeDeviceForMediaStream));
break;
#if defined(OS_WIN)
case Camera::WIN_MEDIA_FOUNDATION:
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kForceMediaFoundationVideoCapture);
ASSERT_TRUE(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceMediaFoundationVideoCapture));
break;
#endif
default:
break;
if (std::get<0>(GetParam()).use_fake) {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kUseFakeDeviceForMediaStream);
ASSERT_TRUE(base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kUseFakeDeviceForMediaStream));
}
}

bool RunImageCaptureTestCase(const std::string& command) override {
// TODO(chfremer): Enable test cases using the video capture service with
// real cameras as soon as root cause for https://crbug.com/733582 is
// understood and resolved.
if ((std::get<0>(GetParam()).camera != Camera::FAKE) &&
if ((!std::get<0>(GetParam()).use_fake) &&
(std::get<1>(GetParam()).use_video_capture_service)) {
LOG(INFO) << "Skipping this test case";
return true;
Expand Down
2 changes: 1 addition & 1 deletion content/test/data/media/image_capture_test.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<body>
<script type="text/javascript" src="webrtc_test_utilities.js"></script>
<script>
const WIDTH = 640;
const WIDTH = 320;
/** @const */ var CONSTRAINTS = { width: { max : WIDTH } };

// Returns a Promise resolved with |object| after a delay of |delayInMs|.
Expand Down
154 changes: 13 additions & 141 deletions media/capture/video/video_capture_device_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@
#include "testing/gtest/include/gtest/gtest.h"

#if defined(OS_WIN)
#include <mfcaptureengine.h>
#include "base/win/scoped_com_initializer.h"
#include "base/win/windows_version.h" // For fine-grained suppression.
#include "media/capture/video/win/video_capture_device_factory_win.h"
#include "media/capture/video/win/video_capture_device_mf_win.h"
#endif

#if defined(OS_MACOSX)
Expand Down Expand Up @@ -90,16 +88,9 @@
#define MAYBE_GetPhotoState DISABLED_GetPhotoState
#endif

// Wrap the TEST_P macro into another one to allow to preprocess |test_name|
// macros. Needed until https://github.com/google/googletest/issues/389 is
// fixed.
#define WRAPPED_TEST_P(test_case_name, test_name) \
TEST_P(test_case_name, test_name)

using ::testing::_;
using ::testing::Invoke;
using ::testing::SaveArg;
using ::testing::Return;

namespace media {
namespace {
Expand All @@ -125,35 +116,6 @@ static bool IsDeviceUsableForTesting(
};
#endif

enum VideoCaptureImplementationTweak {
NONE,
#if defined(OS_WIN)
WIN_MEDIA_FOUNDATION
#endif
};

#if defined(OS_WIN)
class MockMFPhotoCallback final : public IMFCaptureEngineOnSampleCallback {
public:
~MockMFPhotoCallback() {}

MOCK_METHOD2(DoQueryInterface, HRESULT(REFIID, void**));
MOCK_METHOD0(DoAddRef, ULONG(void));
MOCK_METHOD0(DoRelease, ULONG(void));
MOCK_METHOD1(DoOnSample, HRESULT(IMFSample*));

STDMETHOD(QueryInterface)(REFIID riid, void** object) override {
return DoQueryInterface(riid, object);
}

STDMETHOD_(ULONG, AddRef)() override { return DoAddRef(); }

STDMETHOD_(ULONG, Release)() override { return DoRelease(); }

STDMETHOD(OnSample)(IMFSample* sample) override { return DoOnSample(sample); }
};
#endif

class MockVideoCaptureClient : public VideoCaptureDevice::Client {
public:
MOCK_METHOD0(DoReserveOutputBuffer, void(void));
Expand Down Expand Up @@ -299,19 +261,7 @@ testing::Environment* const mojo_test_env =

} // namespace

class VideoCaptureDeviceTest
: public testing::TestWithParam<
std::tuple<gfx::Size, VideoCaptureImplementationTweak>> {
public:
#if defined(OS_WIN)
scoped_refptr<IMFCaptureEngineOnSampleCallback> CreateMockPhotoCallback(
MockMFPhotoCallback* mock_photo_callback,
VideoCaptureDevice::TakePhotoCallback callback,
VideoCaptureFormat format) {
return scoped_refptr<IMFCaptureEngineOnSampleCallback>(mock_photo_callback);
}
#endif

class VideoCaptureDeviceTest : public testing::TestWithParam<gfx::Size> {
protected:
typedef VideoCaptureDevice::Client Client;

Expand Down Expand Up @@ -344,23 +294,13 @@ class VideoCaptureDeviceTest
static_cast<VideoCaptureDeviceFactoryAndroid*>(
video_capture_device_factory_.get())
->ConfigureForTesting();
#elif defined(OS_WIN)
static_cast<VideoCaptureDeviceFactoryWin*>(
video_capture_device_factory_.get())
->set_use_media_foundation_for_testing(UseWinMediaFoundation());
#endif
EXPECT_CALL(*video_capture_client_, DoReserveOutputBuffer()).Times(0);
EXPECT_CALL(*video_capture_client_, DoOnIncomingCapturedBuffer()).Times(0);
EXPECT_CALL(*video_capture_client_, DoOnIncomingCapturedVideoFrame())
.Times(0);
}

#if defined(OS_WIN)
bool UseWinMediaFoundation() {
return std::get<1>(GetParam()) == WIN_MEDIA_FOUNDATION;
}
#endif

void ResetWithNewClient() {
video_capture_client_.reset(new MockVideoCaptureClient(base::Bind(
&VideoCaptureDeviceTest::OnFrameCaptured, base::Unretained(this))));
Expand Down Expand Up @@ -467,7 +407,7 @@ class VideoCaptureDeviceTest
#define MAYBE_OpenInvalidDevice OpenInvalidDevice
#endif
// Tries to allocate an invalid device and verifies it doesn't work.
WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
TEST_F(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
VideoCaptureDeviceDescriptor invalid_descriptor;
invalid_descriptor.device_id = "jibberish";
invalid_descriptor.display_name = "jibberish";
Expand Down Expand Up @@ -499,12 +439,12 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_OpenInvalidDevice) {
}

// Allocates the first enumerated device, and expects a frame.
WRAPPED_TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
const auto descriptor = FindUsableDeviceDescriptor();
if (!descriptor)
return;

const gfx::Size& size = std::get<0>(GetParam());
const gfx::Size& size = GetParam();
if (!IsCaptureSizeSupported(*descriptor, size))
return;
const int width = size.width();
Expand Down Expand Up @@ -534,22 +474,14 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, CaptureWithSize) {
}

const gfx::Size kCaptureSizes[] = {gfx::Size(640, 480), gfx::Size(1280, 720)};
const VideoCaptureImplementationTweak kCaptureImplementationTweaks[] = {
NONE,
#if defined(OS_WIN)
WIN_MEDIA_FOUNDATION
#endif
};

INSTANTIATE_TEST_CASE_P(
VideoCaptureDeviceTests,
VideoCaptureDeviceTest,
testing::Combine(testing::ValuesIn(kCaptureSizes),
testing::ValuesIn(kCaptureImplementationTweaks)));
INSTANTIATE_TEST_CASE_P(VideoCaptureDeviceTests,
VideoCaptureDeviceTest,
testing::ValuesIn(kCaptureSizes));

// Allocates a device with an uncommon resolution and verifies frames are
// captured in a close, much more typical one.
WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
TEST_F(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
const auto descriptor = FindUsableDeviceDescriptor();
if (!descriptor)
return;
Expand All @@ -576,7 +508,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_AllocateBadSize) {
}

// Cause hangs on Windows, Linux. Fails Android. https://crbug.com/417824
WRAPPED_TEST_P(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
TEST_F(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
const auto descriptor = FindUsableDeviceDescriptor();
if (!descriptor)
return;
Expand Down Expand Up @@ -620,7 +552,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, DISABLED_ReAllocateCamera) {
}

// Starts the camera in 720p to try and capture MJPEG format.
WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
TEST_F(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
GetFirstDeviceDescriptorSupportingPixelFormat(PIXEL_FORMAT_MJPEG);
if (!device_descriptor) {
Expand Down Expand Up @@ -657,7 +589,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_CaptureMjpeg) {
device->StopAndDeAllocate();
}

WRAPPED_TEST_P(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
TEST_F(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
// Use PIXEL_FORMAT_MAX to iterate all device names for testing
// GetDeviceSupportedFormats().
std::unique_ptr<VideoCaptureDeviceDescriptor> device_descriptor =
Expand All @@ -669,7 +601,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {

// Starts the camera and verifies that a photo can be taken. The correctness of
// the photo is enforced by MockImageCaptureClient.
WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
TEST_F(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
const auto descriptor = FindUsableDeviceDescriptor();
if (!descriptor)
return;
Expand Down Expand Up @@ -718,7 +650,7 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
}

// Starts the camera and verifies that the photo capabilities can be retrieved.
WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_GetPhotoState) {
TEST_F(VideoCaptureDeviceTest, MAYBE_GetPhotoState) {
const auto descriptor = FindUsableDeviceDescriptor();
if (!descriptor)
return;
Expand Down Expand Up @@ -769,64 +701,4 @@ WRAPPED_TEST_P(VideoCaptureDeviceTest, MAYBE_GetPhotoState) {
device->StopAndDeAllocate();
}

#if defined(OS_WIN)
// Verifies that the photo callback is correctly released by MediaFoundation
WRAPPED_TEST_P(VideoCaptureDeviceTest, CheckPhotoCallbackRelease) {
if (!UseWinMediaFoundation())
return;

std::unique_ptr<VideoCaptureDeviceDescriptor> descriptor =
GetFirstDeviceDescriptorSupportingPixelFormat(PIXEL_FORMAT_MJPEG);
if (!descriptor) {
DVLOG(1) << "No usable media foundation device descriptor. Exiting test.";
return;
}

EXPECT_CALL(*video_capture_client_, OnError(_, _)).Times(0);
EXPECT_CALL(*video_capture_client_, OnStarted());

std::unique_ptr<VideoCaptureDevice> device(
video_capture_device_factory_->CreateDevice(*descriptor));
ASSERT_TRUE(device);

VideoCaptureParams capture_params;
capture_params.requested_format.frame_size.SetSize(320, 240);
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_));

if (!static_cast<VideoCaptureDeviceMFWin*>(device.get())
->get_use_photo_stream_to_take_photo_for_testing()) {
DVLOG(1) << "The device is not using the MediaFoundation photo callback. "
"Exiting test.";
device->StopAndDeAllocate();
return;
}

MockMFPhotoCallback* callback = new MockMFPhotoCallback();
EXPECT_CALL(*callback, DoQueryInterface(_, _)).WillRepeatedly(Return(S_OK));
EXPECT_CALL(*callback, DoAddRef()).WillOnce(Return(1U));
EXPECT_CALL(*callback, DoRelease()).WillOnce(Return(1U));
EXPECT_CALL(*callback, DoOnSample(_)).WillOnce(Return(S_OK));
static_cast<VideoCaptureDeviceMFWin*>(device.get())
->set_create_mf_photo_callback_for_testing(
base::BindRepeating(&VideoCaptureDeviceTest::CreateMockPhotoCallback,
base::Unretained(this), callback));

VideoCaptureDevice::TakePhotoCallback scoped_callback = base::BindOnce(
&MockImageCaptureClient::DoOnPhotoTaken, image_capture_client_);

base::RunLoop run_loop;
base::RepeatingClosure quit_closure =
BindToCurrentLoop(run_loop.QuitClosure());
EXPECT_CALL(*image_capture_client_.get(), OnCorrectPhotoTaken())
.WillOnce(RunClosure(quit_closure));

device->TakePhoto(std::move(scoped_callback));
run_loop.Run();

device->StopAndDeAllocate();
}
#endif

}; // namespace media
Loading

0 comments on commit c7e93d2

Please sign in to comment.