[FFmpeg-devel] [PATCH] RV30/40 decoder

[Michael Niedermayer]

On Sun, Nov 18, 2007 at 11:11:24AM +0200, Kostya wrote:
> Well, it roughly the same feature-wise as it was,
> I just don't think I will improve it soon, yet
> it is playable (and maybe will attract samples
> and patches, I'm an optimist).

next part of the review ...

rv34.h:
[...]
> +    int header_size;       ///< header size in bits

unused


[...]

> +    int mb_bits;             ///< bits needed to read MB offet in slice header
[...]
> +    int vlc_set;             ///< index of currently selected VLC set
[...]
> +    SliceInfo prev_si;       ///< info for the saved slice

unused


rv30.c:
[...]
> +    get_bits(gb, 3);

skip_bits()


[...]

> +    int code;
> +
> +    code = ff_rv34_get_omega(gb) - 1;

can be merged


[...]

> +        if(!lim[k]){
> +            src += stride * 4;
> +            continue;
> +        }
> +        for(i = 0; i < 4; i++){
> +            t = ((src[-2*step] - src[1*step]) - (src[-1*step] - src[0*step])*4) >> 3;
> +            diff = av_clip(t, -lim[k], lim[k]);
> +            src[-1*step] = cm[src[-1*step] + diff];
> +            src[ 0*step] = cm[src[ 0*step] - diff];
> +            src += stride;
> +        }
> +    }

IMHO the following is cleaner:

if(lim[k]){
    for(i = 0; i < 4; i++){
        ...
    }
}else{
    src += stride * 4;
}


[...]

> +static void rv30_loop_filter(RV34DecContext *r)
> +{
> +    MpegEncContext *s = &r->s;
> +    int mb_pos;
> +    int i, j;
> +    uint8_t *Y, *U, *V;

U and V are unused, i assume this implementation is not correct but
incomplete? if so a FIXME/TODO comment should be added at least

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071125/a639191c/attachment.pgp>