[FFmpeg-devel] [PATCH] RV40 Loop Filter
Kostya
kostya.shishkov
Mon Oct 27 16:12:33 CET 2008
On Mon, Oct 27, 2008 at 03:55:38PM +0100, Michael Niedermayer wrote:
> On Sun, Oct 26, 2008 at 03:41:09PM +0200, Kostya wrote:
[...]
>
> have you confirmed that this loop filter is bit exact?
> Id like to make really sure that this is correct before i spend time
> on finding a clean implementation.
Yes, it is. But as I've mentioned, there is simpler and saner filter
there which I tend to use instead of this.
> [...]
> > + if(s->width * s->height <= 0x6300){
> > + betaY += beta;
> > + }
>
> 0x6300= 176*144
>
> [...]
> > + c_v_deblock[i] = ((uvcbp[0][i] << 1) & ~0x5) | (uvcbp[2][i] >> 1)
> > + | (uvcbp[3][i] << 4) | uvcbp[0][i];
> > + c_h_deblock[i] = (uvcbp[3][i] << 4) | uvcbp[0][i] | (uvcbp[1][i] >> 2)
> > + | (uvcbp[3][i] << 6) | (uvcbp[0][i] << 2);
> > + uvcbp[0][i] = (uvcbp[3][i] << 4) | uvcbp[0][i];
>
> this can be simplified a lot, the term "(uvcbp[3][i] << 4) | uvcbp[0][i]"
> occurs several times in there
> besides id like to repeat that this should be using a 2d array NOT bits.
> if you insist on using bits, then they MUST be documented, that is each bit
> must be documented so its clear to which block or edge it belongs.
those are rather obvious, it's *_deblock variables that give me a headache
> > + if(!s->mb_x){
> > + c_v_deblock[i] &= ~0x5;
> > + }
> > + if(!s->mb_y){
> > + c_h_deblock[i] &= ~0x3;
> > + }
>
> > + if(s->mb_y == s->mb_height - 1 || mbtype[0] == 2 || mbtype[3] == 2){
>
> mbtype should never be compared against litteral numbers, always
> named values like from an enum.
that's a misleading name here, sorry
first I store real macroblock type here, then just something
resembling filtering strength in H264 draft (=1 for most MB types,
=2 for intra MBs and 1MV P-frame interblock with separately coded
DCs).
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
More information about the ffmpeg-devel
mailing list