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

Update audio code to use size_t more correctly, webrtc/common_audio/ 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:26:46]

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

[Peter Kasting, 2015-07-23 00:26:47]

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
File webrtc/common_audio/blocker.cc (right):

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
webrtc/common_audio/blocker.cc:125:
input_buffer_.MoveReadPosition(-static_cast<int>(initial_delay_));
A couple places we call MoveReadPosition() with a (possibly-)negative value we
compute from one or more size_ts.  Rather than casting, another option would be
to split MoveReadPosition() into two functions:

  void MoveReadPositionForward(size_t bytes);
  void MoveReadPositionBackward(size_t bytes);

This would avoid the need for casts.  I'm not sure whether it's worth doing;
feel free to opine.

[Andrew MacDonald, 2015-07-24 19:01:56]

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
File webrtc/common_audio/blocker.cc (right):

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
webrtc/common_audio/blocker.cc:125:
input_buffer_.MoveReadPosition(-static_cast<int>(initial_delay_));
On 2015/07/23 00:26:47, Peter Kasting wrote:
> A couple places we call MoveReadPosition() with a (possibly-)negative value we
> compute from one or more size_ts.  Rather than casting, another option would
be
> to split MoveReadPosition() into two functions:
> 
>   void MoveReadPositionForward(size_t bytes);
>   void MoveReadPositionBackward(size_t bytes);
> 
> This would avoid the need for casts.  I'm not sure whether it's worth doing;
> feel free to opine.

I think that makes a lot of sense. CL up here:
https://codereview.webrtc.org/1252943007/

I looked at similarly modifying the underlying C code, but the echo canceller
depends on it rather subtly. New code will all be in C++, so I think it's fair
to restrict the change to the wrapper.

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g.c
File webrtc/common_audio/fft4g.c (right):

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g...
webrtc/common_audio/fft4g.c:291: static void makewt(size_t nw, size_t *ip, float
*w);
It's not totally clear to me that this "work area for bit reversal" should be a
size_t. Would you mind explaining the rationale?

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/lappe...
File webrtc/common_audio/lapped_transform.cc (right):

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/lappe...
webrtc/common_audio/lapped_transform.cc:87: CHECK_GT(block_length_, 0U);
My impression is that a lowercase u suffix is more common in our codebase, but I
don't feel strongly about it. Up to you.

[Peter Kasting, 2015-07-27 23:23:39]

New snap up.

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
File webrtc/common_audio/blocker.cc (right):

https://codereview.webrtc.org/1227203003/diff/20001/webrtc/common_audio/block...
webrtc/common_audio/blocker.cc:125:
input_buffer_.MoveReadPosition(-static_cast<int>(initial_delay_));
On 2015/07/24 19:01:55, andrew wrote:
> On 2015/07/23 00:26:47, Peter Kasting wrote:
> > A couple places we call MoveReadPosition() with a (possibly-)negative value
we
> > compute from one or more size_ts.  Rather than casting, another option would
> be
> > to split MoveReadPosition() into two functions:
> > 
> >   void MoveReadPositionForward(size_t bytes);
> >   void MoveReadPositionBackward(size_t bytes);
> > 
> > This would avoid the need for casts.  I'm not sure whether it's worth doing;
> > feel free to opine.
> 
> I think that makes a lot of sense. CL up here:
> https://codereview.webrtc.org/1252943007/

I will update once you land this.

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g.c
File webrtc/common_audio/fft4g.c (right):

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g...
webrtc/common_audio/fft4g.c:291: static void makewt(size_t nw, size_t *ip, float
*w);
On 2015/07/24 19:01:55, andrew wrote:
> It's not totally clear to me that this "work area for bit reversal" should be
a
> size_t. Would you mind explaining the rationale?

The type of *ip has to match the type of |n| in the externally-visible rdft()
declaration (or, in these helpers, |nw|, |nc|, |n|, etc.).  This is because
these are all passed around interchangeably, stored in each other, etc.

AFAICT from a quick read, |ip| basically holds offsets into the other tables, so
semantically its meaning is array-of-size_t.

(Also I updated a few comments here to match my changes.)

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/lappe...
File webrtc/common_audio/lapped_transform.cc (right):

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/lappe...
webrtc/common_audio/lapped_transform.cc:87: CHECK_GT(block_length_, 0U);
On 2015/07/24 19:01:55, andrew wrote:
> My impression is that a lowercase u suffix is more common in our codebase, but
I
> don't feel strongly about it. Up to you.

I have no preference, changed to 'u'.

[Andrew MacDonald, 2015-07-28 07:20:06]

lgtm

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g.c
File webrtc/common_audio/fft4g.c (right):

https://codereview.webrtc.org/1227203003/diff/80001/webrtc/common_audio/fft4g...
webrtc/common_audio/fft4g.c:291: static void makewt(size_t nw, size_t *ip, float
*w);
On 2015/07/27 23:23:39, Peter Kasting wrote:
> On 2015/07/24 19:01:55, andrew wrote:
> > It's not totally clear to me that this "work area for bit reversal" should
be
> a
> > size_t. Would you mind explaining the rationale?
> 
> The type of *ip has to match the type of |n| in the externally-visible rdft()
> declaration (or, in these helpers, |nw|, |nc|, |n|, etc.).  This is because
> these are all passed around interchangeably, stored in each other, etc.
> 
> AFAICT from a quick read, |ip| basically holds offsets into the other tables,
so
> semantically its meaning is array-of-size_t.
> 
> (Also I updated a few comments here to match my changes.)

OK thanks for the explanation.