[FFmpeg-devel] [Patch] QT RLE encoder, bis

[Michael Niedermayer]

Hi

On Tue, Jun 26, 2007 at 11:49:26PM +0200, Alexis Ballier wrote:
> Hi,
> 
> >[...]
> >> +/**
> >> + * Computes the best RLE sequence for a line
> >> + */
> >> +static void qtrle_dump_line(QtrleEncContext *s, AVFrame *p, int line, 
> >uint8_t **buf)
> >
> >encode_line() or something else without "dump" seems to be a better name
> >dump reminds me of dumping the thing as a hex values to stderr
> 
> 
> renamed to qtrle_encode_line
> 
> 
> >> +        total_skip_cost = (s->length_table[i+skipcount] + 2);
> >
> >superflous ()
> >
> 
> *cough* those () were really useless and stupid :)
> 
> 
> >> +            total_bulk_cost = s->length_table[i+1] + temp_cost;
> >> +            bulkcount  = 1;
> >> +
> >> +            for(j=2;j<=limit;j++){
> >> +                temp_cost += s->pixel_size;
> >> +                if(s->length_table[i+j] + temp_cost < total_bulk_cost){
> >> +                    /* We have found a better bulk copy ... */
> >> +                    total_bulk_cost = s->length_table[i+j] + temp_cost;
> >> +                    bulkcount = j;
> >> +                }
> >> +            }
> >
> >the loop could begin with j=1 and total_bulk_cost could be set to INT_MAX
> >that would avoid the bulkcount  = 1
> 
> 
> that's better & cleaner indeed
> 
> 
> >> +    bytestream_put_be32(&buf, 0);                     // CHUNK SIZE, 
> >patched later
> >> +
> >
> >trailing whitespace is forbidden in svn
> 
> Hmmm I thought I had been careful about this, probably not that much,
> fixed anyway

[...]
> +    s->max_buf_size=(s->avctx->width*s->avctx->height*s->pixel_size /* image base material */
> +                     + 15                                           /* header + footer */
> +                     + s->avctx->height*2                           /* skip code+rle end */
> +                     + s->avctx->width/MAX_RLE_BULK + 1             /* rle codes */);

superfluos ()


[...]
> +/** 

trailing whitespace
may i suggest that you try grep ' $'


[...]
> +            if(memcmp(p->data[0]+(start_line*p->linesize[0]),
> +                      s->previous_frame.data[0]+(start_line*p->linesize[0]),
> +                      p->linesize[0]))

superfluos ()


[...]
> +    if((start_line==0 && end_line == s->avctx->height) || start_line==s->avctx->height)

the () here is superfluos too, though its less annoying than (a*b)+c
so if you prefer the extra () here i wont mind keeping that though personally
id remove it if it where my code


[...]
> +static int qtrle_encode_frame(AVCodecContext *avctx, uint8_t *buf, int buf_size, void *data)
> +{
> +    QtrleEncContext * const s = (QtrleEncContext *)avctx->priv_data;

unneeded cast

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070627/a04a4ab6/attachment.pgp>