Fix the maximum native sample rate in AudioProcessing

webrtc/voice_engine/utility.cc

Index: webrtc/voice_engine/utility.cc
diff --git a/webrtc/voice_engine/utility.cc b/webrtc/voice_engine/utility.cc
index 82ef076d41b01d70af7a60f55d4d22d2c2baaf41..6d5093456307ce5b5d743a8ea13ee88a5fbe0abe 100644
--- a/webrtc/voice_engine/utility.cc
+++ b/webrtc/voice_engine/utility.cc
@@ -85,9 +85,16 @@ void DownConvertToCodecFormat(const int16_t* src_data,
   assert(codec_num_channels == 1 || codec_num_channels == 2);
   dst_af->Reset();
 
-  // Never upsample the capture signal here. This should be done at the
-  // end of the send chain.
-  int destination_rate = std::min(codec_rate_hz, sample_rate_hz);
+  // Chose the lowest native sample rate which is higher or equal than at least
+  // one of the input or codec rates.
+  int destination_rate;

[Andrew MacDonald, 2015/09/16 02:35:08]

I think you should move this to transmit_mixer.cc and do the whole thing in one
place.

It's unfortunate we have to do this at all. Ideally voice engine would be using
an APM interface that handled arbitrary rates.

I suppose it's better to get 48 kHz support in voice engine immediately, but
what do you think about delaying until we have arbitrary rate support in the
int16 interface? We could rip out a bunch of this stuff from voice engine and
just tell APM what we want.

[aluebs-webrtc, 2015/09/16 03:02:53]

What do you mean with "do the whole thing" in one place. I feel this is the
right place, since it is the function that takes the codec and input signal
constrains and converts to the right format. I think that passing in directly
destination_rate is not consistent with passing in codec_num_channels for
example.
I agree that it is unfortunate that we have to do this. I didn't know there was
an effort to support arbitrary sample rates in the int16 interface. I actually
thought that interface was going away completely and we didn't want to change
its behavior at this point. But we could wait until the new voiceengine uses the
float interface, which supports arbitrary sample rates. But I feel that could
take some time. WDYT?

[Andrew MacDonald, 2015/09/16 04:04:08]

Mainly, I don't like that both DownConvertToCodecFormat and GenerateAudioFrame
have to know about AudioProcessing details.

Also (just noticed) transmit_mixer.cc isn't the only caller. It's called from
channel.cc as well, and I'm not sure that this behavior change won't impact it.

I'd revert to the original behavior here and move this additional check to
GenerateAudioFrame instead. Is that possible?
 
Ha, no effort, but I thought we talked about it at some point. I want the
AudioFrame interface to go away, but it remains to be seen if we replace that
with another int16 interface. If voice engine moves to the float interface, all
the better.

[aluebs-webrtc, 2015/09/22 01:35:29]

Agreed. I didn't move it to the float interface, since that is better done in
another CL.
We discussed using the AudioConverter, but that one has a float interface as
well, so I just unified DownConvertToCodecFormat tp RemixAndResample and let
GenerateAudioFrame (and the caller in channel.cc) take care of which sampling
rate to use. WDYT?

[Andrew MacDonald, 2015/09/23 02:35:52]

What you have looks good. Thanks for the cleanup.

+  for (size_t i = 0; i < kNumAudioProcNativeSampleRates; ++i) {

[hlundin-webrtc, 2015/09/16 13:45:49]

Nit: You can use a C++11 range-based for loop here.

[aluebs-webrtc, 2015/09/22 01:35:29]

Since now it is defined in the header and declared in the cc file, this is no
longer possible.

+    destination_rate = kAudioProcNativeSampleRatesHz[i];
+    if (destination_rate >= sample_rate_hz ||
+        destination_rate >= codec_rate_hz) {
+      break;
+    }
+  }
 
   // If no stereo codecs are in use, we downmix a stereo stream from the
   // device early in the chain, before resampling.