From f186834e01cfc51c8a7e6044bd518a5ca6ba4fc0 Mon Sep 17 00:00:00 2001
From: "engedy@chromium.org"
<engedy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 21 Mar 2014 15:06:32 +0000
Subject: Revert 258566 "Consolidate camera supported formats in VideoCapt..."
Cause: perf regression -- increased number of static initializers.
> Consolidate camera supported formats in VideoCaptureManager after their enumeration.
>
> Currently VideoCaptureManager holds a list of camera
> supported formats that include repeated resolution-
> frame rate pairs with different pixel formats. Since
> the renderer does not understand pixel formats and
> the decision about which one to use is taken at the
> VideoCaptureDevice level, this info can be trimmed,
> and a consolidated list of available resolutions with
> a single frame rate is calculated on enumeration.
>
> The list of supported formats is ordered using
> std::sort() in increasing frame_size, then
> decreasing frame_rate. All the duplicated
> frame_size are subsequently removed via
> std::unique(), effectively keeping the one with
> highest frame_rate. All remaining formats are
> marked as pixel_format I420.
>
> FakeVCD gets a |g_internal_supported_formats| to
> intervene in its supported format enumeration
> process. This is used in the VCM unit tests to
> feed a precooked list of formats to check that
> the consolidation algorithm produces the
> expected result.
>
> BUG=309554
>
> Review URL: https://codereview.chromium.org/202003002
TBR=mcasas@chromium.org
Review URL: https://codereview.chromium.org/208263003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258584 0039d316-1c4b-4281-b951-d872f2087c98
---
.../renderer_host/media/video_capture_manager.cc | 39 ----------
.../media/video_capture_manager_unittest.cc | 85 ----------------------
media/video/capture/fake_video_capture_device.cc | 13 +---
media/video/capture/fake_video_capture_device.h | 3 -
4 files changed, 1 insertion(+), 139 deletions(-)
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index 82e3215..54ad96e 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -33,44 +33,6 @@
#endif
#endif
-namespace {
-
-// Compares two VideoCaptureFormat by checking smallest frame_size area, then
-// by _largest_ frame_rate. Used to order a VideoCaptureFormats vector.
-bool IsCaptureFormatSmaller(const media::VideoCaptureFormat& format1,
- const media::VideoCaptureFormat& format2) {
- if (format1.frame_size.GetArea() == format2.frame_size.GetArea())
- return format1.frame_rate > format2.frame_rate;
- return format1.frame_size.GetArea() < format2.frame_size.GetArea();
-}
-
-bool IsCaptureFormatSizeEqual(const media::VideoCaptureFormat& format1,
- const media::VideoCaptureFormat& format2) {
- return format1.frame_size.GetArea() == format2.frame_size.GetArea();
-}
-
-// This function receives a list of capture formats, removes duplicated
-// resolutions while keeping the highest frame rate for each, and forcing I420
-// pixel format.
-void ConsolidateCaptureFormats(media::VideoCaptureFormats* formats) {
- if (formats->empty())
- return;
- std::sort(formats->begin(), formats->end(), IsCaptureFormatSmaller);
- // Due to the ordering imposed, the largest frame_rate is kept while removing
- // duplicated resolutions.
- media::VideoCaptureFormats::iterator last =
- std::unique(formats->begin(), formats->end(), IsCaptureFormatSizeEqual);
- formats->erase(last, formats->end());
- // Mark all formats as I420, since this is what the renderer side will get
- // anyhow: the actual pixel format is decided at the device level.
- for (media::VideoCaptureFormats::iterator it = formats->begin();
- it != formats->end(); ++it) {
- it->pixel_format = media::PIXEL_FORMAT_I420;
- }
-}
-
-} // namespace
-
namespace content {
VideoCaptureManager::DeviceEntry::DeviceEntry(
@@ -497,7 +459,6 @@ VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread(
*it, &(device_info.supported_formats));
break;
}
- ConsolidateCaptureFormats(&device_info.supported_formats);
new_devices_info_cache.push_back(device_info);
}
return new_devices_info_cache;
diff --git a/content/browser/renderer_host/media/video_capture_manager_unittest.cc b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
index f82f9ce..a24d2c1 100644
--- a/content/browser/renderer_host/media/video_capture_manager_unittest.cc
+++ b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
@@ -27,36 +27,6 @@ using ::testing::InSequence;
using ::testing::Return;
using ::testing::SaveArg;
-namespace {
-
-// Compose a list of some resolutions, each with some frame rates and I420
-// pixel format, then repeat the same sequence with MJPEG format.
-void GetListOfSupportedFormatsForTest(media::VideoCaptureFormats* formats) {
- const gfx::Size supported_sizes[] = {gfx::Size(352, 288),
- gfx::Size(176, 144),
- gfx::Size(960, 720)};
- const size_t supported_sizes_length = arraysize(supported_sizes);
- const int supported_frame_rates[] = {10, 25, 16, 29};
- const size_t supported_frame_rates_length = arraysize(supported_frame_rates);
- formats->clear();
- for (size_t i = 0; i < supported_sizes_length; ++i) {
- for (size_t j = 0; j < supported_frame_rates_length; ++j) {
- formats->push_back(media::VideoCaptureFormat(supported_sizes[i],
- supported_frame_rates[j],
- media::PIXEL_FORMAT_I420));
- }
- }
- for (size_t i = 0; i < supported_sizes_length; ++i) {
- for (size_t j = 0; j < supported_frame_rates_length; ++j) {
- formats->push_back(media::VideoCaptureFormat(supported_sizes[i],
- supported_frame_rates[j],
- media::PIXEL_FORMAT_MJPEG));
- }
- }
-}
-
-} // namespace
-
namespace content {
// Listener class used to track progress of VideoCaptureManager test.
@@ -482,59 +452,4 @@ TEST_F(VideoCaptureManagerTest, CloseWithoutStop) {
vcm_->Unregister();
}
-// Test that reordering the supported formats of the device produces what we
-// expect on a particular test list fed via GetListOfSupportedFormatsForTest().
-TEST_F(VideoCaptureManagerTest, ConsolidateCaptureFormats) {
- StreamDeviceInfoArray devices;
- int video_session_id = 0;
- media::VideoCaptureFormats supported_formats;
- supported_formats.clear();
-
- // Populate a test-specific supported formats list and copy it into the
- // FakeVideoCaptureDevice.
- GetListOfSupportedFormatsForTest(&supported_formats);
- media::FakeVideoCaptureDevice::SetSupportedFormats(supported_formats);
-
- InSequence s;
- EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
- .WillOnce(SaveArg<1>(&devices));
- vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
- message_loop_->RunUntilIdle();
-
- EXPECT_CALL(*listener_, Opened(MEDIA_DEVICE_VIDEO_CAPTURE, _));
- video_session_id = vcm_->Open(devices.front());
- message_loop_->RunUntilIdle();
-
- supported_formats.clear();
- EXPECT_TRUE(
- vcm_->GetDeviceSupportedFormats(video_session_id, &supported_formats));
-
- // Check that the returned size and contents are exactly what we feed from the
- // test supported formats populate callback, reordered and consolidated. The
- // consolidated format list should not include duplicated resolutions, only
- // the max frame rate per resolution and only I420 pixel format.
- ASSERT_EQ(supported_formats.size(), 3u);
- EXPECT_EQ(supported_formats[0].frame_size.width(), 176);
- EXPECT_EQ(supported_formats[0].frame_size.height(), 144);
- EXPECT_EQ(supported_formats[0].frame_rate, 29);
- EXPECT_EQ(supported_formats[0].pixel_format, media::PIXEL_FORMAT_I420);
- EXPECT_EQ(supported_formats[1].frame_size.width(), 352);
- EXPECT_EQ(supported_formats[1].frame_size.height(), 288);
- EXPECT_EQ(supported_formats[1].frame_rate, 29);
- EXPECT_EQ(supported_formats[1].pixel_format, media::PIXEL_FORMAT_I420);
- EXPECT_EQ(supported_formats[2].frame_size.width(), 960);
- EXPECT_EQ(supported_formats[2].frame_size.height(), 720);
- EXPECT_EQ(supported_formats[2].frame_rate, 29);
- EXPECT_EQ(supported_formats[2].pixel_format, media::PIXEL_FORMAT_I420);
-
- // Clean up the test supported_formats inside FakeVideoCaptureDevice.
- supported_formats.clear();
- media::FakeVideoCaptureDevice::SetSupportedFormats(supported_formats);
-
- EXPECT_CALL(*listener_, Closed(MEDIA_DEVICE_VIDEO_CAPTURE, _));
- vcm_->Close(video_session_id);
- message_loop_->RunUntilIdle();
- vcm_->Unregister();
-}
-
} // namespace content
diff --git a/media/video/capture/fake_video_capture_device.cc b/media/video/capture/fake_video_capture_device.cc
index 113fccf..d3d53d9 100644
--- a/media/video/capture/fake_video_capture_device.cc
+++ b/media/video/capture/fake_video_capture_device.cc
@@ -20,8 +20,6 @@ namespace media {
static const int kFakeCaptureTimeoutMs = 50;
static const int kFakeCaptureBeepCycle = 20; // Visual beep every 1s.
static const int kFakeCaptureCapabilityChangePeriod = 30;
-// TODO(mcasas): Do not rely on this static variable http://crbug.com/323913.
-static VideoCaptureFormats g_internal_supported_formats;
enum { kNumberOfFakeDevices = 2 };
bool FakeVideoCaptureDevice::fail_next_create_ = false;
@@ -33,12 +31,6 @@ size_t FakeVideoCaptureDevice::NumberOfFakeDevices(void) {
return number_of_devices_;
}
-//static
-void FakeVideoCaptureDevice::SetSupportedFormats(
- const VideoCaptureFormats& new_formats) {
- g_internal_supported_formats = new_formats;
-}
-
// static
void FakeVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
// Empty the name list.
@@ -56,10 +48,7 @@ void FakeVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
void FakeVideoCaptureDevice::GetDeviceSupportedFormats(
const Name& device,
VideoCaptureFormats* supported_formats) {
- if (!g_internal_supported_formats.empty()){
- *supported_formats = g_internal_supported_formats;
- return;
- }
+
const size_t supported_sizes_length = 3;
const gfx::Size supported_sizes[] = {gfx::Size(320, 240),
gfx::Size(640, 480),
diff --git a/media/video/capture/fake_video_capture_device.h b/media/video/capture/fake_video_capture_device.h
index 74bc225..399a682 100644
--- a/media/video/capture/fake_video_capture_device.h
+++ b/media/video/capture/fake_video_capture_device.h
@@ -20,8 +20,6 @@ namespace media {
class MEDIA_EXPORT FakeVideoCaptureDevice : public VideoCaptureDevice {
public:
- typedef void (*SupportedFormatsCallback)(VideoCaptureFormats*);
-
static VideoCaptureDevice* Create(const Name& device_name);
virtual ~FakeVideoCaptureDevice();
// Used for testing. This will make sure the next call to Create will
@@ -29,7 +27,6 @@ class MEDIA_EXPORT FakeVideoCaptureDevice : public VideoCaptureDevice {
static void SetFailNextCreate();
static void SetNumberOfFakeDevices(size_t number_of_devices);
static size_t NumberOfFakeDevices();
- static void SetSupportedFormats(const VideoCaptureFormats& new_formats);
static void GetDeviceNames(Names* device_names);
static void GetDeviceSupportedFormats(const Name& device,
--
cgit v1.1