[FFmpeg-devel] [PATCH] RV40 Loop Filter

[Kostya]

On Sun, Oct 26, 2008 at 05:52:25PM +0100, 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.
> 
> What this function does is comparing motion vectors, there are no motion
> vectors in I frames.
 
In RV40 - yes, in RV30 - looks like it does something else
(for I-frames as well).
 
> [...]
> > > [...]
> > > > [lots of loop filter invoking] 
> > > > > 
> > > > > the word mess is probably the best way to describe this
> > > > > as far as i can tell you are packing all the bits related to deblocking
> > > > > and then later duplicate code each with hardcoded masks to extract them
> > > > > again.
> > > > 
> > > > We have a saying here "To make a candy from crap", which I think describes
> > > > current situation. I'd like to shot the group of men who proposed the loop
> > > > filter in the form RV40 has it.
> > > 
> > > there arent many codecs around that are cleanly designed ...
> > > Some things here and there are ok but terrible messes like this are more
> > > common.
> > > We dont have too much of a choice, to support things the mess has to be
> > > implemented. If it can be done cleaner/simpler thats a big advantage in the
> > > long term, easier to maintain, understand, optimize; smaller and faste, ...
> > 
> > Also I think that forcing someone to understand it counts as
> > a psychological abuse and the sentence on it should be
> > debugging X8 frames or implementing interlaced mode in VC-1
> > (sorry, can't remember more evil codecs).
> 
> Theres a problem with X8 ? I thought it was implemented correctly ...

They are, but there's a problem with them - they exist.
And you should remember what stopped people from REing them sooner.

Bink video gives me the similar feeling.
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB