Update audio code to use size_t more correctly,

webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h

Index: webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h
diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h b/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h
index 5abbd7ad446d9266a8e513d684b7468bbb185146..084135e41d6de32208c078f3fa371605dabebd8b 100644
--- a/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h
+++ b/webrtc/modules/audio_coding/codecs/isac/fix/source/structs.h
@@ -34,7 +34,7 @@ typedef struct Bitstreamstruct_dec {
   int16_t   full;             /* 0 - first byte in memory filled, second empty*/
   /* 1 - both bytes are empty (we just filled the previous memory */
 
-  int stream_size;  /* The size of stream. */
+  size_t stream_size;  /* The size of stream. */

[minyue-webrtc, 2015/08/12 16:08:08]

could you add a unit in the comment

[Peter Kasting, 2015/08/14 22:32:26]

Done.

 } Bitstr_dec;
 
 /* Bitstream struct for encoder */
@@ -178,8 +178,8 @@ typedef struct {
   int16_t pitchCycles;
   int16_t A;
   int16_t B;
-  int16_t pitchIndex;
-  int16_t stretchLag;
+  size_t pitchIndex;
+  size_t stretchLag;

[minyue-webrtc, 2015/08/12 16:08:08]

I am not sure if Lag is a count. What unit does it have? Can it be negative
(non-causal)? 

[Peter Kasting, 2015/08/14 22:32:26]

From reading caller code, I believe it's a sample count.
It better not, or the existing code is broken, since in many places it indexes
arrays by this value.

[hlundin-webrtc, 2015/08/17 08:13:21]

I agree with Minyue, Even though I believe it is always a positive value, you
will be introducing new inconsistencies if you change this. strechLag is often
given its value from another int16_t variable (lastPitchLag_Q7), which may
ripple even further.

Also, I recall that some of the signal processing code in WebRTC was written
with negative indexing. So just because it is used to index arrays doesn't
necessarily mean it is non-negative.

[Peter Kasting, 2015/08/17 08:23:53]

There are a number of lag-related variables I've changed to be size_t.  I don't
think changing this back in isolation makes sense, nor does trying to change all
these (as they interact with a variety of other variables that are also now
size_t).  And at the core I don't think such a change makes sense semantically;
in the cases I've changed these, I believe these _should_ be size_t, as they are
e.g. sample counts.

I intentionally didn't change lastPitchLag_Q7 as its units are not precisely
"sample counts" (due to shifting), and changing it turned out to introduce more
problems.  However, I did check its uses.  We only read its value in one place,
and I cast back to size_t appropriately at that spot.

Regarding negative indexing, there is at least one place where I caught that and
accommodated it, but its use should be very rare, and it isn't happening here as
some code does things like indexing stack-allocated buffers (so a negative index
is clearly wrong by inspection).

I tried multiple solutions and thought through this quite a bit while
constructing this part of the change set, and I stand by it.

[hlundin-webrtc, 2015/08/18 09:03:16]

Acknowledged.

   int16_t *prevPitchLP;                                  // [ FRAMESAMPLES/2 ]; saved 240
   int16_t seed;