Update audio code to use size_t more correctly, webrtc/common_audio/resampler/

webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h

 /*
  *  Copyright (c) 2013 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.
  */
 
 // Modified from the Chromium original here:
 // src/media/base/sinc_resampler_unittest.cc
 
 #ifndef WEBRTC_COMMON_AUDIO_RESAMPLER_SINUSOIDAL_LINEAR_CHIRP_SOURCE_H_
 #define WEBRTC_COMMON_AUDIO_RESAMPLER_SINUSOIDAL_LINEAR_CHIRP_SOURCE_H_
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/common_audio/resampler/sinc_resampler.h"
 
 namespace webrtc {
 
 // Fake audio source for testing the resampler.  Generates a sinusoidal linear
 // chirp (http://en.wikipedia.org/wiki/Chirp) which can be tuned to stress the
 // resampler for the specific sample rate conversion being used.
 class SinusoidalLinearChirpSource : public SincResamplerCallback {
  public:
-  // |delay_samples| can be used to insert a fractional sample delay into the
-  // source.  It will produce zeros until non-negative time is reached.
-  SinusoidalLinearChirpSource(int sample_rate, int samples,
-                              double max_frequency, double delay_samples);
+  // |delay_samples| can be used to insert a sample delay into the source.  It
+  // will produce zeros until non-negative time is reached.
+  SinusoidalLinearChirpSource(int sample_rate, size_t samples,
+                              double max_frequency, size_t delay_samples);

[Andrew MacDonald, 2015/07/23 01:07:03]

I see that we don't make use of the fractional delay, but why remove that
possibility? It doesn't make the implementation any simpler.

[Peter Kasting, 2015/07/23 06:02:16]

I think it eliminates a cast (to avoid signed-vs.-unsigned when comparing size_t
against double) but at the cost of adding a different one, so that's a wash.

It seems strange and misleading to allow for fractional samples when we don't
use that.  Maybe it's just me, but fractional samples seem like a weird thing,
and even if the implementation is not actually simpler, it feels less complex
that we implement dealing with integral samples using some floating-point math
under the hood, than that we go ahead and expose an API dealing in fractional
samples.

Semantically, I'm using size_t to represent "count of samples".  So if
|delay_samples| is a count of samples, but it's using a different type, that
feels weird.

There's a lot of "feels" in the justification above, which is probably another
way of saying my rationale isn't very strong.  If you still want me to restore
this, I will.

[Andrew MacDonald, 2015/07/24 03:28:33]

It might seem weird at first blush, but this whole resampling thing is about
interpolating fractional sample values, so it's not odd at all in this context.
I have a small preference for restoring it.

[Peter Kasting, 2015/07/27 23:35:22]

Restored.

 
   virtual ~SinusoidalLinearChirpSource() {}
 
-  void Run(int frames, float* destination) override;
+  void Run(size_t frames, float* destination) override;
 
-  double Frequency(int position);
+  double Frequency(size_t position);
 
  private:
   enum {
     kMinFrequency = 5
   };
 
   int sample_rate_;
-  int total_samples_;
+  size_t total_samples_;
   double max_frequency_;
   double k_;
-  int current_index_;
-  double delay_samples_;
+  size_t current_index_;
+  size_t delay_samples_;
 
   DISALLOW_COPY_AND_ASSIGN(SinusoidalLinearChirpSource);
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_COMMON_AUDIO_RESAMPLER_SINUSOIDAL_LINEAR_CHIRP_SOURCE_H_