theora dev - Oct 2009 - Possible inefficiency in encode.c

Hi,

I am very new to Theora, having just started working through the code a few
weeks ago.

I am working on a requantization tool to reduce bit rates, hopefully on the
fly, for some video conferencing work.

As I was working through the encoding phase I noticed this line in encode.c:

     for(ti=_enc->dct_token_offs[pli][zzi];ti<ndct_tokens;ti++){

It's around line 804, but I am working with 1.1b3 sources so it may have
moved a bit.

Anyway, I am thinking that this line might be an adequate substitute:

     for(ti=0;ti<ndct_tokens;ti++){

Because the tokens are now stored in separate per plane arrays instead of
all strung together in one big array like they used to be. I presume the
point of doing that was to eliminate the need for dct_token_offs altogether.

I see dct_token_offs being used in a couple of other places too.

I could be wrong of course. Please don't beat this neophyte up if I am :-)

Thanks,
Chris.

Chris Cooksey wrote:
> Because the tokens are now stored in separate per plane arrays instead of
> all strung together in one big array like they used to be. I presume the
> point of doing that was to eliminate the need for dct_token_offs
altogether.
The actual point was so that the token lists could be filled in a
different order than the one in which they will appear in the bitstream.
However, one of the consequences of this is that EOB runs cannot span
lists, even though the bitstream allows it.

This is fixed up after tokenization, before packing the tokens into the
packet, in oc_enc_tokenize_finish(). What this means is that sometimes
the first token in the list must be skipped, because it was an EOB run
that has actually been merged with the last token in a different list.
dct_token_offs[][] marks which lists need to skip such a token (i.e.,
it's always either 0 or 1).

It would actually probably be faster to keep things in a single
contiguous array, with offsets to the individual lists, just because it
would remove an extra indirection that C compilers generally do a poor
job of optimizing. We did this in the decoder, and it did provide a
small speed-up. I just never got around to doing it in the encoder.