[FFmpeg-cvslog] r15786 - in trunk/libavcodec: rv34.c rv34.h

[Kostya]

On Fri, Nov 07, 2008 at 11:57:47PM +0100, Michael Niedermayer wrote:
> On Fri, Nov 07, 2008 at 08:18:22AM +0100, kostya wrote:
> > Author: kostya
> > Date: Fri Nov  7 08:18:22 2008
> > New Revision: 15786
> > 
> > Log:
> > Calculating an additional MV-based deblocking pattern is the same
> > for both RV3 and RV4, so move it into common code.
> [...]
> > +static int rv34_set_deblock_coef(RV34DecContext *r)
> > +{
> > +    MpegEncContext *s = &r->s;
> > +    int mvmask = 0, i, j;
> > +    int midx = s->mb_x * 2 + s->mb_y * 2 * s->b8_stride;
> > +    int16_t (*motion_val)[2] = s->current_picture_ptr->motion_val[0][midx];
> 
> > +    if(s->pict_type == FF_I_TYPE)
> > +        return 0;
> 
> i dont think this is needed anymore
 
of course, will drop soon
 
> > +    for(j = 0; j < 16; j += 8){
> > +        for(i = 0; i < 2; i++){
> > +            if(is_mv_diff_gt_3(motion_val + i, 1))
> > +                mvmask |= 0x11 << (j + i*2);
> > +            if(is_mv_diff_gt_3(motion_val + i, s->b8_stride))
> > +                mvmask |= 0x03 << (j + i*2);
> > +        }
> > +        motion_val += s->b8_stride;
> > +    }
> > +    return mvmask;
> > +}
> 
> have you confirmed that this is correct and that unrelated bits really
> are overwriting each other?
> what iam trying to say is that these 2 if() should write in seperate
> variables i think, there should be IMO 1 mvmask for vertical and one
> for horizontal edges.

Yes, calculating two separate masks should be done here.
As a side note, while RV30 stores result into separate masks, RV40 combines
them into single mask. While I understand their logic sometimes (when it's
written in H.264 drafts), most cases seems like alien logic. 
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB