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

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

This is a piece of https://codereview.webrtc.org/1230503003 , split out to a
separate change to make reviewing easier.

BUG=chromium:81439
TEST=none

[Peter Kasting, 2015-07-23 00:34:42]

pkasting@chromium.org changed reviewers:
+ andrew@webrtc.org

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

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinc_resampler.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinc_resampler.h:1: /*
Are you intending to modify Chromium's version similarly:
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/sinc_re...

It would be nice to minimize the diff between them.

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h:30: double
max_frequency, size_t delay_samples);
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]

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinc_resampler.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinc_resampler.h:1: /*
On 2015/07/23 at 01:07:03, andrew wrote:
> Are you intending to modify Chromium's version similarly:
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/sinc_re...
> 
> It would be nice to minimize the diff between them.

I wasn't, mostly because I was afraid it would be another months-long yak shave
and I didn't have the heart if so.  I probably should.  Ugh :(

It kinda sucks that we have this code in two places.  Can it be put somewhere
shareable between the two?

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h:30: double
max_frequency, size_t delay_samples);
On 2015/07/23 at 01:07:03, andrew wrote:
> 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.

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]

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinc_resampler.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinc_resampler.h:1: /*
On 2015/07/23 06:02:16, Peter Kasting wrote:
> On 2015/07/23 at 01:07:03, andrew wrote:
> > Are you intending to modify Chromium's version similarly:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/sinc_re...
> > 
> > It would be nice to minimize the diff between them.
> 
> I wasn't, mostly because I was afraid it would be another months-long yak
shave
> and I didn't have the heart if so.  I probably should.  Ugh :(

I can sympathize. They've already diverged somewhat, so it's not a big deal.

> 
> It kinda sucks that we have this code in two places.  Can it be put somewhere
> shareable between the two?

Yes, we might be able to have Chromium use webrtc's version.

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h:30: double
max_frequency, size_t delay_samples);
On 2015/07/23 06:02:16, Peter Kasting wrote:
> On 2015/07/23 at 01:07:03, andrew wrote:
> > 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.
> 
> 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.

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.

> 
> 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.

I have a small preference for restoring it.

[Peter Kasting, 2015-07-24 06:46:10]

On 2015/07/24 03:28:33, andrew wrote:
> I have a small preference for restoring it.

OK, will do.

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

New snap up.

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
File webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h (right):

https://codereview.webrtc.org/1231713002/diff/20001/webrtc/common_audio/resam...
webrtc/common_audio/resampler/sinusoidal_linear_chirp_source.h:30: double
max_frequency, size_t delay_samples);
On 2015/07/24 03:28:33, andrew wrote:
> On 2015/07/23 06:02:16, Peter Kasting wrote:
> > On 2015/07/23 at 01:07:03, andrew wrote:
> > > 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.
> > 
> > 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.
> 
> 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.
> 
> > 
> > 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.
> 
> I have a small preference for restoring it.

Restored.

[Andrew MacDonald, 2015-07-27 23:50:39]

lgtm