Convert some more things to size_t.

Convert some more things to size_t.

These changes stem from requests by Andrew on https://codereview.webrtc.org/1228823002/ to eliminate some "return -1"s and change to using asserts plus returning size_ts.  I then also converted the relevant connected bits.

This also cleans up a bunch of style issues, e.g. no spaces around operators.

BUG=chromium:81439
TEST=none
R=andrew@webrtc.org, henrik.lundin@webrtc.org, niklas.enbom@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/1380e266ff48be9718ce0867cfd65058cb09c5fc

[Peter Kasting, 2015-08-26 20:53:50]

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

[Peter Kasting, 2015-08-27 00:37:18]

pkasting@chromium.org changed reviewers:
+ henrik.lundin@webrtc.org, niklas.enbom@webrtc.org

[Peter Kasting, 2015-08-27 00:39:03]

I didn't bother splitting this change into stylistic, functional, and type
change phases since it's small enough to (hopefully) just be reviewed as-is.

Andrew: webrtc/common_audio/signal_processing/ OWNERS
Henrik: webrtc/modules/audio_coding/ OWNERS
Niklas: webrtc/modules/audio_device/ OWNERS

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/ilbc/get_sync_seq.c (right):

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/get_sync_seq.c:63: further-past
sequences would also be outside the buffer. */
Because |period| contains only non-negative numbers, |blockStartPos|' elements
will only ever decrease in value as we iterate more times.  Since the call to
WebRtcIlbcfix_Refiner() is the only thing in this loop body with side effects
that affect anything (we would also update the elements of |blockStartPos| and
|lagBlock|, but nothing below this loop would read the particular elements we
updated), once the conditional guarding the call to WebRtcIlbcfix_Refiner()
fails, it's safe to just bail out of the loop entirely.

Not only is this faster, it avoids the case that otherwise could occur where
|blockStartPos|' elements could actually be set to negative values, which
wouldn't be compatible with using size_ts for some of these objects.

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/get_sync_seq.c:69: value = (value >
period_q) ? (value - period_q) : 0;
Previously |value| was just inlined directly into the
WebRtcIlbcfix_NearestNeighbor() call, and could result in a negative value; but
since the elements of |plocs| are all non-negative, we know that would have the
same effect as passing in zero, so this code change won't result in a functional
difference.

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/ilbc/nearest_neighbor.c (right):

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/nearest_neighbor.c:25: size_t min_diff =
static_cast<size_t>(-1);
I changed this implementation for a couple of reasons.

First, the old implementation was based on doing signed subtractions and then
squaring to get a positive value.  That doesn't really work with size_ts, so I
would have had to insert a lot of different casts, and it still would only work
if the input values happened to be close enough to the desired value for the
result to fit in an int32_t.  (They always are, but why make this function
depend on that external condition?)

Second, and more importantly, the code assumed (arlength == ENH_NBLOCKS_TOT (==
8)), and would potentially buffer-overflow otherwise.  This seems very unsafe,
and if it's really desirable, why pass in |arlength| at all?  Why not just
hardcode this -- and the calling code, in similar places -- to use this
constant?

Third, the old code seemed needlessly convoluted.  Why calculate the squared
error just to be able to use WebRtcSpl_MinIndexW32() to do the index selection?

The new code is simpler, safer, and more like what a reader would expect.  It
seems at a glance like it might be slower -- all those branches inside the loop!
-- but I'm not convinced it is; after all, MinIndexW32 has to do something very
similar, so things aren't as dissimilar as they appear, and we're losing a
multiplication besides.  In any case, since there are only 8 elements in the
array, I doubt the resulting speed difference is going to be important anywhere.

[Andrew MacDonald, 2015-08-27 02:38:17]

lgtm on webrtc/common_audio/signal_processing/

[niklas.enbom, 2015-08-27 15:50:18]

audio_device lgtm
On 2015/08/27 02:38:17, Andrew MacDonald wrote:
> lgtm on webrtc/common_audio/signal_processing/

[hlundin-webrtc, 2015-08-28 06:48:19]

4 nits, otherwise lgtm.

Thanks!

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/ilbc/nearest_neighbor.c (right):

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/nearest_neighbor.c:27: size_t diff =
(array[i] < value) ? (value - array[i]) : (array[i] - value);
Nit: const size_t diff

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/codecs/ilbc/refiner.c (right):

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/refiner.c:110: size_t st = ENH_FL0 -
searchSegStartPos - tloc2;
const

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/refiner.c:114: size_t st =
searchSegStartPos + tloc2 - ENH_FL0;
const

https://codereview.webrtc.org/1305983003/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/codecs/ilbc/refiner.c:116: size_t en = st +
ENH_VECTL - idatal;
const

[Peter Kasting, 2015-08-28 18:58:15]

Added consts.  (I love local const but it's discouraged in Chromium, so I never
get to use it!)  Landing.

[Peter Kasting, 2015-08-28 18:58:19]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2015-08-28 18:58:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, niklas.enbom@webrtc.org, andrew@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1305983003/#ps120001 (title:
"Add consts")

[commit-bot: I haz the power, 2015-08-28 18:58:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1305983003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1305983003/120001

[commit-bot: I haz the power, 2015-08-28 18:59:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-28 18:59:55]

Try jobs failed on following builders:
  linux_gn on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn/builds/5288)
  linux_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/9131)
  linux_tsan2 on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/6458)

[Peter Kasting, 2015-08-28 19:02:40]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2015-08-28 19:02:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, niklas.enbom@webrtc.org, andrew@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1305983003/#ps140001 (title:
"Use C-style cast in .c file")

[commit-bot: I haz the power, 2015-08-28 19:02:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1305983003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1305983003/140001

[commit-bot: I haz the power, 2015-08-28 19:08:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-28 19:08:19]

Try jobs failed on following builders:
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/5341)

[Peter Kasting, 2015-08-28 19:40:15]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2015-08-28 19:40:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, niklas.enbom@webrtc.org, andrew@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1305983003/#ps160001 (title:
"Support Android's C89 mode")

[commit-bot: I haz the power, 2015-08-28 19:40:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1305983003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1305983003/160001

[commit-bot: I haz the power, 2015-08-28 21:40:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-28 21:40:29]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[Peter Kasting, 2015-08-28 21:58:39]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2015-08-28 21:58:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1305983003/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1305983003/160001

[commit-bot: I haz the power, 2015-08-28 23:58:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-28 23:58:57]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[Peter Kasting, 2015-08-29 00:31:22]

Committed patchset #9 (id:160001) manually as
1380e266ff48be9718ce0867cfd65058cb09c5fc (presubmit successful).