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

[Kostya]

On Tue, Jun 27, 2006 at 03:39:02PM +0200, Michael Niedermayer wrote:
> 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

OK, I will
> 
> 
> [...]
> 
> > +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?

That code is old one from vc9. I didn't wrote and even touched it. I also think that
those bitplanes will be eventually replaced with MpegEncContext variables.
> 
> 
> [...]
[3x2 and 2x3 tiling code skipped]
> and store_3x2(1, bp->stride) and store_3x2(bp->stride, 1)
> 
> of coarse maybe thats not worth the mess ...

I think so. It is not reused anywhere else.
> 
> 
> [...]
> > +/** 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

Original code used this mspel to flag that MV is really hpels and not quadpels.
I'll use MpegEncContext->quarter_sample in the future.
> 
> 
> [...]
> 
> 
[AC coefficient decoding skipped]
> 
> 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{
> ...
> }

Right, I just adapted this one from standard.
> 
> [...]
> 
> -- 
> 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
>