[FFmpeg-devel] [PATCH] RV40 Loop Filter

[Aurelien Jacobs]

Michael Niedermayer wrote:

> On Sun, Oct 26, 2008 at 03:41:09PM +0200, Kostya wrote:
> > On Sat, Oct 25, 2008 at 11:14:25AM +0200, Michael Niedermayer wrote:
> > > On Sat, Oct 25, 2008 at 10:08:44AM +0300, Kostya wrote:
> > > > On Wed, Oct 22, 2008 at 10:53:23AM +0200, Michael Niedermayer wrote:
> > > > > On Tue, Oct 21, 2008 at 09:23:21AM +0300, Kostya wrote:
> > > [...]
> > > > > [...]
> > > > > > +static int rv40_set_deblock_coef(RV34DecContext *r)
> > > > > > +{
> > > > > > +    MpegEncContext *s = &r->s;
> > > > > > +    int mvmask = 0, i, j, dx, dy;
> > > > > > +    int midx = s->mb_x * 2 + s->mb_y * 2 * s->b8_stride;
> > > > > 
> > > > > > +    if(s->pict_type == FF_I_TYPE)
> > > > > > +        return 0;
> > > > > 
> > > > > why is this even called for i frames?
> > > > 
> > > > I intend to use it for calculating macroblock-specific deblock
> > > > strength in RV30.
> > > 
> > > fine but how is that related to having the pict_type check inside the
> > > function compared to outside?
> >  
> > For RV30 setting deblock coefficients would be performed for
> > I-frames as well.
> 
> so there are 2 different functions
> 
> if(rv30)
>     rv30_set_deblock_coef()
> else if(!I)
>     rv40_set_deblock_coef()
> 
> clean, simple, fast, ...

This would be more like:

if(ENABLE_RV30_DECODER && rv30)
    rv30_set_deblock_coef();
else if(ENABLE_RV40_DECODER && !I)
    rv40_set_deblock_coef();

> vs.
> 
> ctx->func_ptr()
> 
> init(){
>     if(rv30)
>         ctx->func_ptr= func30
>     else
>         ctx->func_ptr= func40
> }

In practice, it is in fact:

rv30_init(){
    ctx->func_ptr= func30;
}

rv40_init(){
    ctx->func_ptr= func40;
}

> func40(){
>     if(I)
>         return;
> }
> 
> This is not simple,

At least it's cleaner regarding separation of codecs in their own
compilation unit (ie. it doesn't require conditional compilation
of some blocks of code).
It also spare one if() per macroblock. Probably not very relevant,
speedwise, but still...

> and calling functions that just return is IMHO also not clean.

I tend to agree.
But I think the alternative, is not that much cleaner.

Aurel