Skip to content

Commit

Permalink
Move keyboard stream ChromeOS ifdefs into AIDM.
Browse files Browse the repository at this point in the history
Adding #if defined(OS_CHROMEOS) everywhere in the new mojo input stream
factory is a pain (and ugly), so add a dummy KeyboardMicRegistration for
other platforms and put platform-checking macros in
AudioInputDeviceManager instead.

Bug: 653861
Change-Id: If538fb6452f048fd6175a46e0e39dda3a2adfe5e
Reviewed-on: https://chromium-review.googlesource.com/757376
Reviewed-by: Olga Sharonova <[email protected]>
Commit-Queue: Max Morin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#515953}
  • Loading branch information
Max Morin authored and Commit Bot committed Nov 13, 2017
1 parent ae6c98d commit f303b12
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
#include "base/strings/stringprintf.h"
#include "build/build_config.h"
#include "content/browser/media/capture/desktop_capture_device_uma_types.h"
#include "content/browser/media/capture/web_contents_audio_input_stream.h"
#include "content/browser/media/media_internals.h"
Expand Down Expand Up @@ -107,9 +106,7 @@ std::unique_ptr<media::AudioInputDelegate> AudioInputDelegateImpl::Create(
media::UserInputMonitor* user_input_monitor,
AudioInputDeviceManager* audio_input_device_manager,
std::unique_ptr<media::AudioLog> audio_log,
#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration keyboard_mic_registration,
#endif
uint32_t shared_memory_count,
int stream_id,
int session_id,
Expand Down Expand Up @@ -154,9 +151,7 @@ std::unique_ptr<media::AudioInputDelegate> AudioInputDelegateImpl::Create(
subscriber, audio_manager, mirroring_manager, user_input_monitor,
possibly_modified_parameters, std::move(writer),
std::move(foreign_socket), std::move(audio_log),
#if defined(OS_CHROMEOS)
std::move(keyboard_mic_registration),
#endif
stream_id, render_process_id, render_frame_id, automatic_gain_control,
device));
}
Expand All @@ -170,9 +165,7 @@ AudioInputDelegateImpl::AudioInputDelegateImpl(
std::unique_ptr<AudioInputSyncWriter> writer,
std::unique_ptr<base::CancelableSyncSocket> foreign_socket,
std::unique_ptr<media::AudioLog> audio_log,
#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration keyboard_mic_registration,
#endif
int stream_id,
int render_process_id,
int render_frame_id,
Expand All @@ -184,9 +177,7 @@ AudioInputDelegateImpl::AudioInputDelegateImpl(
foreign_socket_(std::move(foreign_socket)),
audio_log_(std::move(audio_log)),
controller_(),
#if defined(OS_CHROMEOS)
keyboard_mic_registration_(std::move(keyboard_mic_registration)),
#endif
stream_id_(stream_id),
render_process_id_(render_process_id),
weak_factory_(this) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ class CONTENT_EXPORT AudioInputDelegateImpl : public media::AudioInputDelegate {
media::UserInputMonitor* user_input_monitor,
AudioInputDeviceManager* audio_input_device_manager,
std::unique_ptr<media::AudioLog> audio_log,
#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration
keyboard_mic_registration,
#endif
uint32_t shared_memory_count,
int stream_id,
int session_id,
Expand All @@ -70,10 +68,8 @@ class CONTENT_EXPORT AudioInputDelegateImpl : public media::AudioInputDelegate {
std::unique_ptr<AudioInputSyncWriter> writer,
std::unique_ptr<base::CancelableSyncSocket> foreign_socket,
std::unique_ptr<media::AudioLog> audio_log,
#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration
keyboard_mic_registration,
#endif
int stream_id,
int render_process_id,
int render_frame_id,
Expand All @@ -97,10 +93,8 @@ class CONTENT_EXPORT AudioInputDelegateImpl : public media::AudioInputDelegate {
std::unique_ptr<base::CancelableSyncSocket> foreign_socket_;
const std::unique_ptr<media::AudioLog> audio_log_;
scoped_refptr<media::AudioInputController> controller_;
#if defined(OS_CHROMEOS)
const AudioInputDeviceManager::KeyboardMicRegistration
keyboard_mic_registration_;
#endif
const int stream_id_;
const int render_process_id_;
base::WeakPtrFactory<AudioInputDelegateImpl> weak_factory_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,7 @@ class AudioInputDelegateTest : public testing::Test {
media_stream_manager_.audio_input_device_manager(),
MediaInternals::GetInstance()->CreateAudioLog(
media::AudioLogFactory::AudioComponent::AUDIO_INPUT_CONTROLLER),
#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration(),
#endif
shared_memory_count, kStreamId, session_id, kRenderProcessId,
kRenderFrameId, enable_agc, ValidAudioParameters());
}
Expand Down
12 changes: 0 additions & 12 deletions content/browser/renderer_host/media/audio_input_device_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,25 +132,13 @@ void AudioInputDeviceManager::Close(int session_id) {
}

#if defined(OS_CHROMEOS)
AudioInputDeviceManager::KeyboardMicRegistration::KeyboardMicRegistration()
: shared_registration_count_(nullptr) {}

AudioInputDeviceManager::KeyboardMicRegistration::KeyboardMicRegistration(
KeyboardMicRegistration&& other)
: shared_registration_count_(other.shared_registration_count_) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
other.shared_registration_count_ = nullptr;
}

AudioInputDeviceManager::KeyboardMicRegistration&
AudioInputDeviceManager::KeyboardMicRegistration::operator=(
KeyboardMicRegistration&& other) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DeregisterIfNeeded();
shared_registration_count_ = other.shared_registration_count_;
return *this;
}

AudioInputDeviceManager::KeyboardMicRegistration::~KeyboardMicRegistration() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DeregisterIfNeeded();
Expand Down
12 changes: 7 additions & 5 deletions content/browser/renderer_host/media/audio_input_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ class CONTENT_EXPORT AudioInputDeviceManager : public MediaStreamProvider {
int Open(const MediaStreamDevice& device) override;
void Close(int session_id) override;

#if defined(OS_CHROMEOS)
// Owns a keyboard mic stream registration.
// Owns a keyboard mic stream registration. Dummy implementation on platforms
// other than Chrome OS.
class KeyboardMicRegistration {
public:
#if defined(OS_CHROMEOS)
// No registration.
KeyboardMicRegistration();
KeyboardMicRegistration() = default;

KeyboardMicRegistration(KeyboardMicRegistration&& other);
KeyboardMicRegistration& operator=(KeyboardMicRegistration&& other);

~KeyboardMicRegistration();

Expand All @@ -75,9 +75,11 @@ class CONTENT_EXPORT AudioInputDeviceManager : public MediaStreamProvider {
// a member of the AudioInputDeviceManager, which lives as long as the IO
// thread, so the pointer will be valid for the lifetime of the
// registration.
int* shared_registration_count_;
int* shared_registration_count_ = nullptr;
#endif
};

#if defined(OS_CHROMEOS)
// Registers that a stream using keyboard mic has been opened or closed.
// Keeps count of how many such streams are open and activates and
// inactivates the keyboard mic accordingly. The (in)activation is done on the
Expand Down
19 changes: 6 additions & 13 deletions content/browser/renderer_host/media/audio_input_renderer_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,25 +145,20 @@ void AudioInputRendererHost::OnCreateStream(
->RegisterKeyboardMicStream(
base::BindOnce(&AudioInputRendererHost::DoCreateStream, this,
stream_id, render_frame_id, session_id, config));
} else {
DoCreateStream(stream_id, render_frame_id, session_id, config,
AudioInputDeviceManager::KeyboardMicRegistration());
return;
}
#else
DoCreateStream(stream_id, render_frame_id, session_id, config);
#endif
DoCreateStream(stream_id, render_frame_id, session_id, config,
AudioInputDeviceManager::KeyboardMicRegistration());
}

void AudioInputRendererHost::DoCreateStream(
int stream_id,
int render_frame_id,
int session_id,
const AudioInputHostMsg_CreateStream_Config& config
#if defined(OS_CHROMEOS)
,
AudioInputDeviceManager::KeyboardMicRegistration keyboard_mic_registration
#endif
) {
const AudioInputHostMsg_CreateStream_Config& config,
AudioInputDeviceManager::KeyboardMicRegistration
keyboard_mic_registration) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);

DCHECK_GT(render_frame_id, 0);
Expand All @@ -180,9 +175,7 @@ void AudioInputRendererHost::DoCreateStream(
media_stream_manager_->audio_input_device_manager(),
MediaInternals::GetInstance()->CreateAudioLog(
media::AudioLogFactory::AUDIO_INPUT_CONTROLLER),
#if defined(OS_CHROMEOS)
std::move(keyboard_mic_registration),
#endif
config.shared_memory_count, stream_id, session_id, render_process_id_,
render_frame_id, config.automatic_gain_control, config.params);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,8 @@ class CONTENT_EXPORT AudioInputRendererHost
int stream_id,
int render_frame_id,
int session_id,
const AudioInputHostMsg_CreateStream_Config& config
#if defined(OS_CHROMEOS)
,
AudioInputDeviceManager::KeyboardMicRegistration registration
#endif
);
const AudioInputHostMsg_CreateStream_Config& config,
AudioInputDeviceManager::KeyboardMicRegistration registration);

// Record the audio input stream referenced by |stream_id|.
void OnRecordStream(int stream_id);
Expand Down

0 comments on commit f303b12

Please sign in to comment.