Update audio code to use size_t more correctly,

webrtc/modules/audio_coding/neteq/background_noise.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.
  */
 
 #include "webrtc/modules/audio_coding/neteq/background_noise.h"
 
 #include <assert.h>
 #include <string.h>  // memcpy
 
 #include <algorithm>  // min, max
 
 #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
 #include "webrtc/modules/audio_coding/neteq/audio_multi_vector.h"
 #include "webrtc/modules/audio_coding/neteq/post_decode_vad.h"
 
 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_]),
       mode_(NetEq::kBgnOn) {
   Reset();
 }
 
 BackgroundNoise::~BackgroundNoise() {}
 
 void BackgroundNoise::Reset() {
@@ skipping 109 matching lines @@
 }
 
 const int16_t* BackgroundNoise::FilterState(size_t channel) const {
   assert(channel < num_channels_);
   return channel_parameters_[channel].filter_state;
 }
 
 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));
 }
 
 int16_t BackgroundNoise::Scale(size_t channel) const {
   assert(channel < num_channels_);
   return channel_parameters_[channel].scale;
 }
 int16_t BackgroundNoise::ScaleShift(size_t channel) const {
   assert(channel < num_channels_);
   return channel_parameters_[channel].scale_shift;
 }
 
 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);
   correlation_scale = std::max(0, correlation_scale);
 
   static const int kCorrelationStep = -1;
   WebRtcSpl_CrossCorrelation(auto_correlation, signal, signal, length,
                              kMaxLpcOrder + 1, correlation_scale,
                              kCorrelationStep);
 
@@ skipping 61 matching lines @@
 
   // Normalize residual_energy to 29 or 30 bits before sqrt.
   int16_t norm_shift = WebRtcSpl_NormW32(residual_energy) - 1;
   if (norm_shift & 0x1) {
     norm_shift -= 1;  // Even number of shifts required.
   }
   assert(norm_shift >= 0);  // Should always be positive.
   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?
   parameters.scale_shift =
       static_cast<int16_t>(13 + ((kLogResidualLength + norm_shift) / 2));
 
   initialized_ = true;
 }
 
 }  // namespace webrtc