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

[Alexis Ballier]

Hi,

> [...]
> > +/**
>
> trailing whitespace
> may i suggest that you try grep ' $'


Hmm good idea, I didn't even think about doing it like that, I was
stupidly manually searching for them while rereading the code :/


> [...]
> > +    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


removed


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

Indeed, removed all of them

Also changed to "temp_cost = 1 + s->pixel_size + !i;", with some luck
gcc might optimize it better


I tried to improve style consistency with things like :
- a space after a comma in function calls, space after ';' in for's
- a space around '+', '-', '>' & friends, '=', but not '*', which I
find more readable (but this is discussable with the "space after a
comma" in function calls)
- if/for/while *space* (condition) *space* {

There was something I wasn't sure :

if (!s->frame.key_frame && !memcmp(this_line, prev_line, s->pixel_size))
    skipcount  = FFMIN(skipcount + 1, MAX_RLE_SKIP);
else skipcount = 0;

I didn't know if you prefered 4 spaces indentation or better
alignment, I assumed 4 spaces had preference


Also added my name in the header, and just wrote that the file is
based on flashsvenc.c as Benjamin asked


Regards,

Alexis.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qtrle_encoder_try4.patch
Type: text/x-patch
Size: 13943 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070627/8cc870e4/attachment.bin>