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

[Michael Niedermayer]

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:
[...]

> Index: libavcodec/rv40.c
> ===================================================================
> --- libavcodec/rv40.c	(revision 15732)
> +++ libavcodec/rv40.c	(working copy)
> @@ -247,7 +247,426 @@
>      return 0;
>  }
>  
> +#define CLIP_SYMM(a, b) av_clip(a, -(b), b)

ok


>  /**
> + * weaker deblocking very similar to the one described in 4.4.2 of JVT-A003r1
> + */
> +static inline void rv40_weak_loop_filter(uint8_t *src, const int step,
> +                                         const int filter_p1, const int filter_q1,
> +                                         const int alpha, const int beta,
> +                                         const int lim0, const int lim1, const int lim2,
> +                                         const int diff_p1p0, const int diff_q1q0,
> +                                         const int diff_p1p2, const int diff_q1q2)
> +{
> +    uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;
> +    int t, u, diff;
> +
> +    t = src[0*step] - src[-1*step];
> +    if(!t)
> +        return;
> +    u = (alpha * FFABS(t)) >> 7;
> +    if(u > 3 - (filter_p1 && filter_q1))
> +        return;
> +
> +    t <<= 2;
> +    if(filter_p1 && filter_q1)
> +        t += src[-2*step] - src[1*step];
> +    diff = CLIP_SYMM((t + 4) >> 3, lim0);
> +    src[-1*step] = cm[src[-1*step] + diff];
> +    src[ 0*step] = cm[src[ 0*step] - diff];
> +    if(FFABS(diff_p1p2) <= beta && filter_p1){
> +        t = (diff_p1p0 + diff_p1p2 - diff) >> 1;
> +        src[-2*step] = cm[src[-2*step] - CLIP_SYMM(t, lim2)];
> +    }
> +    if(FFABS(diff_q1q2) <= beta && filter_q1){
> +        t = (diff_q1q0 + diff_q1q2 + diff) >> 1;
> +        src[ 1*step] = cm[src[ 1*step] - CLIP_SYMM(t, lim1)];
> +    }
> +}
> +

lim0,1,2 are pretty bad names


[...]
> +            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.


[...]
> +            /* 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

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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081114/b5a13f2c/attachment.pgp>