[FFmpeg-devel] [PATCH] RV40 Loop Filter (again)
Michael Niedermayer
michaelni
Sat Nov 15 11:10:26 CET 2008
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:
> [...]
> > > /**
> > > + * 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
>
> what about lim_p0q0, lim_p1 and lim_q1 ?
better than lim012
>
> > [...]
> > > + 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;
>
> > [...]
> > > + /* 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.
Also is the decoder bitexact already?
If not, why not?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I wish the Xiph folks would stop pretending they've got something they
do not. Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- 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/20081115/cbf617b5/attachment.pgp>
More information about the ffmpeg-devel
mailing list