[Ffmpeg-cvslog] r6162 - trunk/libavcodec/vc1.c

[Michael Niedermayer]

Hi

On Mon, Sep 04, 2006 at 06:24:52AM +0200, kostya wrote:
> Author: kostya
> Date: Mon Sep  4 06:24:49 2006
> New Revision: 6162
> 
> Modified:
>    trunk/libavcodec/vc1.c
> 
> Log:
> VC-1 Advanced Profile support (progressive only, tested on WVC1 samples)
> 
> 
[...]
> +        lowquant = (v->pq > 12) ? 0 : 1;

lowquant = (v->pq <= 12); or without the ()


> +        v->mv_mode = mv_pmode_table[lowquant][get_prefix(gb, 1, 4)];
> +        if (v->mv_mode == MV_PMODE_INTENSITY_COMP)
> +        {
> +            int scale, shift, i;
> +            v->mv_mode2 = mv_pmode_table2[lowquant][get_prefix(gb, 1, 3)];
> +            v->lumscale = get_bits(gb, 6);
> +            v->lumshift = get_bits(gb, 6);
> +            /* fill lookup tables for intensity compensation */
> +            if(!v->lumscale) {
> +                scale = -64;
> +                shift = (255 - v->lumshift * 2) << 6;
> +                if(v->lumshift > 31)
> +                    shift += 128 << 6;
> +            } else {
> +                scale = v->lumscale + 32;
> +                if(v->lumshift > 31)
> +                    shift = (v->lumshift - 64) << 6;
> +                else
> +                    shift = v->lumshift << 6;

hmm, isnt
v->lumscale = get_bits (gb, 6); 
v->lumshift = get_sbits(gb, 6);
if(!v->lumscale) {
    scale = -64;
    shift = (255 - v->lumshift * 2) << 6;
} else {
    scale = v->lumscale + 32;
    shift = v->lumshift << 6;

equivalent and simpler, except the different value in v->lumshift ?



[...]
>  
>      /* Store the quantized DC coeff, used for prediction */
> -
>      if (n < 4) {

cosmetic ...


[...]
> +    } else { // VC1/WVC1
> +        int edata_size = avctx->extradata_size;
> +        uint8_t *edata = avctx->extradata;
> +
> +        if(avctx->extradata_size < 16) {
> +            av_log(avctx, AV_LOG_ERROR, "Extradata size too small: %i\n", edata_size);
> +            return -1;
> +        }
> +        while(edata_size > 8) {
> +            // test if we've found header
> +            if(BE_32(edata) == 0x0000010F) {
> +                edata += 4;
> +                edata_size -= 4;
> +                break;
> +            }
> +            edata_size--;
> +            edata++;
> +        }

somehow i think that

uint32_t state=-1;
for(;edata_size > 8 ; edata_size--) {
    state= (state<<8) + *edata++;
    if(state == 0x0000010F)
        break;
}

would be better, faster and more consistant to the equivalent code in other
codecs

you could also use ff_find_start_code(), though that might be overkill ...


[...]
>      /* skip B-frames if we don't have reference frames */
> -    if(s->last_picture_ptr==NULL && (s->pict_type==B_TYPE || s->dropable)) return -1;//buf_size;
> +    if(s->last_picture_ptr==NULL && (s->pict_type==B_TYPE || s->dropable)){
> +        if(buf2)av_free(buf2);

the if() isnt needed, av_free(NULL) is safe, why ohh why is every single
ffmpeg developer adding these checks ;)


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is