Changed mixing to be done at the minimal possible frequency.

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 a7af8c33d74a85df86a237cb299f5dcd12ce97d0..6c3fbd5f8570efe893b1c6510c38bbdc6e5b7c4d 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.cc
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
@@ -137,6 +137,32 @@ AudioMixerImpl::SourceStatusList::iterator FindSourceInList(
       });
 }
 
+// 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)
@@ -186,12 +212,16 @@ rtc::scoped_refptr<AudioMixerImpl> AudioMixerImpl::Create() {
       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);
   }