Re: More frames in one packet

On Jul 27, 2011, at 5:22 PM, Jean-Marc Valin wrote:

> speex_bits_write() does automatically include a terminator. Just be
> aware that if there's less than 5 remaining bits in the current byte, it
> does not need to do so since there could not possibly be another frame.

I think there is a bug in nb_celp.c related to this. If there are exactly 5 terminator bits at the end of the bits, then nb_decode() will remove them, leaving 0 additional bits. I.e., if the last octet of a packet looks like abc01111, all bits will be removed. The other possibilities leave bits after a decode:

=================================
final octet	bits after decode
---------------------------------
a0111111	11
ab011111	1
abc01111	
abcd0111	0111
abcde011	011
abcdef01	01
abcdefg0	0
abcdefgh	
=================================

Line 1128 of nb_celp.c (and most likely similar lines in any other decoders) should not just return -1 if speex_bits_remaining(bits) < 5. Instead, if there are no bits, it should return -1. If there are fewer than 5 bits, it should extract them and ensure that they are in fact the appropriate terminator bits, returning -1 if so, and -2 if not. (As a side note, returning -1 for an expected condition seems wrong, but probably too late to change now.)

Likewise, line 1175 should get the remainder of the bits in the octet, ensure that they are the correct terminator, returning -1 if so, -2 if not.

Doing this would make the behavior consistent with regard to removing the terminator during a decode.

At any rate, the terminator seems broken on the TI C5x DSP since as I understand the terminator (from RFC 5574), it's 0 followed by 1s until the end of the octet, but here this could insert an additional octet of the form 01111111 or 11111111. (Note that by definition, the size of a char is 1 byte, you can have more than 8 bits per byte (and hence per char). Thus BYTES_PER_CHAR is misnamed.)

-- 
Steve Checkoway