[FFmpeg-devel] [PATCH i/N] RV30 loop filter

[Michael Niedermayer]

On Sat, Mar 08, 2008 at 07:15:18PM +0200, Kostya wrote:
> On Sun, Feb 24, 2008 at 12:37:24AM +0100, Michael Niedermayer wrote:
> > On Sat, Feb 23, 2008 at 12:57:29PM +0200, Kostya wrote:
> > > $subj
> > > 
> > > Probably correct.
> > 
> [...]
> > > +static void rv30_loop_filter(RV34DecContext *r)
> > > +{
> > > +    MpegEncContext *s = &r->s;
> > > +    int mb_pos;
> > > +    int i, j;
> > 
> > > +    int strength = 0;//FIXME how to determine correct value?
> > > +    const uint8_t* lim = rv30_loop_filt_lim[strength];
> > 
> > This does not look correct.
> 
> I believe it is - rv30_loop_filt_lim is an array of 32-element mapping
> tables between quantizer and limit values.

I mean strength=0 above vs. your claim about "Probably correct." above.
This just does not look correct at all, the FIXME also contradicts that this
is correct.


[...]
> > > +                cbp1 = r->cbp_chroma[mb_pos - 1];
> > > +                cbp2 = r->cbp_chroma[mb_pos];
> > > +                for(i = 0; i < 8; i += 4, cbp1 >>= 2, cbp2 >>= 2){
> > > +                    if((cbp1 & 0x02) && (cbp2 & 0x01))
                                            ^^
> > > +                        rv30_weak_loop_filter(s->dest[1] + i * s->uvlinesize, 1, s->uvlinesize, lim[q]);
> > > +                    if((cbp1 & 0x20) && (cbp2 & 0x10))
> > > +                        rv30_weak_loop_filter(s->dest[2] + i * s->uvlinesize, 1, s->uvlinesize, lim[q]);
> > > +                }
> > > +            }
> > > +            q = s->current_picture_ptr->qscale_table[mb_pos];
> > > +            for(j = 4; j < 16; j += 4){
> > > +                cbp1 = r->cbp_luma[mb_pos] >> ((j >> 2) - 1);
> > > +                for(i = 0; i < 16; i += 4, cbp1 >>= 4)
> > > +                    if(cbp1 & 3)
> > > +                        rv30_weak_loop_filter(s->dest[0] + j + i * s->linesize, 1, s->linesize, lim[q]);
> > > +            }
> > > +            cbp1 = r->cbp_chroma[mb_pos];
> > > +            for(i = 0; i < 8; i += 4, cbp1 >>= 2){
> > > +                if(cbp1 & 0x03)
                          ^^^^^^^^^^^
> > > +                    rv30_weak_loop_filter(s->dest[1] + 4 + i * s->uvlinesize, 1, s->uvlinesize, lim[q]);
> > > +                if(cbp1 & 0x30)
> > > +                    rv30_weak_loop_filter(s->dest[2] + 4 + i * s->uvlinesize, 1, s->uvlinesize, lim[q]);
> > > +            }
> > 
> > This code does cbpA || cbpB the one above does cbpA && cbpB, this is strange.
> 
> before it was luma, here it is chroma and I just merged U and V filter calls together

The first code excutes rv30_weak_loop_filter when both blocks are coded
The second code excutes rv30_weak_loop_filter when either block is coded
They are both for chroma


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

It is not what we do, but why we do it that matters.
-------------- 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/20080308/b5732a25/attachment.pgp>