[Ffmpeg-cvslog] r5530 - in trunk/libavcodec: vc1.c vc1acdata.h vc1data.h

Michael Niedermayer michaelni
Tue Jun 27 15:39:02 CEST 2006


Hi

On Tue, Jun 27, 2006 at 05:12:01AM +0200, kostya wrote:
[...]

> +enum Profile {
> +    PROFILE_SIMPLE,
> +    PROFILE_MAIN,
> +    PROFILE_COMPLEX, ///< TODO: WMV9 specific
> +    PROFILE_ADVANCED
> +};

you could add a =0, =1 and so on to them, while redundant, it IMHO makes
things more readable


[...]

> +static int alloc_bitplane(BitPlane *bp, int width, int height)
>  {
> +    if (!bp || bp->width<0 || bp->height<0) return -1;
> +    bp->data = (uint8_t*)av_malloc(width*height);

the cast is unneeded, IIRC rich once flamed someone due to such casts :)
also please be carefull with malloc(x*y) code if the result of the 
multiplication doesnt fit in an int then this can under some curcumstances
be exploitable
and yes i also think this cant happen here but i thought i mention it
anyway as width and height is checked but not checked for the overflow
additionally the if() above checks bp->w/h while w/h is used, is that
intended?


[...]
> +    case IMODE_DIFF6:
> +    case IMODE_NORM6:
> +        if(!(bp->height % 3) && (bp->width % 3)) { // use 2x3 decoding
> +            for(y = 0; y < bp->height; y+= 3) {
> +                for(x = bp->width & 1; x < bp->width; x += 2) {
> +                    code = get_vlc2(gb, vc1_norm6_vlc.table, VC1_NORM6_VLC_BITS, 2);
> +                    if(code < 0){
> +                        av_log(v->s.avctx, AV_LOG_DEBUG, "invalid NORM-6 VLC\n");
> +                        return -1;
> +                    }
> +                    planep[x + 0] = (code >> 0) & 1;
> +                    planep[x + 1] = (code >> 1) & 1;
> +                    planep[x + 0 + bp->stride] = (code >> 2) & 1;
> +                    planep[x + 1 + bp->stride] = (code >> 3) & 1;
> +                    planep[x + 0 + bp->stride * 2] = (code >> 4) & 1;
> +                    planep[x + 1 + bp->stride * 2] = (code >> 5) & 1;
> +                }
> +                planep += bp->stride * 3;
>              }
> +            if(bp->width & 1) decode_colskip(bp->data, 1, bp->height, bp->stride, &v->s.gb);
> +        } else { // 3x2
> +            for(y = bp->height & 1; y < bp->height; y += 2) {
> +                for(x = bp->width % 3; x < bp->width; x += 3) {
> +                    code = get_vlc2(gb, vc1_norm6_vlc.table, VC1_NORM6_VLC_BITS, 2);
> +                    if(code < 0){
> +                        av_log(v->s.avctx, AV_LOG_DEBUG, "invalid NORM-6 VLC\n");
> +                        return -1;
> +                    }
> +                    planep[x + 0] = (code >> 0) & 1;
> +                    planep[x + 1] = (code >> 1) & 1;
> +                    planep[x + 2] = (code >> 2) & 1;
> +                    planep[x + 0 + bp->stride] = (code >> 3) & 1;
> +                    planep[x + 1 + bp->stride] = (code >> 4) & 1;
> +                    planep[x + 2 + bp->stride] = (code >> 5) & 1;
> +                }

this could be simplified with a macro:
#define store_3x2(a, b)\
code = get_vlc2(gb, vc1_norm6_vlc.table, VC1_NORM6_VLC_BITS, 2);\
if(code < 0){\
    av_log(v->s.avctx, AV_LOG_DEBUG, "invalid NORM-6 VLC\n");\
    return -1;\
}\
planep[x + 0*a + 0*b] = (code >> 0) & 1;\
planep[x + 1*a + 0*b] = (code >> 1) & 1;\
planep[x + 2*a + 0*b] = (code >> 2) & 1;\
planep[x + 0*a + 1*b] = (code >> 3) & 1;\
planep[x + 1*a + 1*b] = (code >> 4) & 1;\
planep[x + 2*a + 1*b] = (code >> 5) & 1;

and store_3x2(1, bp->stride) and store_3x2(bp->stride, 1)

of coarse maybe thats not worth the mess ...


[...]
> +/** Do motion compensation over 1 macroblock
> + * Mostly adapted hpel_motion and qpel_motion from mpegvideo.c
> + */
> +static void vc1_mc_1mv(VC1Context *v)
> +{
> +    MpegEncContext *s = &v->s;
> +    DSPContext *dsp = &v->s.dsp;
> +    uint8_t *srcY, *srcU, *srcV;
> +    int dxy, mx, my, src_x, src_y;
> +    int width = s->mb_width * 16, height = s->mb_height * 16;
> +
> +    if(!v->s.last_picture.data[0])return;
> +
> +    mx = s->mv[0][0][0] >> s->mspel;
> +    my = s->mv[0][0][1] >> s->mspel;
> +    srcY = s->last_picture.data[0];
> +    srcU = s->last_picture.data[1];
> +    srcV = s->last_picture.data[2];
> +
> +    if(s->mspel) { // hpel mc

MpegEncContext->mspel==1 is not hpel but microsofts silly
horizontal qpel with vertical hpel mode they used in wmv2


[...]

> +        escape = decode210(gb);
> +        if (escape == 0) {
> +            index = get_vlc2(gb, vc1_ac_coeff_table[codingset].table, AC_VLC_BITS, 3);
> +            run = vc1_index_decode_table[codingset][index][0];
> +            level = vc1_index_decode_table[codingset][index][1];
> +            lst = index >= vc1_last_decode_table[codingset];
> +            if(lst)
> +                level += vc1_last_delta_level_table[codingset][run];
> +            else
> +                level += vc1_delta_level_table[codingset][run];
> +            if(get_bits(gb, 1))
> +                level = -level;
> +        } else if (escape == 1) {
> +            index = get_vlc2(gb, vc1_ac_coeff_table[codingset].table, AC_VLC_BITS, 3);
> +            run = vc1_index_decode_table[codingset][index][0];
> +            level = vc1_index_decode_table[codingset][index][1];
> +            lst = index >= vc1_last_decode_table[codingset];
> +            if(lst)
> +                run += vc1_last_delta_run_table[codingset][level] + 1;
> +            else
> +                run += vc1_delta_run_table[codingset][level] + 1;
> +            if(get_bits(gb, 1))
> +                level = -level;
> +        } else {
> +            int sign;
> +            lst = get_bits(gb, 1);
> +            if(v->s.esc3_level_length == 0) {
> +                if(v->pq < 8 || v->dquantfrm) { // table 59
> +                    v->s.esc3_level_length = get_bits(gb, 3);
> +                    if(!v->s.esc3_level_length)
> +                        v->s.esc3_level_length = get_bits(gb, 2) + 8;
> +                } else { //table 60
> +                    v->s.esc3_level_length = get_prefix(gb, 1, 6) + 2;
> +                }
> +                v->s.esc3_run_length = 3 + get_bits(gb, 2);
> +            }
> +            run = get_bits(gb, v->s.esc3_run_length);
> +            sign = get_bits(gb, 1);
> +            level = get_bits(gb, v->s.esc3_level_length);
> +            if(sign)
> +                level = -level;
> +        }


this could be simplified like:

if(escape!=2){
    index = get_vlc2(gb, vc1_ac_coeff_table[codingset].table, AC_VLC_BITS, 3);
    run = vc1_index_decode_table[codingset][index][0];
    level = vc1_index_decode_table[codingset][index][1];
    lst = index >= vc1_last_decode_table[codingset];
    if(escape==0){
        ...
    }else{
        ...
    }
    if(get_bits(gb, 1))
        level = -level;
}else{
...
}

[...]

-- 
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




More information about the ffmpeg-cvslog mailing list