Commit 04097f12 authored by Max Morin's avatar Max Morin Committed by Commit Bot

Disable AudioInputDevice UMA for mirroring.

The amount of mirroring streams far outnumber the number of microphone
streams, which causes these histograms to poorly represent microphone
streams. This change ensures that the AudioInputDevice histograms
keeps measuring the same thing when the mirroring service is enabled.

Also fix lint.

Bug: 912926
Change-Id: Ib08b9a5a452d0ece9494b30aa23c02d0d18e1bd1
Reviewed-on: https://chromium-review.googlesource.com/c/1370166
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: default avatarOlga Sharonova <olka@chromium.org>
Reviewed-by: default avatarYuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615947}
parent e70befeb
......@@ -718,7 +718,7 @@ void Session::OnAnswer(const std::vector<FrameSenderConfig>& audio_configs,
audio_input_device_ = new media::AudioInputDevice(
std::make_unique<CapturedAudioInput>(base::BindRepeating(
&Session::CreateAudioStream, base::Unretained(this))),
base::ThreadPriority::NORMAL);
media::AudioInputDevice::Purpose::kLoopback);
audio_input_device_->Initialize(mirror_settings_.GetAudioCaptureParams(),
audio_capturing_callback_.get());
audio_input_device_->Start();
......
......@@ -178,7 +178,7 @@ AudioDeviceFactory::NewAudioCapturerSource(
return base::MakeRefCounted<media::AudioInputDevice>(
AudioInputIPCFactory::get()->CreateAudioInputIPC(render_frame_id, params),
base::ThreadPriority::REALTIME_AUDIO);
media::AudioInputDevice::Purpose::kUserInput);
}
// static
......
......@@ -45,6 +45,16 @@ const int kCheckMissingCallbacksIntervalSeconds = 5;
// data from the source.
const int kGotDataCallbackIntervalSeconds = 1;
base::ThreadPriority ThreadPriorityFromPurpose(
AudioInputDevice::Purpose purpose) {
switch (purpose) {
case AudioInputDevice::Purpose::kUserInput:
return base::ThreadPriority::REALTIME_AUDIO;
case AudioInputDevice::Purpose::kLoopback:
return base::ThreadPriority::NORMAL;
}
}
} // namespace
// Takes care of invoking the capture callback on the audio thread.
......@@ -56,6 +66,7 @@ class AudioInputDevice::AudioThreadCallback
AudioThreadCallback(const AudioParameters& audio_parameters,
base::ReadOnlySharedMemoryRegion shared_memory_region,
uint32_t total_segments,
bool enable_uma,
CaptureCallback* capture_callback,
base::RepeatingClosure got_data_callback);
~AudioThreadCallback() override;
......@@ -66,6 +77,7 @@ class AudioInputDevice::AudioThreadCallback
void Process(uint32_t pending_data) override;
private:
const bool enable_uma_;
base::ReadOnlySharedMemoryRegion shared_memory_region_;
base::ReadOnlySharedMemoryMapping shared_memory_mapping_;
const base::TimeTicks start_time_;
......@@ -87,8 +99,9 @@ class AudioInputDevice::AudioThreadCallback
};
AudioInputDevice::AudioInputDevice(std::unique_ptr<AudioInputIPC> ipc,
base::ThreadPriority thread_priority)
: thread_priority_(thread_priority),
Purpose purpose)
: thread_priority_(ThreadPriorityFromPurpose(purpose)),
enable_uma_(purpose == AudioInputDevice::Purpose::kUserInput),
callback_(nullptr),
ipc_(std::move(ipc)),
state_(IDLE),
......@@ -129,12 +142,14 @@ void AudioInputDevice::Stop() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
TRACE_EVENT0("audio", "AudioInputDevice::Stop");
UMA_HISTOGRAM_BOOLEAN(
"Media.Audio.Capture.DetectedMissingCallbacks",
alive_checker_ ? alive_checker_->DetectedDead() : false);
if (enable_uma_) {
UMA_HISTOGRAM_BOOLEAN(
"Media.Audio.Capture.DetectedMissingCallbacks",
alive_checker_ ? alive_checker_->DetectedDead() : false);
UMA_HISTOGRAM_ENUMERATION("Media.Audio.Capture.StreamCallbackError2",
had_error_);
UMA_HISTOGRAM_ENUMERATION("Media.Audio.Capture.StreamCallbackError2",
had_error_);
}
had_error_ = kNoError;
// Close the stream, if we haven't already.
......@@ -249,7 +264,7 @@ void AudioInputDevice::OnStreamCreated(
// Unretained is safe since |alive_checker_| outlives |audio_callback_|.
audio_callback_ = std::make_unique<AudioInputDevice::AudioThreadCallback>(
audio_parameters_, std::move(shared_memory_region),
kRequestedSharedMemoryCount, callback_,
kRequestedSharedMemoryCount, enable_uma_, callback_,
base::BindRepeating(&AliveChecker::NotifyAlive,
base::Unretained(alive_checker_.get())));
audio_thread_ =
......@@ -332,12 +347,14 @@ AudioInputDevice::AudioThreadCallback::AudioThreadCallback(
const AudioParameters& audio_parameters,
base::ReadOnlySharedMemoryRegion shared_memory_region,
uint32_t total_segments,
bool enable_uma,
CaptureCallback* capture_callback,
base::RepeatingClosure got_data_callback_)
: AudioDeviceThread::Callback(
audio_parameters,
ComputeAudioInputBufferSize(audio_parameters, 1u),
total_segments),
enable_uma_(enable_uma),
shared_memory_region_(std::move(shared_memory_region)),
start_time_(base::TimeTicks::Now()),
no_callbacks_received_(true),
......@@ -354,8 +371,10 @@ AudioInputDevice::AudioThreadCallback::AudioThreadCallback(
}
AudioInputDevice::AudioThreadCallback::~AudioThreadCallback() {
UMA_HISTOGRAM_LONG_TIMES("Media.Audio.Capture.InputStreamDuration",
base::TimeTicks::Now() - start_time_);
if (enable_uma_) {
UMA_HISTOGRAM_LONG_TIMES("Media.Audio.Capture.InputStreamDuration",
base::TimeTicks::Now() - start_time_);
}
}
void AudioInputDevice::AudioThreadCallback::MapSharedMemory() {
......@@ -383,8 +402,10 @@ void AudioInputDevice::AudioThreadCallback::Process(uint32_t pending_data) {
TRACE_EVENT_BEGIN0("audio", "AudioInputDevice::AudioThreadCallback::Process");
if (no_callbacks_received_) {
UMA_HISTOGRAM_TIMES("Media.Audio.Render.InputDeviceStartTime",
base::TimeTicks::Now() - start_time_);
if (enable_uma_) {
UMA_HISTOGRAM_TIMES("Media.Audio.Render.InputDeviceStartTime",
base::TimeTicks::Now() - start_time_);
}
no_callbacks_received_ = false;
}
......
......@@ -67,9 +67,12 @@ namespace media {
class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
public AudioInputIPCDelegate {
public:
enum Purpose : int8_t { kUserInput, kLoopback };
// NOTE: Clients must call Initialize() before using.
AudioInputDevice(std::unique_ptr<AudioInputIPC> ipc,
base::ThreadPriority thread_priority);
// |enable_uma| controls logging of UMA stats. It is used to ensure that
// stats are not logged for mirroring service streams.
AudioInputDevice(std::unique_ptr<AudioInputIPC> ipc, Purpose purpose);
// AudioCapturerSource implementation.
void Initialize(const AudioParameters& params,
......@@ -122,6 +125,8 @@ class MEDIA_EXPORT AudioInputDevice : public AudioCapturerSource,
const base::ThreadPriority thread_priority_;
const bool enable_uma_;
CaptureCallback* callback_;
// A pointer to the IPC layer that takes care of sending requests over to
......
......@@ -3,6 +3,9 @@
// found in the LICENSE file.
#include "media/audio/audio_input_device.h"
#include <utility>
#include "base/memory/ptr_util.h"
#include "base/memory/shared_memory.h"
#include "base/message_loop/message_loop.h"
......@@ -67,7 +70,7 @@ TEST(AudioInputDeviceTest, Noop) {
base::MessageLoopForIO io_loop;
MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
scoped_refptr<AudioInputDevice> device(new AudioInputDevice(
base::WrapUnique(input_ipc), base::ThreadPriority::REALTIME_AUDIO));
base::WrapUnique(input_ipc), AudioInputDevice::Purpose::kUserInput));
}
ACTION_P(ReportStateChange, device) {
......@@ -82,7 +85,7 @@ TEST(AudioInputDeviceTest, FailToCreateStream) {
MockCaptureCallback callback;
MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
scoped_refptr<AudioInputDevice> device(new AudioInputDevice(
base::WrapUnique(input_ipc), base::ThreadPriority::REALTIME_AUDIO));
base::WrapUnique(input_ipc), AudioInputDevice::Purpose::kUserInput));
device->Initialize(params, &callback);
EXPECT_CALL(*input_ipc, CreateStream(_, _, _, _))
.WillOnce(ReportStateChange(device.get()));
......@@ -118,7 +121,7 @@ TEST(AudioInputDeviceTest, CreateStream) {
MockCaptureCallback callback;
MockAudioInputIPC* input_ipc = new MockAudioInputIPC();
scoped_refptr<AudioInputDevice> device(new AudioInputDevice(
base::WrapUnique(input_ipc), base::ThreadPriority::REALTIME_AUDIO));
base::WrapUnique(input_ipc), AudioInputDevice::Purpose::kUserInput));
device->Initialize(params, &callback);
EXPECT_CALL(*input_ipc, CreateStream(_, _, _, _))
......
......@@ -5,6 +5,7 @@
#include "services/audio/public/cpp/device_factory.h"
#include <memory>
#include <utility>
#include "base/bind.h"
#include "base/threading/platform_thread.h"
......@@ -20,7 +21,7 @@ scoped_refptr<media::AudioCapturerSource> CreateInputDevice(
std::move(connector), device_id, std::move(log));
return base::MakeRefCounted<media::AudioInputDevice>(
std::move(ipc), base::ThreadPriority::REALTIME_AUDIO);
std::move(ipc), media::AudioInputDevice::Purpose::kUserInput);
}
scoped_refptr<media::AudioCapturerSource> CreateInputDevice(
......@@ -30,7 +31,7 @@ scoped_refptr<media::AudioCapturerSource> CreateInputDevice(
std::make_unique<InputIPC>(std::move(connector), device_id, nullptr);
return base::MakeRefCounted<media::AudioInputDevice>(
std::move(ipc), base::ThreadPriority::REALTIME_AUDIO);
std::move(ipc), media::AudioInputDevice::Purpose::kUserInput);
}
} // namespace audio
......@@ -5,8 +5,10 @@
#ifndef SERVICES_AUDIO_PUBLIC_CPP_DEVICE_FACTORY_H_
#define SERVICES_AUDIO_PUBLIC_CPP_DEVICE_FACTORY_H_
#include "media/audio/audio_input_device.h"
#include <memory>
#include <string>
#include "media/audio/audio_input_device.h"
#include "services/audio/public/mojom/stream_factory.mojom.h"
#include "services/service_manager/public/cpp/connector.h"
......
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