[FFmpeg-devel] [PATCH] Indeo 5 decoder

[Kostya]

On Mon, Feb 01, 2010 at 02:38:33AM -0500, Alexander Strange wrote:
> 
> On Feb 1, 2010, at 2:17 AM, Kostya wrote:
> 
> > $subj
> > <indeo5.patch>
> 
> 
> > +/**
> > + *  standard picture dimensions (width, height)
> > + */
> > +static uint16_t ivi5_common_pic_sizes[30] ...
> 
> static const, and the table's floated off to the right.

changed, also shrinked to uint8_t as suggested by Peter Ross
 
> > +    int             buf_switch;      ///< used to switch between three buffers
> > +    int             dst_buf;
> > +    int             ref_buf;
> 
> 
> These are FFSWAPed lower down using uint8_t.

indeed, changed

> > +            /* decode block and macroblock sizes */
> > +            mb_size  = get_bits1(&ctx->gb);
> > +            blk_size = get_bits1(&ctx->gb) ? 4 : 8;
> 
> 
> 4 << get_bits1() if you want.

8 >> get_bits1() actually

> > +            /* select transform function and scan pattern according with plane and band number */
> > +            switch ((p << 2) + i) {
> 
> 
> "according with" -> "according to"

corrected all occurences

> p * 4 + i saves parens.

using shifts in such case is our tradition, no?

> > +            /* select dequant matrix according with plane and band number */
> 
> Same.
> 
> > +        band1 = &ctx->planes[1].bands[i];
> > +        band2 = &ctx->planes[2].bands[i];
> > +
> > +        band2->width         = band1->width;
> > +        band2->height        = band1->height;
> > +        band2->mb_size       = band1->mb_size;
> > +        band2->blk_size      = band1->blk_size;
> > +        band2->is_halfpel    = band1->is_halfpel;
> > +        band2->quant_mat     = band1->quant_mat;
> > +        band2->scan          = band1->scan;
> > +        band2->inv_transform = band1->inv_transform;
> > +        band2->dc_transform  = band1->dc_transform;
> > +        band2->is_2d_trans   = band1->is_2d_trans;
> 
> 
> memcpy them?

no, they contain buffer pointers
 
> > +    /* decode rvmap probability corrections if any */
> > +    band->num_corr = 0; /* there is no corrections */
> 
> 
> "there are"

corrected these too

> IVIBandDesc in ivi_common seems to have a lot of int variables with bools stored in them too, but it's always hard to tell if those should be uint8_t or not.

well, memory is not an issue here while using uint8_t for single bool is
not justified for me
-------------- next part --------------
A non-text attachment was scrubbed...
Name: indeo5.patch
Type: text/x-diff
Size: 44647 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100201/5dd63358/attachment.patch>