On Mon, Jun 18, 2012 at 04:16:01PM +0200, Christophe Gisquet wrote:
> Hi,
> 
> > +typedef struct Model {
> > +    int cum_prob[MODEL_MAX_SYMS + 1];
> > +    int weights[MODEL_MAX_SYMS + 1];
> > +    int idx2sym[MODEL_MAX_SYMS + 1];
> > +    int sym2idx[MODEL_MAX_SYMS + 1];
> > +    int num_syms;
> > +    int thr_weight, threshold;
> > +} Model;
> 
> How high are the weights typically? Same for the LUT values of idx2sym
> and sym2idx. num_syms seems to be set to 8 or 2, but there is probably
> something I missed in between.

There's one model for all pixel values (i.e. 256 symbols).
Original decoder can have up to 1023 symbols but I have not found such large
model anywhere.
 
> Basically, I am wondering if a smaller type could be used.

It can, but I don't think it's worth the effort (even though in practice
probably every field can be made int8_t or uint8_t).

> > +static void arith_normalise(ArithCoder *c)
> [...]
> > +static int arith_get_bit(ArithCoder *c)
> [...]
> > +static int arith_get_bits(ArithCoder *c, int bits)
> [...]
> > +static int arith_get_number(ArithCoder *c, int mod_val)
> 
> I guess you are not interested in this part, but probably those (and
> mostly those) could benefit from some av_inline, but most compilers
> probably get it right on their own.
> 
> > +        memset(neighbours, src[-1], 4);
> 
> Something like AV_WN32A(neighbours, src[-1]*0x01010101) maybe? I guess
> other parts would be more in need than this but still...
> 
> I haven't checked even remotely if there is any issue with incorrect
> bitstreams, which would probably be a more interesting review.

Well, since we use checked bitstream reader there's no possibility of
overreads during arithmetic coder normalisation and it should not go into
infinite loops (I check partitioning).
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to