Changed mixing to be done at the minimal possible frequency.

webrtc/modules/audio_mixer/audio_mixer_impl.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 119 matching lines @@
 AudioMixerImpl::SourceStatusList::iterator FindSourceInList(
     AudioMixerImpl::Source const* audio_source,
     AudioMixerImpl::SourceStatusList* audio_source_list) {
   return std::find_if(
       audio_source_list->begin(), audio_source_list->end(),
       [audio_source](const std::unique_ptr<AudioMixerImpl::SourceStatus>& p) {
         return p->audio_source == audio_source;
       });
 }
 
+// Rounds the maximal audio source frequency up to an APM-native
+// frequency.
+int CalculateMixingFrequency(
+    const AudioMixerImpl::SourceStatusList& audio_source_list) {
+  if (audio_source_list.empty()) {
+    return AudioMixerImpl::kDefaultFrequency;
+  }
+  using APM = AudioProcessing::NativeRate;
+  int maximal_frequency = 0;
+  for (const auto& source_status : audio_source_list) {
+    const int source_needed_frequency =
+        source_status->audio_source->PreferredSampleRate();
+    RTC_DCHECK_LE(APM::kSampleRate8kHz, source_needed_frequency);
+    RTC_DCHECK_LE(source_needed_frequency, APM::kSampleRate48kHz);

[the sun, 2016/11/01 08:51:29]

nit: _GE and switch order makes for a nicer printout whenever the DCHECK hits
(but I agree this is more readable...)

[hlundin-webrtc, 2016/11/01 09:32:33]

I challenge this, and provide supporting evidence. In a test program, I entered
a failing DCHECK:

RTC_DCHECK_GE(argc, 10);
# Check failed: argc >= 10 (3 vs. 10)

RTC_DCHECK_LE(10, argc);
# Check failed: 10 <= argc (10 vs. 3)

So, I see no reason to stick to either variant from a printout-readability point
of view.

+    maximal_frequency = std::max(maximal_frequency, source_needed_frequency);
+  }
+
+  static const std::vector<int> native_rates = {
+      APM::kSampleRate8kHz, APM::kSampleRate16kHz, APM::kSampleRate32kHz,
+      APM::kSampleRate48kHz};
+  const auto rounded_up_index = std::lower_bound(
+      native_rates.begin(), native_rates.end(), maximal_frequency);
+  RTC_DCHECK(rounded_up_index != native_rates.end());
+  return *rounded_up_index;
+}
+
 }  // namespace
 
 AudioMixerImpl::AudioMixerImpl(std::unique_ptr<AudioProcessing> limiter)
     : audio_source_list_(),
       use_limiter_(true),
       time_stamp_(0),
       limiter_(std::move(limiter)) {
   SetOutputFrequency(kDefaultFrequency);
 }
 
@@ skipping 29 matching lines @@
   }
 
   if (limiter->gain_control()->Enable(true) != limiter->kNoError) {
     return nullptr;
   }
 
   return rtc::scoped_refptr<AudioMixerImpl>(
       new rtc::RefCountedObject<AudioMixerImpl>(std::move(limiter)));
 }
 
-void AudioMixerImpl::Mix(int sample_rate,
-                         size_t number_of_channels,
+void AudioMixerImpl::Mix(size_t number_of_channels,
                          AudioFrame* audio_frame_for_mixing) {
   RTC_DCHECK(number_of_channels == 1 || number_of_channels == 2);
   RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
 
+  const int sample_rate = [&]() {

[the sun, 2016/11/01 08:51:29]

I think this is a dubious use of a lambda. The need to make sample_rate const
isn't that big IMO, and writing simple code is ALWAYS preferable to using
advanced, new golden hammers. For this type of construct, this is a lot more
common (and to most developers more straightforward):

int sample_rate = 0;
{
    rtc::CritScope lock(&crit_);
    sample_rate = CalculateMixingFrequency(audio_source_list_);
}

[hlundin-webrtc, 2016/11/01 09:32:33]

Acknowledged.

[aleloi, 2016/11/01 10:28:59]

I'd like to keep it unless reviewers insist otherwise. My argument is that the
syntactical overhead isn't particularly big, that most devs are quite familiar
with lambdas and that we already use exactly the same pattern in several other
places in the code base (consistency :D). I did a grep on "}()" and found 4 more
exact copies of this pattern and a few more places where a larger lambda was
created and executed.

+    rtc::CritScope lock(&crit_);
+    return CalculateMixingFrequency(audio_source_list_);
+  }();
+
   if (OutputFrequency() != sample_rate) {
     SetOutputFrequency(sample_rate);
   }
 
   AudioFrameList mix_list;
   {
     rtc::CritScope lock(&crit_);
     mix_list = GetAudioFromSources();
 
     for (const auto& frame : mix_list) {
@@ skipping 140 matching lines @@
 
   const auto iter = FindSourceInList(audio_source, &audio_source_list_);
   if (iter != audio_source_list_.end()) {
     return (*iter)->is_mixed;
   }
 
   LOG(LS_ERROR) << "Audio source unknown";
   return false;
 }
 }  // namespace webrtc