Skip to content

Commit

Permalink
audio::OutputController logging cleanup
Browse files Browse the repository at this point in the history 
Change-Id: Ia4867e1444bb571977a4fe40462655192e7713fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3143701
Auto-Submit: Olga Sharonova <olka@chromium.org>
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918801}
  • Loading branch information
Olga Sharonova authored and Chromium LUCI CQ committed Sep 7, 2021
1 parent c8908aa commit b0a8704
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 79 deletions.
126 changes: 47 additions & 79 deletions services/audio/output_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,29 +36,6 @@ namespace {
constexpr base::TimeDelta kPowerMonitorLogInterval =
base::TimeDelta::FromSeconds(15);

// Used to log the result of rendering startup.
// Elements in this enum should not be deleted or rearranged; the only
// permitted operation is to add new elements before
// STREAM_CREATION_RESULT_MAX and update STREAM_CREATION_RESULT_MAX.
enum StreamCreationResult {
STREAM_CREATION_OK = 0,
STREAM_CREATION_CREATE_FAILED = 1,
STREAM_CREATION_OPEN_FAILED = 2,
STREAM_CREATION_RESULT_MAX = STREAM_CREATION_OPEN_FAILED,
};

void LogStreamCreationForDeviceChangeResult(StreamCreationResult result) {
UMA_HISTOGRAM_ENUMERATION(
"Media.AudioOutputController.ProxyStreamCreationResultForDeviceChange",
result, STREAM_CREATION_RESULT_MAX + 1);
}

void LogInitialStreamCreationResult(StreamCreationResult result) {
UMA_HISTOGRAM_ENUMERATION(
"Media.AudioOutputController.ProxyStreamCreationResult", result,
STREAM_CREATION_RESULT_MAX + 1);
}

const char* StateToString(OutputController::State state) {
switch (state) {
case OutputController::kEmpty:
Expand Down Expand Up @@ -184,27 +161,50 @@ bool OutputController::CreateStream() {
return state_ == kCreated;
}

void OutputController::RecreateStream(OutputController::RecreateReason reason) {
DCHECK(task_runner_->BelongsToCurrentThread());
TRACE_EVENT1("audio", "OutputController::RecreateStream", "reason",
static_cast<int>(reason));
// static
void OutputController::ReportStreamCreationUma(
OutputController::RecreateReason reason,
StreamCreationResult result) {
switch (reason) {
case RecreateReason::INITIAL_STREAM:
UMA_HISTOGRAM_ENUMERATION(
"Media.AudioOutputController."
"ProxyStreamCreationResultForDeviceChange",
result);
return;
case RecreateReason::DEVICE_CHANGE:
UMA_HISTOGRAM_ENUMERATION(
"Media.AudioOutputController.ProxyStreamCreationResult", result);
return;
case RecreateReason::LOCAL_OUTPUT_TOGGLE:
return; // Not counted in UMAs.
}
return;
}

std::string str = "RecreateStream({reason=";
// static
const char* OutputController::RecreateReasonToString(
OutputController::RecreateReason reason) {
switch (reason) {
case RecreateReason::INITIAL_STREAM:
base::StringAppendF(&str, "INITIAL_STREAM}, ");
break;
return "INITIAL_STREAM";
case RecreateReason::DEVICE_CHANGE:
base::StringAppendF(&str, "DEVICE_CHANGE}, ");
break;
return "DEVICE_CHANGE";
case RecreateReason::LOCAL_OUTPUT_TOGGLE:
base::StringAppendF(&str, "LOCAL_OUTPUT_TOGGLE}, ");
break;
return "LOCAL_OUTPUT_TOGGLE";
}
base::StringAppendF(&str, "{params=[%s]} [state=%s])",
params_.AsHumanReadableString().c_str(),
StateToString(state_));
SendLogMessage("%s", str.c_str());
return "Invalid";
}

void OutputController::RecreateStream(OutputController::RecreateReason reason) {
DCHECK(task_runner_->BelongsToCurrentThread());
TRACE_EVENT1("audio", "OutputController::RecreateStream", "reason",
RecreateReasonToString(reason));

SendLogMessage("RecreateStream({reason = %s}, {params = [%s]}, [state = %s])",
RecreateReasonToString(reason),
params_.AsHumanReadableString().c_str(),
StateToString(state_));

// Close() can be called before Create() is executed.
if (state_ == kClosed)
Expand All @@ -214,7 +214,7 @@ void OutputController::RecreateStream(OutputController::RecreateReason reason) {
DCHECK_EQ(kEmpty, state_);

if (disable_local_output_) {
SendLogMessage("%s => (WARNING: using a fake audio output stream)",
SendLogMessage("%s => (WARNING: local output disabed, using a fake stream)",
__func__);
// Create a fake AudioOutputStream that will continue pumping the audio
// data, but does not play it out anywhere. Pumping the audio data is
Expand All @@ -233,19 +233,7 @@ void OutputController::RecreateStream(OutputController::RecreateReason reason) {
if (!stream_) {
SendLogMessage("%s => (ERROR: failed to create output stream)", __func__);
state_ = kError;
// TODO(crbug.com/896484): Results should be counted iff the |stream_| is
// not a fake one. The |reason| for a non-fake stream to be created doesn't
// matter, right?
switch (reason) {
case RecreateReason::INITIAL_STREAM:
LogInitialStreamCreationResult(STREAM_CREATION_CREATE_FAILED);
break;
case RecreateReason::DEVICE_CHANGE:
LogStreamCreationForDeviceChangeResult(STREAM_CREATION_CREATE_FAILED);
break;
case RecreateReason::LOCAL_OUTPUT_TOGGLE:
break; // Not counted in UMAs.
}
ReportStreamCreationUma(reason, StreamCreationResult::kCreateFailed);
handler_->OnControllerError();
return;
}
Expand All @@ -255,44 +243,24 @@ void OutputController::RecreateStream(OutputController::RecreateReason reason) {
SendLogMessage("%s => (ERROR: failed to open the created output stream)",
__func__);
StopCloseAndClearStream();
// TODO(crbug.com/896484): Here too.
switch (reason) {
case RecreateReason::INITIAL_STREAM:
LogInitialStreamCreationResult(STREAM_CREATION_OPEN_FAILED);
break;
case RecreateReason::DEVICE_CHANGE:
LogStreamCreationForDeviceChangeResult(STREAM_CREATION_OPEN_FAILED);
break;
case RecreateReason::LOCAL_OUTPUT_TOGGLE:
break; // Not counted in UMAs.
}
state_ = kError;
ReportStreamCreationUma(reason, StreamCreationResult::kOpenFailed);
handler_->OnControllerError();
return;
}

// TODO(crbug.com/896484): Here three.
switch (reason) {
case RecreateReason::INITIAL_STREAM:
LogInitialStreamCreationResult(STREAM_CREATION_OK);
break;
case RecreateReason::DEVICE_CHANGE:
LogStreamCreationForDeviceChangeResult(STREAM_CREATION_OK);
break;
case RecreateReason::LOCAL_OUTPUT_TOGGLE:
break; // Not counted in UMAs.
}

audio_manager_->AddOutputDeviceChangeListener(this);

// We have successfully opened the stream. Set the initial volume.
stream_->SetVolume(volume_);

// Finally set the state to kCreated. Note that, it is possible that the
// stream is fake in this state due to the fallback mechanism in the audio
// output dispatcher which falls back to a fake stream if audio parameters
// are invalid or if a physical stream can't be opened for some reason.
state_ = kCreated;
ReportStreamCreationUma(reason, StreamCreationResult::kOk);

// We have successfully opened the stream. Set the initial volume.
stream_->SetVolume(volume_);

audio_manager_->AddOutputDeviceChangeListener(this);
}

void OutputController::Play() {
Expand Down
17 changes: 17 additions & 0 deletions services/audio/output_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,17 @@ class OutputController : public media::AudioOutputStream::AudioSourceCallback,
LOCAL_OUTPUT_TOGGLE = 2,
};

// Used to log the result of rendering startup.
// Elements in this enum should not be deleted or rearranged; the only
// permitted operation is to add new elements before kMaxValue and update
// kMaxValue.
enum class StreamCreationResult {
kOk = 0,
kCreateFailed = 1,
kOpenFailed = 2,
kMaxValue = kOpenFailed,
};

// Used to store various stats about a stream. The lifetime of this object is
// from play until pause. The underlying physical stream may be changed when
// resuming playback, hence separate stats are logged for each play/pause
Expand Down Expand Up @@ -226,6 +237,12 @@ class OutputController : public media::AudioOutputStream::AudioSourceCallback,
base::OneShotTimer wedge_timer_;
};

// Reports UMA statistics for stream creation.
static void ReportStreamCreationUma(RecreateReason reason,
StreamCreationResult result);

static const char* RecreateReasonToString(RecreateReason reason);

// Closes the current stream and re-creates a new one via the AudioManager. If
// reason is LOCAL_OUTPUT_TOGGLE, the new stream will be a fake one and UMA
// counts will not be incremented.
Expand Down

0 comments on commit b0a8704

Please sign in to comment.