Skip to content

Commit

Permalink
Renew some Audio histograms, remove others
Browse files Browse the repository at this point in the history 
Renewing:
Media.Audio.Render.GetSourceDataTimeMax.WebRTC*
Media.AudioOutputStreamProxy.StreamFormat*
Media.Audio.Render.GetSourceDataTime.WebRTC*

Removing:
OBSOLETE_HISTOGRAMS=Media.Audio.OutputDeviceMixer.StreamDuration.{MixingStatus}.{LatencyTag} and Media.Audio.OutputDeviceMixer.NoopMixingDuration are no longer used.

The removed checks in output_device_mixer_impl.cc do not actually
check the behaviour of the mixer, they only check that we are writing
to the now-removed histogram correctly.

Fixed: 1488491, 1495077, 1495067, 1474597
Change-Id: I7e5caa2dce73e4d0e099ab5fe1fd0886df66cfb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4973969
Reviewed-by: Johannes Kron <kron@chromium.org>
Commit-Queue: Fredrik Hernqvist <fhernqvist@google.com>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216879}
  • Loading branch information
Fredrik Hernqvist authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent c28d63c commit cb8e4b4
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 114 deletions.
65 changes: 2 additions & 63 deletions services/audio/output_device_mixer_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ class OutputDeviceMixerImpl::MixTrack final
"MixTrack::StartProvidingAudioToMixingGraph", "this",
static_cast<void*>(this));
DCHECK(audio_source_callback_);
RegisterPlaybackStarted();
graph_input_->Start(audio_source_callback_);
}

Expand All @@ -119,7 +118,6 @@ class OutputDeviceMixerImpl::MixTrack final
static_cast<void*>(this));
DCHECK(audio_source_callback_);
graph_input_->Stop();
RegisterPlaybackStopped(PlaybackType::kMixed);
}

bool OpenIndependentRenderingStream() {
Expand Down Expand Up @@ -152,7 +150,6 @@ class OutputDeviceMixerImpl::MixTrack final
}
}

RegisterPlaybackStarted();
rendering_stream_->Start(this);
}

Expand All @@ -163,7 +160,6 @@ class OutputDeviceMixerImpl::MixTrack final
DCHECK(audio_source_callback_);
if (rendering_stream_) {
rendering_stream_->Stop();
RegisterPlaybackStopped(PlaybackType::kIndependent);
}
}

Expand Down Expand Up @@ -194,30 +190,6 @@ class OutputDeviceMixerImpl::MixTrack final
}

private:
enum class PlaybackType { kMixed, kIndependent };

void RegisterPlaybackStarted() {
DCHECK(playback_activation_time_for_uma_.is_null());
playback_activation_time_for_uma_ = base::TimeTicks::Now();
}

void RegisterPlaybackStopped(PlaybackType playback_type) {
if (playback_type == PlaybackType::kIndependent &&
playback_activation_time_for_uma_.is_null()) {
return; // Stop() for an independent stream can be called multiple times.
}
DCHECK(!playback_activation_time_for_uma_.is_null());

base::UmaHistogramLongTimes(
base::StrCat(
{"Media.Audio.OutputDeviceMixer.StreamDuration.",
((playback_type == PlaybackType::kMixed) ? "Mixed." : "Unmixed."),
media::AudioLatency::ToString(
graph_input_->GetParams().latency_tag())}),
base::TimeTicks::Now() - playback_activation_time_for_uma_);
playback_activation_time_for_uma_ = base::TimeTicks();
}

// media::AudioOutputStream::AudioSourceCallback implementation to intercept
// error reporting during independent playback.
int OnMoreData(base::TimeDelta delay,
Expand Down Expand Up @@ -260,8 +232,6 @@ class OutputDeviceMixerImpl::MixTrack final
std::unique_ptr<media::AudioOutputStream, StreamAutoClose> rendering_stream_ =
nullptr;

base::TimeTicks playback_activation_time_for_uma_;

TrackError error_ = TrackError::kNone;
};

Expand Down Expand Up @@ -369,10 +339,6 @@ class OutputDeviceMixerImpl::MixingStats {
}

~MixingStats() {
if (!noop_mixing_start_.is_null()) {
LogNoopMixingDuration();
}

DCHECK(!start_.is_null());
base::TimeDelta duration = base::TimeTicks::Now() - start_;
LogPerDeviceUma(duration, suffix_);
Expand All @@ -383,24 +349,9 @@ class OutputDeviceMixerImpl::MixingStats {

void RemoveListener() { listener_count_.Decrement(); }

void AddActiveTrack() {
active_track_count_.Increment();
if (!noop_mixing_start_.is_null()) {
// First track after a period of feeding silence to the listeners.
DCHECK_EQ(active_track_count_.GetCurrent(), 1);
DCHECK(listener_count_.GetCurrent());
LogNoopMixingDuration();
}
}
void AddActiveTrack() { active_track_count_.Increment(); }

void RemoveActiveTrack() {
active_track_count_.Decrement();
if (listener_count_.GetCurrent() && !active_track_count_.GetCurrent()) {
// No more tracks, so we start feeding silence to listeners.
DCHECK(noop_mixing_start_.is_null());
noop_mixing_start_ = base::TimeTicks::Now();
}
}
void RemoveActiveTrack() { active_track_count_.Decrement(); }

private:
// A helper to track the max value.
Expand All @@ -423,14 +374,6 @@ class OutputDeviceMixerImpl::MixingStats {
int max_value_;
};

void LogNoopMixingDuration() {
DCHECK(!noop_mixing_start_.is_null());
base::UmaHistogramLongTimes(
"Media.Audio.OutputDeviceMixer.NoopMixingDuration",
base::TimeTicks::Now() - noop_mixing_start_);
noop_mixing_start_ = base::TimeTicks();
}

void LogPerDeviceUma(base::TimeDelta duration, const char* suffix) {
constexpr int kMaxActiveStreamCount = 50;
constexpr int kMaxListeners = 20;
Expand All @@ -452,10 +395,6 @@ class OutputDeviceMixerImpl::MixingStats {
MaxTracker active_track_count_;
MaxTracker listener_count_;
const base::TimeTicks start_;

// Start of the period when there are no active tracks and we play and feed
// silence to the listeners.
base::TimeTicks noop_mixing_start_;
};

OutputDeviceMixerImpl::OutputDeviceMixerImpl(
Expand Down
58 changes: 7 additions & 51 deletions tools/metrics/histograms/metadata/media/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,9 +976,7 @@ chromium-metrics-reviews@google.com.
Duration of the interval between the moment OutputDeviceMixer started mixed
playback (referece playback signal is requested by at least one listener and
there is at least one stream managed by the mixer playing) and the moment
OutputDeviceMixer stopped mixed playback (there are no more listeners). It
may include intervals recorded by
Media.Audio.OutputDeviceMixer.NoopMixingDuration histogram.
OutputDeviceMixer stopped mixed playback (there are no more listeners).
</summary>
<token key="Device">
<variant name="" summary="All"/>
Expand All @@ -987,21 +985,6 @@ chromium-metrics-reviews@google.com.
</token>
</histogram>

<histogram name="Media.Audio.OutputDeviceMixer.NoopMixingDuration" units="ms"
expires_after="2024-09-01">
<owner>olka@chromium.org</owner>
<owner>tguilbert@chromium.org</owner>
<owner>webrtc-audio-uma@google.com</owner>
<summary>
Duration of the interval when there are no output streams to mix but there
are reference signal listeners present, so OutputDeviceMixer renders silence
and feeds it to the listeners as well. Recorded when an output stream starts
playing (and thus playback is not empty any more) or when all listeners are
gone (and thus mixing stops). There may be multiple such intervals during an
interval recorded by Media.Audio.OutputDeviceMixer.MixingDuration.
</summary>
</histogram>

<histogram name="Media.Audio.OutputDeviceMixer.OvertimeCount" units="count"
expires_after="2024-09-01">
<owner>olka@chromium.org</owner>
Expand All @@ -1016,32 +999,6 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>

<histogram
name="Media.Audio.OutputDeviceMixer.StreamDuration.{MixingStatus}.{LatencyTag}"
units="ms" expires_after="2024-09-01">
<owner>olka@chromium.org</owner>
<owner>tguilbert@chromium.org</owner>
<owner>webrtc-audio-uma@google.com</owner>
<summary>
Duration of the interval when an output stream managed by OutputDeviceMixer
is playing independently (Unmixed) or as a part of the mix (Mixed). Measured
between the moment the stream starts playing or its playback mode
(Mixed/Unmixed) changes, and the moment the stream stops playing or its
playback mode changes.
</summary>
<token key="MixingStatus">
<variant name="Mixed" summary="Stream is played as a part of the mix"/>
<variant name="Unmixed" summary="Stream is played independently"/>
</token>
<token key="LatencyTag">
<variant name="LatencyExactMs"/>
<variant name="LatencyInteractive"/>
<variant name="LatencyPlayback"/>
<variant name="LatencyRtc"/>
<variant name="LatencyUnknown"/>
</token>
</histogram>

<histogram name="Media.Audio.OutputDeviceMixer.StreamStatus"
enum="AudioOutputDeviceMixerStreamStatus" expires_after="2024-09-01">
<owner>olka@chromium.org</owner>
Expand Down Expand Up @@ -1165,8 +1122,8 @@ chromium-metrics-reviews@google.com.
</histogram>

<histogram name="Media.Audio.Render.GetSourceDataTime.WebRTC"
units="microseconds" expires_after="2023-12-04">
<owner>guidou@chromium.org</owner>
units="microseconds" expires_after="2024-10-01">
<owner>fhernqvist@google.com</owner>
<owner>olka@chromium.org</owner>
<owner>webrtc-audio-uma@google.com</owner>
<summary>
Expand All @@ -1183,8 +1140,8 @@ chromium-metrics-reviews@google.com.
</histogram>

<histogram name="Media.Audio.Render.GetSourceDataTimeMax.WebRTC"
units="microseconds" expires_after="2023-11-12">
<owner>guidou@chromium.org</owner>
units="microseconds" expires_after="2024-10-01">
<owner>fhernqvist@google.com</owner>
<owner>olka@chromium.org</owner>
<owner>webrtc-audio-uma@google.com</owner>
<summary>
Expand Down Expand Up @@ -1792,14 +1749,13 @@ chromium-metrics-reviews@google.com.
</histogram>

<histogram name="Media.AudioOutputStreamProxy.StreamFormat"
enum="AudioOutputProxyStreamFormat" expires_after="2023-12-04">
enum="AudioOutputProxyStreamFormat" expires_after="2024-10-01">
<owner>olka@chromium.org</owner>
<owner>tguilbert@chromium.org</owner>
<owner>webrtc-audio-uma@google.com</owner>
<summary>
Records format used by AudioManager to create audio output stream proxy. If
a fake stream is created it results in muted audio playback. Warning: this
histogram was expired from 2021-01-01 to 2022-01-21; data may be missing.
a fake stream is created it results in muted audio playback.
</summary>
</histogram>

Expand Down

0 comments on commit cb8e4b4

Please sign in to comment.