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 21f753984ac442ea682173c5fc9cc7b4ad7a290d..ea4e7d19cd78271ead65fc20382e92f839f31e37 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;

[aleloi, 2016/10/10 13:06:31]

This change slightly alters the mixing behavior. If a frame is muted, it
contributed to num_mixed_audio_sources, but not to mix_list.size(). The change
is safe, because this is how the old mixer works: exactly
num_mixed_audio_sources are always mixed.

[the sun, 2016/10/10 13:35:49]

Acknowledged.

 
   // We only use the limiter if we're actually mixing multiple streams.
   MixFromList(audio_frame_for_mixing, mix_list, use_limiter_);
@@ -247,39 +240,35 @@ 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;
+bool AudioMixerImpl::AddSource(Source* audio_source) {

[the sun, 2016/10/10 13:35:49]

RTC_DCHECK(audio_source);

+  rtc::CritScope lock(&crit_);
+  const bool is_mixed = FindSourceInList(audio_source, &audio_source_list_) !=
+                        audio_source_list_.end();
+  if (is_mixed) {
+    LOG(LS_WARNING) << "Source already added to mixer";
+    return false;

[the sun, 2016/10/10 13:35:49]

I think you should just DCHECK(!is_mixed) and always return true from this
method:

RTC_CHECK_EQ(FindSourceInList(audio_source, &audio_source_list_),
audio_source_list_.end());

Because this is a logic error. The client code should never call the interface
like that. Note that since we're making this an api/ interface, we will still
need the return code though, as there may be other reasons the method may fail,
for other implementations.

[aleloi, 2016/10/10 14:02:27]

I agree: it's better to see this as an error instead of having clients silently
ignore a returned 'false'.

   }
-  return 0;
+  audio_source_list_.emplace_back(audio_source, false, 0);
+  return true;
 }
 
+bool AudioMixerImpl::RemoveSource(Source* audio_source) {

[the sun, 2016/10/10 13:35:49]

Same comments apply as in AddSource()

[aleloi, 2016/10/10 14:02:27]

Done.

+  rtc::CritScope lock(&crit_);
+  const bool is_mixed = FindSourceInList(audio_source, &audio_source_list_) !=
+                        audio_source_list_.end();
+  if (!is_mixed) {
+    return false;
+  }
+  const auto iter = FindSourceInList(audio_source, &audio_source_list_);
+  if (iter == audio_source_list_.end()) {
+    LOG_T_F(LS_WARNING) << "Source not present in mixer";
+    return false;
+  }
+  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 +320,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_);