Skip to content

Commit

Permalink
Correctly handle enumerations for AudioManagers that report only the …
Browse files Browse the repository at this point in the history
…default audio output device.

BUG=578993

Review URL: https://codereview.chromium.org/1602083003

Cr-Commit-Position: refs/heads/master@{#370113}
(cherry picked from commit bbfc18f)

Review URL: https://codereview.chromium.org/1607923002 .

Cr-Commit-Position: refs/branch-heads/2623@{crosswalk-project#9}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
  • Loading branch information
Guido Urdaneta committed Jan 19, 2016
1 parent 0ab0238 commit 92eee5f
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@ AudioOutputDeviceEnumeration EnumerateDevicesOnDeviceThread(
media::AudioDeviceNames device_names;
audio_manager->GetAudioOutputDeviceNames(&device_names);

snapshot.has_actual_devices = !device_names.empty();

// If no devices in enumeration, return a list with a default device
if (device_names.empty()) {
snapshot.push_back({media::AudioManagerBase::kDefaultDeviceId,
media::AudioManager::GetDefaultDeviceName(),
audio_manager->GetDefaultOutputStreamParameters()});
if (!snapshot.has_actual_devices) {
snapshot.devices.push_back(
{media::AudioManagerBase::kDefaultDeviceId,
media::AudioManager::GetDefaultDeviceName(),
audio_manager->GetDefaultOutputStreamParameters()});
return snapshot;
}

for (const media::AudioDeviceName& name : device_names) {
snapshot.push_back(
snapshot.devices.push_back(
{name.unique_id, name.device_name,
name.unique_id == media::AudioManagerBase::kDefaultDeviceId
? audio_manager->GetDefaultOutputStreamParameters()
Expand All @@ -43,6 +46,16 @@ AudioOutputDeviceEnumeration EnumerateDevicesOnDeviceThread(

} // namespace

AudioOutputDeviceEnumeration::AudioOutputDeviceEnumeration(
const std::vector<AudioOutputDeviceInfo>& devices,
bool has_actual_devices)
: devices(devices), has_actual_devices(has_actual_devices) {}

AudioOutputDeviceEnumeration::AudioOutputDeviceEnumeration()
: has_actual_devices(false) {}

AudioOutputDeviceEnumeration::~AudioOutputDeviceEnumeration() {}

AudioOutputDeviceEnumerator::AudioOutputDeviceEnumerator(
media::AudioManager* audio_manager,
CachePolicy cache_policy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,19 @@ struct AudioOutputDeviceInfo {
media::AudioParameters output_params;
};

typedef std::vector<AudioOutputDeviceInfo> AudioOutputDeviceEnumeration;
// The result of an enumeration. It is used only in the browser side.
struct AudioOutputDeviceEnumeration {
public:
AudioOutputDeviceEnumeration(
const std::vector<AudioOutputDeviceInfo>& devices,
bool has_actual_devices);
AudioOutputDeviceEnumeration();
~AudioOutputDeviceEnumeration();

std::vector<AudioOutputDeviceInfo> devices;
bool has_actual_devices;
};

typedef base::Callback<void(const AudioOutputDeviceEnumeration&)>
AudioOutputDeviceEnumerationCB;

Expand All @@ -59,11 +71,15 @@ class CONTENT_EXPORT AudioOutputDeviceEnumerator {

// Does an enumeration and provides the results to the callback.
// If there are no physical devices, the result contains a single entry with
// the default parameters provided by the underlying audio manager.
// the default parameters provided by the underlying audio manager and with
// the |has_actual_devices| field set to false.
// The behavior with no physical devices is there to ease the transition
// from the use of RenderThreadImpl::GetAudioHardwareConfig(), which always
// provides default parameters, even if there are no devices.
// See https://crbug.com/549125.
// Some audio managers always report a single device, regardless of the
// physical devices in the system. In this case the |has_actual_devices| field
// is set to true to differentiate from the case of no physical devices.
void Enumerate(const AudioOutputDeviceEnumerationCB& callback);

// Invalidates the current cache.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h"
#include "base/thread_task_runner_handle.h"
#include "content/browser/renderer_host/media/audio_output_device_enumerator.h"
#include "content/public/test/test_browser_thread_bundle.h"
Expand All @@ -25,24 +26,62 @@ using testing::_;
namespace content {

namespace {

class MockAudioManager : public media::FakeAudioManager {
public:
MockAudioManager() : FakeAudioManager(&fake_audio_log_factory_) {}
MockAudioManager(size_t num_devices)
: FakeAudioManager(&fake_audio_log_factory_), num_devices_(num_devices) {}
MockAudioManager() : MockAudioManager(0) {}
~MockAudioManager() override {}
MOCK_METHOD1(GetAudioOutputDeviceNames, void(media::AudioDeviceNames*));

MOCK_METHOD1(MockGetAudioOutputDeviceNames, void(media::AudioDeviceNames*));

void GetAudioOutputDeviceNames(
media::AudioDeviceNames* device_names) override {
DCHECK(device_names->empty());
MockGetAudioOutputDeviceNames(device_names);
if (num_devices_ > 0) {
device_names->push_back(
media::AudioDeviceName(AudioManager::GetDefaultDeviceName(),
AudioManagerBase::kDefaultDeviceId));
for (size_t i = 0; i < num_devices_; i++) {
device_names->push_back(
media::AudioDeviceName("FakeDeviceName_" + base::IntToString(i),
"FakeDeviceId_" + base::IntToString(i)));
}
}
}

private:
media::FakeAudioLogFactory fake_audio_log_factory_;
size_t num_devices_;
DISALLOW_COPY_AND_ASSIGN(MockAudioManager);
};

// Fake audio manager that exposes only the default device
class OnlyDefaultDeviceAudioManager : public MockAudioManager {
public:
OnlyDefaultDeviceAudioManager() {}
~OnlyDefaultDeviceAudioManager() override {}
void GetAudioOutputDeviceNames(
media::AudioDeviceNames* device_names) override {
DCHECK(device_names->empty());
MockGetAudioOutputDeviceNames(device_names);
device_names->push_front(
media::AudioDeviceName(AudioManager::GetDefaultDeviceName(),
AudioManagerBase::kDefaultDeviceId));
}

private:
DISALLOW_COPY_AND_ASSIGN(OnlyDefaultDeviceAudioManager);
};

} // namespace

class AudioOutputDeviceEnumeratorTest : public ::testing::Test {
public:
AudioOutputDeviceEnumeratorTest()
: thread_bundle_(), task_runner_(base::ThreadTaskRunnerHandle::Get()) {
audio_manager_.reset(new MockAudioManager());
}
: thread_bundle_(), task_runner_(base::ThreadTaskRunnerHandle::Get()) {}

~AudioOutputDeviceEnumeratorTest() override {}

Expand All @@ -65,6 +104,14 @@ class AudioOutputDeviceEnumeratorTest : public ::testing::Test {
}
}

void EnumerateCountCallback(size_t num_entries_expected,
bool actual_devices_expected,
const AudioOutputDeviceEnumeration& result) {
EXPECT_EQ(actual_devices_expected, result.has_actual_devices);
EXPECT_EQ(num_entries_expected, result.devices.size());
task_runner_->PostTask(FROM_HERE, run_loop_.QuitClosure());
}

void QuitCallback(const AudioOutputDeviceEnumeration& result) {
MockCallback(result);
task_runner_->PostTask(FROM_HERE, run_loop_.QuitClosure());
Expand All @@ -82,7 +129,8 @@ class AudioOutputDeviceEnumeratorTest : public ::testing::Test {

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateWithCache) {
const int num_calls = 10;
EXPECT_CALL(*audio_manager_, GetAudioOutputDeviceNames(_)).Times(1);
audio_manager_.reset(new MockAudioManager());
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_)).Times(1);
EXPECT_CALL(*this, MockCallback(_)).Times(num_calls);
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
Expand All @@ -95,7 +143,9 @@ TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateWithCache) {

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateWithNoCache) {
const int num_calls = 10;
EXPECT_CALL(*audio_manager_, GetAudioOutputDeviceNames(_)).Times(num_calls);
audio_manager_.reset(new MockAudioManager());
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_))
.Times(num_calls);
EXPECT_CALL(*this, MockCallback(_)).Times(num_calls);
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
Expand All @@ -106,4 +156,54 @@ TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateWithNoCache) {
run_loop_.Run();
}

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateNoDevices) {
audio_manager_.reset(new MockAudioManager());
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_));
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
AudioOutputDeviceEnumerator::CACHE_POLICY_NO_CACHING);
enumerator.Enumerate(
base::Bind(&AudioOutputDeviceEnumeratorTest::EnumerateCountCallback,
base::Unretained(this), 1, false));
run_loop_.Run();
}

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateOnlyDefaultDevice) {
audio_manager_.reset(new OnlyDefaultDeviceAudioManager());
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_));
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
AudioOutputDeviceEnumerator::CACHE_POLICY_NO_CACHING);
enumerator.Enumerate(
base::Bind(&AudioOutputDeviceEnumeratorTest::EnumerateCountCallback,
base::Unretained(this), 1, true));
run_loop_.Run();
}

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateOneDevice) {
size_t num_devices = 1;
audio_manager_.reset(new MockAudioManager(num_devices));
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_));
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
AudioOutputDeviceEnumerator::CACHE_POLICY_NO_CACHING);
enumerator.Enumerate(
base::Bind(&AudioOutputDeviceEnumeratorTest::EnumerateCountCallback,
base::Unretained(this), num_devices + 1, true));
run_loop_.Run();
}

TEST_F(AudioOutputDeviceEnumeratorTest, EnumerateMultipleDevices) {
size_t num_devices = 5;
audio_manager_.reset(new MockAudioManager(num_devices));
EXPECT_CALL(*audio_manager_, MockGetAudioOutputDeviceNames(_));
AudioOutputDeviceEnumerator enumerator(
audio_manager_.get(),
AudioOutputDeviceEnumerator::CACHE_POLICY_NO_CACHING);
enumerator.Enumerate(
base::Bind(&AudioOutputDeviceEnumeratorTest::EnumerateCountCallback,
base::Unretained(this), num_devices + 1, true));
run_loop_.Run();
}

} // namespace content
4 changes: 2 additions & 2 deletions content/browser/renderer_host/media/audio_renderer_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -840,9 +840,9 @@ void AudioRendererHost::TranslateDeviceID(
const std::string& device_id,
const GURL& security_origin,
const OutputDeviceInfoCB& callback,
const AudioOutputDeviceEnumeration& device_infos) {
const AudioOutputDeviceEnumeration& enumeration) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
for (const AudioOutputDeviceInfo& device_info : device_infos) {
for (const AudioOutputDeviceInfo& device_info : enumeration.devices) {
if (device_id.empty()) {
if (device_info.unique_id == media::AudioManagerBase::kDefaultDeviceId) {
callback.Run(true, device_info);
Expand Down
2 changes: 1 addition & 1 deletion content/browser/renderer_host/media/audio_renderer_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class CONTENT_EXPORT AudioRendererHost : public BrowserMessageFilter {
void TranslateDeviceID(const std::string& device_id,
const GURL& gurl_security_origin,
const OutputDeviceInfoCB& callback,
const AudioOutputDeviceEnumeration& device_infos);
const AudioOutputDeviceEnumeration& enumeration);

// Helper method to check if the authorization procedure for stream
// |stream_id| has started.
Expand Down
6 changes: 2 additions & 4 deletions content/browser/renderer_host/media/media_stream_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -739,10 +739,8 @@ void MediaStreamManager::AudioOutputDevicesEnumerated(
DVLOG(1) << "AudioOutputDevicesEnumerated()";
StreamDeviceInfoArray device_infos;

// If the enumeration contains only one entry, it means there are no devices.
// The single entry contains default parameters from the audio manager.
if (device_enumeration.size() > 1) {
for (const auto& entry : device_enumeration) {
if (device_enumeration.has_actual_devices) {
for (const auto& entry : device_enumeration.devices) {
StreamDeviceInfo device_info(MEDIA_DEVICE_AUDIO_OUTPUT, entry.device_name,
entry.unique_id);
device_infos.push_back(device_info);
Expand Down

0 comments on commit 92eee5f

Please sign in to comment.