Merge M72: "Cleanup comments and interfaces for AudioRendererMixer+Input."

Some cleanups I noticed while working on adding the async callback;
does the following:

- Fixes some spelling errors.
- Removes the need to save a callback to subscribe for errors.
- Removes a stale comment associated with the error callbcks.
- Removes some unused methods from the ARM interface.
- Always posts the OutputDeviceInfoCB per documented contract.
- Switches to "using", "RepeatingCallback", etc where appropriate.
- Switches to base::flat_set/map where appropriate.

BUG=905506
TEST=existing tests all pass.

Change-Id: Ib670c04ed59b98b07c18cc3f3afac44d802a2a68
Reviewed-on: https://chromium-review.googlesource.com/c/1357540
Reviewed-by: Olga Sharonova <[email protected]>
Commit-Queue: Dale Curtis <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#613731}(cherry picked from commit 3b3f55b)
Reviewed-on: https://chromium-review.googlesource.com/c/1362216
Reviewed-by: Dale Curtis <[email protected]>
Cr-Commit-Position: refs/branch-heads/3626@{#51}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}

Author: dalecurtis

content/renderer/media/audio/audio_renderer_mixer_manager.cc

@@ -211,7 +211,7 @@ media::AudioRendererMixer* AudioRendererMixerManager::GetMixer(
       GetMixerOutputParams(input_params, sink_info.output_params(), latency);
   media::AudioRendererMixer* mixer = new media::AudioRendererMixer(
       mixer_output_params, std::move(sink),
-      base::BindRepeating(LogMixerUmaHistogram, latency));
+      base::BindRepeating(&LogMixerUmaHistogram, latency));
   mixers_[key] = {mixer, 1};
   DVLOG(1) << __func__ << " mixer: " << mixer << " latency: " << latency
            << "\n input: " << input_params.AsHumanReadableString()

content/renderer/media/audio/audio_renderer_mixer_manager.h

@@ -60,16 +60,13 @@ class CONTENT_EXPORT AudioRendererMixerManager
       media::AudioLatency::LatencyType latency);
 
   // AudioRendererMixerPool implementation.
-
   media::AudioRendererMixer* GetMixer(
       int source_render_frame_id,
       const media::AudioParameters& input_params,
       media::AudioLatency::LatencyType latency,
       const media::OutputDeviceInfo& sink_info,
       scoped_refptr<media::AudioRendererSink> sink) final;
-
   void ReturnMixer(media::AudioRendererMixer* mixer) final;
-
   scoped_refptr<media::AudioRendererSink> GetSink(
       int source_render_frame_id,
       const std::string& device_id) final;

content/renderer/media/audio/audio_renderer_mixer_manager_unittest.cc

@@ -564,21 +564,21 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlayback) {
 
   if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) {
     // Expecting input sample rate
-    EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(32000, mixer->get_output_params_for_testing().sample_rate());
     // Round up 20 ms (640) to the power of 2.
-    EXPECT_EQ(1024, mixer->GetOutputParamsForTesting().frames_per_buffer());
+    EXPECT_EQ(1024, mixer->get_output_params_for_testing().frames_per_buffer());
   } else {
     // Expecting hardware sample rate
-    EXPECT_EQ(44100, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(44100, mixer->get_output_params_for_testing().sample_rate());
 
 // 20 ms at 44100 is 882 frames per buffer.
 #if defined(OS_WIN)
     // Round up 882 to the nearest multiple of the output buffer size (128).
     // which is 7 * 128 = 896
-    EXPECT_EQ(896, mixer->GetOutputParamsForTesting().frames_per_buffer());
+    EXPECT_EQ(896, mixer->get_output_params_for_testing().frames_per_buffer());
 #else
     // Round up 882 to the power of 2.
-    EXPECT_EQ(1024, mixer->GetOutputParamsForTesting().frames_per_buffer());
+    EXPECT_EQ(1024, mixer->get_output_params_for_testing().frames_per_buffer());
 #endif  // defined(OS_WIN)
   }
 
@@ -606,14 +606,14 @@ TEST_F(AudioRendererMixerManagerTest,
   // 20 ms at 44100 is 882 frames per buffer.
   if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) {
     // Expecting input sample rate
-    EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(32000, mixer->get_output_params_for_testing().sample_rate());
   } else {
     // Expecting hardware sample rate
-    EXPECT_EQ(44100, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(44100, mixer->get_output_params_for_testing().sample_rate());
   }
 
   // Prefer device buffer size (2048) if is larger than 20 ms buffer size.
-  EXPECT_EQ(2048, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(2048, mixer->get_output_params_for_testing().frames_per_buffer());
 
   ReturnMixer(mixer);
 }
@@ -635,15 +635,15 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyPlaybackFakeAudio) {
                kDefaultDeviceId, SinkUseState::kNewSink);
 
   // Expecting input sample rate
-  EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate());
+  EXPECT_EQ(32000, mixer->get_output_params_for_testing().sample_rate());
 
 // 20 ms at 32000 is 640 frames per buffer.
 #if defined(OS_WIN)
   // Use 20 ms buffer.
-  EXPECT_EQ(640, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(640, mixer->get_output_params_for_testing().frames_per_buffer());
 #else
   // Ignore device buffer size, round up 640 to the power of 2.
-  EXPECT_EQ(1024, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(1024, mixer->get_output_params_for_testing().frames_per_buffer());
 #endif  // defined(OS_WIN)
 
   ReturnMixer(mixer);
@@ -675,19 +675,19 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtc) {
           : 44100;
 
   EXPECT_EQ(output_sample_rate,
-            mixer->GetOutputParamsForTesting().sample_rate());
+            mixer->get_output_params_for_testing().sample_rate());
 
 #if defined(OS_LINUX) || defined(OS_MACOSX) || defined(OS_FUCHSIA)
   // Use 10 ms buffer (441 frames per buffer).
   EXPECT_EQ(output_sample_rate / 100,
-            mixer->GetOutputParamsForTesting().frames_per_buffer());
+            mixer->get_output_params_for_testing().frames_per_buffer());
 #elif defined(OS_ANDROID)
   // If hardware buffer size (128) is less than 20 ms (882), use 20 ms buffer
   // (otherwise, use hardware buffer).
-  EXPECT_EQ(882, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(882, mixer->get_output_params_for_testing().frames_per_buffer());
 #else
   // Use hardware buffer size (128).
-  EXPECT_EQ(128, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(128, mixer->get_output_params_for_testing().frames_per_buffer());
 #endif  // defined(OS_LINUX) || defined(OS_MACOSX)
 
   ReturnMixer(mixer);
@@ -709,11 +709,11 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyRtcFakeAudio) {
                kDefaultDeviceId, SinkUseState::kNewSink);
 
   // Expecting input sample rate.
-  EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate());
+  EXPECT_EQ(32000, mixer->get_output_params_for_testing().sample_rate());
 
   // 10 ms at 32000 is 320 frames per buffer. Expect it on all the platforms for
   // fake audio output.
-  EXPECT_EQ(320, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(320, mixer->get_output_params_for_testing().frames_per_buffer());
 
   ReturnMixer(mixer);
 }
@@ -741,14 +741,14 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsLatencyInteractive) {
 
   if (AudioLatency::IsResamplingPassthroughSupported(params.latency_tag())) {
     // Expecting input sample rate.
-    EXPECT_EQ(32000, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(32000, mixer->get_output_params_for_testing().sample_rate());
   } else {
     // Expecting hardware sample rate.
-    EXPECT_EQ(44100, mixer->GetOutputParamsForTesting().sample_rate());
+    EXPECT_EQ(44100, mixer->get_output_params_for_testing().sample_rate());
   }
 
   // Expect hardware buffer size.
-  EXPECT_EQ(128, mixer->GetOutputParamsForTesting().frames_per_buffer());
+  EXPECT_EQ(128, mixer->get_output_params_for_testing().frames_per_buffer());
 
   ReturnMixer(mixer);
 }
@@ -772,13 +772,13 @@ TEST_F(AudioRendererMixerManagerTest, MixerParamsBitstreamFormat) {
 
   // Output parameters should be the same as input properties for bitstream
   // formats.
-  EXPECT_EQ(params.format(), mixer->GetOutputParamsForTesting().format());
+  EXPECT_EQ(params.format(), mixer->get_output_params_for_testing().format());
   EXPECT_EQ(params.channel_layout(),
-            mixer->GetOutputParamsForTesting().channel_layout());
+            mixer->get_output_params_for_testing().channel_layout());
   EXPECT_EQ(params.sample_rate(),
-            mixer->GetOutputParamsForTesting().sample_rate());
+            mixer->get_output_params_for_testing().sample_rate());
   EXPECT_EQ(params.frames_per_buffer(),
-            mixer->GetOutputParamsForTesting().frames_per_buffer());
+            mixer->get_output_params_for_testing().frames_per_buffer());
 
   ReturnMixer(mixer);
 }

media/base/audio_renderer_mixer.cc

@@ -12,6 +12,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/trace_event/trace_event.h"
+#include "media/base/audio_renderer_mixer_input.h"
 #include "media/base/audio_timestamp_helper.h"
 
 namespace media {
@@ -23,8 +24,8 @@ enum { kPauseDelaySeconds = 10 };
 // lock.
 class AudioRendererMixer::UMAMaxValueTracker {
  public:
-  UMAMaxValueTracker(const UmaLogCallback& log_callback)
-      : log_callback_(log_callback), count_(0), max_count_(0) {}
+  UMAMaxValueTracker(UmaLogCallback log_callback)
+      : log_callback_(std::move(log_callback)), count_(0), max_count_(0) {}
 
   ~UMAMaxValueTracker() = default;
 
@@ -52,15 +53,15 @@ class AudioRendererMixer::UMAMaxValueTracker {
 
 AudioRendererMixer::AudioRendererMixer(const AudioParameters& output_params,
                                        scoped_refptr<AudioRendererSink> sink,
-                                       const UmaLogCallback& log_callback)
+                                       UmaLogCallback log_callback)
     : output_params_(output_params),
       audio_sink_(std::move(sink)),
       master_converter_(output_params, output_params, true),
       pause_delay_(base::TimeDelta::FromSeconds(kPauseDelaySeconds)),
       last_play_time_(base::TimeTicks::Now()),
       // Initialize |playing_| to true since Start() results in an auto-play.
       playing_(true),
-      input_count_tracker_(new UMAMaxValueTracker(log_callback)) {
+      input_count_tracker_(new UMAMaxValueTracker(std::move(log_callback))) {
   DCHECK(audio_sink_);
   audio_sink_->Initialize(output_params, this);
   audio_sink_->Start();
@@ -73,7 +74,7 @@ AudioRendererMixer::~AudioRendererMixer() {
   // Ensure that all mixer inputs have removed themselves prior to destruction.
   DCHECK(master_converter_.empty());
   DCHECK(converters_.empty());
-  DCHECK_EQ(error_callbacks_.size(), 0U);
+  DCHECK(error_callbacks_.empty());
 }
 
 void AudioRendererMixer::AddMixerInput(const AudioParameters& input_params,
@@ -93,12 +94,11 @@ void AudioRendererMixer::AddMixerInput(const AudioParameters& input_params,
     if (converter == converters_.end()) {
       std::pair<AudioConvertersMap::iterator, bool> result =
           converters_.insert(std::make_pair(
-              input_sample_rate, base::WrapUnique(
+              input_sample_rate, std::make_unique<LoopbackAudioConverter>(
                                      // We expect all InputCallbacks to be
                                      // capable of handling arbitrary buffer
                                      // size requests, disabling FIFO.
-                                     new LoopbackAudioConverter(
-                                         input_params, output_params_, true))));
+                                     input_params, output_params_, true)));
       converter = result.first;
 
       // Add newly-created resampler as an input to the master mixer.
@@ -132,39 +132,25 @@ void AudioRendererMixer::RemoveMixerInput(
   input_count_tracker_->Decrement();
 }
 
-void AudioRendererMixer::AddErrorCallback(const base::Closure& error_cb) {
+void AudioRendererMixer::AddErrorCallback(AudioRendererMixerInput* input) {
   base::AutoLock auto_lock(lock_);
-  error_callbacks_.push_back(error_cb);
+  error_callbacks_.insert(input);
 }
 
-void AudioRendererMixer::RemoveErrorCallback(const base::Closure& error_cb) {
+void AudioRendererMixer::RemoveErrorCallback(AudioRendererMixerInput* input) {
   base::AutoLock auto_lock(lock_);
-  for (auto it = error_callbacks_.begin(); it != error_callbacks_.end(); ++it) {
-    if (it->Equals(error_cb)) {
-      error_callbacks_.erase(it);
-      return;
-    }
-  }
+  error_callbacks_.erase(input);
+}
 
-  // An error callback should always exist when called.
-  NOTREACHED();
+bool AudioRendererMixer::CurrentThreadIsRenderingThread() {
+  return audio_sink_->CurrentThreadIsRenderingThread();
 }
 
 void AudioRendererMixer::SetPauseDelayForTesting(base::TimeDelta delay) {
   base::AutoLock auto_lock(lock_);
   pause_delay_ = delay;
 }
 
-void AudioRendererMixer::GetOutputDeviceInfoAsync(
-    AudioRendererSink::OutputDeviceInfoCB info_cb) {
-  DVLOG(1) << __func__;
-  return audio_sink_->GetOutputDeviceInfoAsync(std::move(info_cb));
-}
-
-bool AudioRendererMixer::CurrentThreadIsRenderingThread() {
-  return audio_sink_->CurrentThreadIsRenderingThread();
-}
-
 int AudioRendererMixer::Render(base::TimeDelta delay,
                                base::TimeTicks delay_timestamp,
                                int prior_frames_skipped,
@@ -192,8 +178,8 @@ int AudioRendererMixer::Render(base::TimeDelta delay,
 void AudioRendererMixer::OnRenderError() {
   // Call each mixer input and signal an error.
   base::AutoLock auto_lock(lock_);
-  for (const auto& cb : error_callbacks_)
-    cb.Run();
+  for (auto* input : error_callbacks_)
+    input->OnRenderError();
 }
 
 }  // namespace media

media/base/audio_renderer_mixer.h

@@ -11,6 +11,8 @@
 #include <memory>
 #include <string>
 
+#include "base/containers/flat_map.h"
+#include "base/containers/flat_set.h"
 #include "base/macros.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
@@ -20,18 +22,19 @@
 #include "media/base/loopback_audio_converter.h"
 
 namespace media {
+class AudioRendererMixerInput;
 
 // Mixes a set of AudioConverter::InputCallbacks into a single output stream
 // which is funneled into a single shared AudioRendererSink; saving a bundle
 // on renderer side resources.
 class MEDIA_EXPORT AudioRendererMixer
     : public AudioRendererSink::RenderCallback {
  public:
-  typedef base::Callback<void(int)> UmaLogCallback;
+  using UmaLogCallback = base::RepeatingCallback<void(int)>;
 
   AudioRendererMixer(const AudioParameters& output_params,
                      scoped_refptr<AudioRendererSink> sink,
-                     const UmaLogCallback& log_callback);
+                     UmaLogCallback log_callback);
   ~AudioRendererMixer() override;
 
   // Add or remove a mixer input from mixing; called by AudioRendererMixerInput.
@@ -41,35 +44,29 @@ class MEDIA_EXPORT AudioRendererMixer
                         AudioConverter::InputCallback* input);
 
   // Since errors may occur even when no inputs are playing, an error callback
-  // must be registered separately from adding a mixer input.  The same callback
-  // must be given to both the functions.
-  void AddErrorCallback(const base::Closure& error_cb);
-  void RemoveErrorCallback(const base::Closure& error_cb);
-
-  void SetPauseDelayForTesting(base::TimeDelta delay);
-
-  void GetOutputDeviceInfoAsync(AudioRendererSink::OutputDeviceInfoCB info_cb);
+  // must be registered separately from adding a mixer input.
+  void AddErrorCallback(AudioRendererMixerInput* input);
+  void RemoveErrorCallback(AudioRendererMixerInput* input);
 
   // Returns true if called on rendering thread, otherwise false.
   bool CurrentThreadIsRenderingThread();
 
-  const AudioParameters& GetOutputParamsForTesting() { return output_params_; };
+  void SetPauseDelayForTesting(base::TimeDelta delay);
+  const AudioParameters& get_output_params_for_testing() const {
+    return output_params_;
+  }
 
  private:
   class UMAMaxValueTracker;
 
-  // Maps input sample rate to the dedicated converter.
-  using AudioConvertersMap =
-      std::map<int, std::unique_ptr<LoopbackAudioConverter>>;
-
   // AudioRendererSink::RenderCallback implementation.
   int Render(base::TimeDelta delay,
              base::TimeTicks delay_timestamp,
              int prior_frames_skipped,
              AudioBus* audio_bus) override;
   void OnRenderError() override;
 
-  bool is_master_sample_rate(int sample_rate) {
+  bool is_master_sample_rate(int sample_rate) const {
     return sample_rate == output_params_.sample_rate();
   }
 
@@ -83,11 +80,14 @@ class MEDIA_EXPORT AudioRendererMixer
   base::Lock lock_;
 
   // List of error callbacks used by this mixer.
-  typedef std::list<base::Closure> ErrorCallbackList;
-  ErrorCallbackList error_callbacks_ GUARDED_BY(lock_);
+  base::flat_set<AudioRendererMixerInput*> error_callbacks_ GUARDED_BY(lock_);
+
+  // Maps input sample rate to the dedicated converter.
+  using AudioConvertersMap =
+      base::flat_map<int, std::unique_ptr<LoopbackAudioConverter>>;
 
   // Each of these converters mixes inputs with a given sample rate and
-  // resamples them to the output sample rate. Inputs not reqiuring resampling
+  // resamples them to the output sample rate. Inputs not requiring resampling
   // go directly to |master_converter_|.
   AudioConvertersMap converters_ GUARDED_BY(lock_);