[FFmpeg-devel] [PATCH] RV40 Loop Filter (again)

Kostya kostya.shishkov
Sat Nov 15 11:49:47 CET 2008


On Sat, Nov 15, 2008 at 11:10:26AM +0100, Michael Niedermayer wrote:
> On Sat, Nov 15, 2008 at 11:27:41AM +0200, Kostya wrote:
> > On Fri, Nov 14, 2008 at 06:23:30PM +0100, Michael Niedermayer wrote:
> > > On Fri, Nov 14, 2008 at 09:14:46AM +0200, Kostya wrote:
> > > > On Wed, Nov 12, 2008 at 07:46:32PM +0100, Michael Niedermayer wrote:
> > > > > On Wed, Nov 12, 2008 at 09:05:11AM +0200, Kostya wrote:
[...]
> > > > +            p0 = RV40_STRONG_FILTER(src, step, -1, rv40_dither_l[dmode + i]);
> > > > +            p1 = RV40_STRONG_FILTER(src, step,  0, rv40_dither_r[dmode + i]);
> > > > +            diff[0] = src[-1*step];
> > > > +            diff[1] = src[ 0*step];
> > > > +            src[-1*step] = sflag ? av_clip(p0, src[-1*step] - lims, src[-1*step] + lims) : p0;
> > > > +            src[ 0*step] = sflag ? av_clip(p1, src[ 0*step] - lims, src[ 0*step] + lims) : p1;
> > > > +            diff[0] -= src[-1*step];
> > > > +            diff[1] -= src[ 0*step];
> > > > +            p0 = RV40_STRONG_FILTER(src, step, -2, rv40_dither_l[dmode + i] + diff[1]*25);
> > > > +            p1 = RV40_STRONG_FILTER(src, step,  1, rv40_dither_r[dmode + i] + diff[0]*25);
> > > 
> > > I have my doubts about the order of operations being correct
> > > are you sure its not
> > > a = RV40_STRONG_FILTER(src, step, -2, rv40_dither_l[dmode + i]);
> > > b = RV40_STRONG_FILTER(src, step, -1, rv40_dither_l[dmode + i]);
> > > c = RV40_STRONG_FILTER(src, step,  0, rv40_dither_r[dmode + i]);
> > > d = RV40_STRONG_FILTER(src, step,  1, rv40_dither_r[dmode + i]);
> > > ...
> > > ?
> > > 
> > > also the diff[] just compensates for the value being overwritten due to the
> > > order of operations.
> > 
> > I've verified that against binary decoder.
> > As you can see, all filters are interlocked since they all use src[-2], src[-1], src[0]
> > and src[1] values but only one of them is used with the old value (the farthest one in
> > filtering operation).
> > I don't see any easy way to untangle them.
> 
> p0= (26*(s[-1] + s[ 0] + s[1]) + 25*(s[-2] + s[2]) + offl)>>7;
> q0= (26*(s[ 0] + s[ 1] + s[2]) + 25*(s[-1] + s[3]) + offr)>>7;
> p0= clipwhatever(p0)
> q0= clipwhatever(q0)
> p1= (26*(s[-2] + s[-1] + p0  ) + 25*(s[-3] + s[1]) + offl)>>7;
> q1= (26*(q0    + s[ 2] + s[3]) + 25*(s[ 0] + s[4]) + offr)>>7;
 
Exactly. But it's easier to me to include difference into bias then change filter. 
 
> > > [...]
> > > > +            /* This pattern contains bits signalling that horizontal edges of
> > > > +             * the current block can be filtered.
> > > > +             * That happens when either of adjacent subblocks is coded or lies on
> > > > +             * the edge of 8x8 blocks with motion vectors differing by more than
> > > > +             * 3/4 pel in any component.
> > > > +             */
> > > > +            y_h_deblock =   cbp[POS_CUR]
> > > > +                        | ((cbp[POS_BOTTOM]     & MASK_Y_TOP_ROW)  << 16)
> > > > +                        | ((cbp[POS_CUR]                           <<  4) & ~MASK_Y_TOP_ROW)
> > > > +                        | ((cbp[POS_TOP]        & MASK_Y_LAST_ROW) >> 12)
> > > > +                        |   mvmasks[POS_CUR]
> > > > +                        | ((mvmasks[POS_BOTTOM] & MASK_Y_TOP_ROW)  << 16);
> > > > +            /* This pattern contains bits signalling that vertical edges of
> > > > +             * the current block can be filtered.
> > > > +             * That happens when either of adjacent subblocks is coded or lies on
> > > > +             * the edge of 8x8 blocks with motion vectors differing by more than
> > > > +             * 3/4 pel in any component.
> > > > +             */
> > > > +            y_v_deblock =   cbp[POS_CUR]
> > > > +                        | ((cbp[POS_CUR]                      << 1) & ~MASK_Y_LEFT_COL)
> > > > +                        | ((cbp[POS_LEFT] & MASK_Y_RIGHT_COL) >> 3)
> > > > +                        |   mvmasks[POS_CUR];
> > > 
> > > the text and mvmasks do not match
> > 
> > why? RV40 uses combined mvmask for both horizontal and vertical MV.
> 
> The text does not speak of combining anything, and the combination must be
> documented very exactly IMHO.

It says about edge of blocks with differing MVs, nothing is said about the direction,
so it's any edge. 

> Also is the decoder bitexact already?
> If not, why not?

I planned to verify another cases.
For now I can name only one known source of inexactness: motion compensation for luma.
H.264 uses 
(src[0] + (1*src[-2] - 5*src[-1] + 20*src[0] + 20*src[1] - 5*src[2] + src[3] + 16)>>5 + 1) >> 1

RV40 uses

(1*src[-2] - 5*src[-1] + 52*src[0] + 20*src[1] - 5*src[2] + src[3] + 32) >> 6

For example,
20 20 20 |20| 22 23 23
results in 21 for H.264 and 20 for RV40

> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB




More information about the ffmpeg-devel mailing list