Update audio code to use size_t more correctly,

webrtc/modules/audio_coding/neteq/background_noise.cc

Index: webrtc/modules/audio_coding/neteq/background_noise.cc
diff --git a/webrtc/modules/audio_coding/neteq/background_noise.cc b/webrtc/modules/audio_coding/neteq/background_noise.cc
index a59f444c503f098d545465d25f0a55c7a8d105a9..d3df26902cdc2e34159ef4a92b780867eb10e749 100644
--- a/webrtc/modules/audio_coding/neteq/background_noise.cc
+++ b/webrtc/modules/audio_coding/neteq/background_noise.cc
@@ -21,6 +21,9 @@
 
 namespace webrtc {
 
+// static
+const size_t BackgroundNoise::kMaxLpcOrder;

[Peter Kasting, 2015/07/23 19:37:05]

Changing the type of this from int to size_t means some compilers will complain
if it doesn't have an explicit definition in addition to the declaration. 
constexpr might be able to address this if only it worked in MSVC.

[hlundin-webrtc, 2015/08/10 11:30:00]

Acknowledged.

+
 BackgroundNoise::BackgroundNoise(size_t num_channels)
     : num_channels_(num_channels),
       channel_parameters_(new ChannelParameters[num_channels_]),
@@ -150,7 +153,7 @@ const int16_t* BackgroundNoise::FilterState(size_t channel) const {
 void BackgroundNoise::SetFilterState(size_t channel, const int16_t* input,
                                      size_t length) {
   assert(channel < num_channels_);
-  length = std::min(length, static_cast<size_t>(kMaxLpcOrder));
+  length = std::min(length, kMaxLpcOrder);
   memcpy(channel_parameters_[channel].filter_state, input,
          length * sizeof(int16_t));
 }
@@ -165,7 +168,7 @@ int16_t BackgroundNoise::ScaleShift(size_t channel) const {
 }
 
 int32_t BackgroundNoise::CalculateAutoCorrelation(
-    const int16_t* signal, int length, int32_t* auto_correlation) const {
+    const int16_t* signal, size_t length, int32_t* auto_correlation) const {
   int16_t signal_max = WebRtcSpl_MaxAbsValueW16(signal, length);
   int correlation_scale = kLogVecLen -
       WebRtcSpl_NormW32(signal_max * signal_max);
@@ -247,7 +250,7 @@ void BackgroundNoise::SaveParameters(size_t channel,
   residual_energy = residual_energy << norm_shift;
 
   // Calculate scale and shift factor.
-  parameters.scale = WebRtcSpl_SqrtFloor(residual_energy);
+  parameters.scale = static_cast<int16_t>(WebRtcSpl_SqrtFloor(residual_energy));

[hlundin-webrtc, 2015/08/10 11:30:00]

rtc::checked_cast

[Peter Kasting, 2015/08/17 22:49:46]

This shouldn't be necessary, since by definition the square root of a 32-bit
quantity must fit in 16 bits.

If you would like, I could make this clearer by (separately) changing the
signature of WebRtcSpl_SqrtFloor() to return an int16_t.  It looks like we have
many places in the code which already unconditionally cast this return value to
int16_t, so this would enable removing all those casts.

[hlundin-webrtc, 2015/08/18 07:19:18]

That seems like a good idea, if you don't mind.

[Peter Kasting, 2015/08/18 20:32:07]

Actually, I don't think this is safe after all.  The results fit in 16 bits, but
that's 16 value bits, not 15 bits + a sign bit.

The explicit cast I'm adding here doesn't actually change the function of the
code -- it just removes an "implicit cast" warning -- and we seem to get this
wrong in numerous other places.  Rather than just fix this location, I've filed
a bug about this general pattern:
https://code.google.com/p/webrtc/issues/detail?id=4924

I'd appreciate it if you could triage this bug.

[hlundin-webrtc, 2015/08/19 07:36:45]

I saw the bug and will triage it. Thanks for investigating!

   // Add 13 to the |scale_shift_|, since the random numbers table is in
   // Q13.
   // TODO(hlundin): Move the "13" to where the |scale_shift_| is used?