Cleanup of the mixer interface.

webrtc/modules/audio_mixer/audio_mixer_impl.cc

Index: webrtc/modules/audio_mixer/audio_mixer_impl.cc
diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.cc b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
index 25441b606e4a7666a64b9a72fd4d62f6edce53a3..a54121fdb2e78fba597b1681d3439bff9326e3a7 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.cc
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
@@ -137,13 +137,8 @@ AudioMixerImpl::SourceStatusList::iterator FindSourceInList(
 
 }  // namespace
 
-std::unique_ptr<AudioMixer> AudioMixer::Create() {
-  return AudioMixerImpl::Create();
-}
-
 AudioMixerImpl::AudioMixerImpl(std::unique_ptr<AudioProcessing> limiter)
     : audio_source_list_(),
-      num_mixed_audio_sources_(0),
       use_limiter_(true),
       time_stamp_(0),
       limiter_(std::move(limiter)) {
@@ -153,7 +148,7 @@ AudioMixerImpl::AudioMixerImpl(std::unique_ptr<AudioProcessing> limiter)
 
 AudioMixerImpl::~AudioMixerImpl() {}
 
-std::unique_ptr<AudioMixerImpl> AudioMixerImpl::Create() {
+rtc::scoped_refptr<AudioMixerImpl> AudioMixerImpl::Create() {
   Config config;
   config.Set<ExperimentalAgc>(new ExperimentalAgc(false));
   std::unique_ptr<AudioProcessing> limiter(AudioProcessing::Create(config));
@@ -186,8 +181,8 @@ std::unique_ptr<AudioMixerImpl> AudioMixerImpl::Create() {
     return nullptr;
   }
 
-  return std::unique_ptr<AudioMixerImpl>(
-      new AudioMixerImpl(std::move(limiter)));
+  return rtc::scoped_refptr<AudioMixerImpl>(
+      new rtc::RefCountedObject<AudioMixerImpl>(std::move(limiter)));
 }
 
 void AudioMixerImpl::Mix(int sample_rate,
@@ -202,11 +197,9 @@ void AudioMixerImpl::Mix(int sample_rate,
   }
 
   AudioFrameList mix_list;
-  size_t num_mixed_audio_sources;
   {
     rtc::CritScope lock(&crit_);
-    mix_list = GetNonAnonymousAudio();
-    num_mixed_audio_sources = num_mixed_audio_sources_;
+    mix_list = GetAudioFromSources();
   }
 
   for (const auto& frame : mix_list) {
@@ -219,7 +212,7 @@ void AudioMixerImpl::Mix(int sample_rate,
 
   time_stamp_ += static_cast<uint32_t>(sample_size_);
 
-  use_limiter_ = num_mixed_audio_sources > 1;
+  use_limiter_ = mix_list.size() > 1;
 
   // We only use the limiter if we're actually mixing multiple streams.
   MixFromList(audio_frame_for_mixing, mix_list, use_limiter_);
@@ -247,39 +240,26 @@ int AudioMixerImpl::OutputFrequency() const {
   return output_frequency_;
 }
 
-int32_t AudioMixerImpl::SetMixabilityStatus(Source* audio_source,
-                                            bool mixable) {
-  {
-    rtc::CritScope lock(&crit_);
-    const bool is_mixed = FindSourceInList(audio_source, &audio_source_list_) !=
-                          audio_source_list_.end();
-    // API must be called with a new state.
-    if (!(mixable ^ is_mixed)) {
-      return -1;
-    }
-    bool success = false;
-    if (mixable) {
-      success = AddAudioSourceToList(audio_source, &audio_source_list_);
-    } else {
-      success = RemoveAudioSourceFromList(audio_source, &audio_source_list_);
-    }
-    if (!success) {
-      RTC_NOTREACHED();
-      return -1;
-    }
-
-    size_t num_mixed_non_anonymous = audio_source_list_.size();
-    if (num_mixed_non_anonymous > kMaximumAmountOfMixedAudioSources) {
-      num_mixed_non_anonymous = kMaximumAmountOfMixedAudioSources;
-    }
-    num_mixed_audio_sources_ = num_mixed_non_anonymous;
-  }
-  return 0;
+bool AudioMixerImpl::AddSource(Source* audio_source) {
+  RTC_DCHECK(audio_source);
+  rtc::CritScope lock(&crit_);
+  RTC_DCHECK(FindSourceInList(audio_source, &audio_source_list_) ==

[hlundin-webrtc, 2016/10/11 11:50:21]

The argument of DCHECK is not evaluated in release builds. I don't think you
should DCHECK here, but rather simply return false as the interface promises.

[the sun, 2016/10/11 12:10:56]

That's exactly the point. Don't evaluate since it isn't needed.

+             audio_source_list_.end())
+      << "Source already added to mixer";
+  audio_source_list_.emplace_back(audio_source, false, 0);
+  return true;
 }
 
+bool AudioMixerImpl::RemoveSource(Source* audio_source) {
+  RTC_DCHECK(audio_source);
+  rtc::CritScope lock(&crit_);
+  const auto iter = FindSourceInList(audio_source, &audio_source_list_);
+  RTC_DCHECK(iter != audio_source_list_.end()) << "Source not present in mixer";

[hlundin-webrtc, 2016/10/11 11:50:21]

Same here.

[the sun, 2016/10/11 12:10:56]

But here "iter" is used for the deletion.

[hlundin-webrtc, 2016/10/11 12:49:54]

Yes, iter is needed in any case. I was referring to the DCHECK of the "not
found" case. But since we promise not to remove a nonexisting source, this is
fine.

+  audio_source_list_.erase(iter);
+  return true;
+}
 
-
-AudioFrameList AudioMixerImpl::GetNonAnonymousAudio() {
+AudioFrameList AudioMixerImpl::GetAudioFromSources() {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   AudioFrameList result;
   std::vector<SourceFrame> audio_source_mixing_data_list;
@@ -331,24 +311,6 @@ AudioFrameList AudioMixerImpl::GetNonAnonymousAudio() {
   return result;
 }
 
-bool AudioMixerImpl::AddAudioSourceToList(
-    Source* audio_source,
-    SourceStatusList* audio_source_list) const {
-  audio_source_list->emplace_back(audio_source, false, 0);
-  return true;
-}
-
-bool AudioMixerImpl::RemoveAudioSourceFromList(
-    Source* audio_source,
-    SourceStatusList* audio_source_list) const {
-  const auto iter = FindSourceInList(audio_source, audio_source_list);
-  if (iter != audio_source_list->end()) {
-    audio_source_list->erase(iter);
-    return true;
-  } else {
-    return false;
-  }
-}
 
 bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixed_audio) const {
   RTC_DCHECK_RUN_ON(&thread_checker_);