Here is my attempt to use RV30 1/3pel MC in the same way as standard H.264 ones.
On Sun, Dec 23, 2007 at 04:33:04PM +0200, Kostya wrote:
Here is my attempt to use RV30 1/3pel MC in the same way as standard H.264 ones.
[...]
+#define MOD3(a) ((((a) % 3) + 3) % 3) + /** * generic motion compensation function *
@@ -572,21 +574,15 @@ int is16x16 = 1;
if(thirdpel){ -#if 0 /// todo int lx, ly;
mx = s->current_picture_ptr->motion_val[dir][mv_pos][0] / 3; my = s->current_picture_ptr->motion_val[dir][mv_pos][1] / 3; - lx = ((s->current_picture_ptr->motion_val[dir][mv_pos][0] % 3) + 3) % 3; - ly = ((s->current_picture_ptr->motion_val[dir][mv_pos][1] % 3) + 3) % 3; - dxy = ly*3 + lx; - uvmx = -#endif - mx = s->current_picture_ptr->motion_val[dir][mv_pos][0] >> 2; - my = s->current_picture_ptr->motion_val[dir][mv_pos][1] >> 2; - dxy = ((my & 3) << 2) | (mx & 3); - uvmx = mx & 6; - uvmy = my & 6; + lx = MOD3(s->current_picture_ptr->motion_val[dir][mv_pos][0]); + ly = MOD3(s->current_picture_ptr->motion_val[dir][mv_pos][1]); + dxy = ly*4 + lx; + uvmx = MOD3(s->current_picture_ptr->motion_val[dir][mv_pos][0] >> 1); + uvmy = MOD3(s->current_picture_ptr->motion_val[dir][mv_pos][1] >> 1); }else{ mx = s->current_picture_ptr->motion_val[dir][mv_pos][0] >> 2; my = s->current_picture_ptr->motion_val[dir][mv_pos][1] >> 2;
something doesnt look correct on this the mx/my are not rounded the same way as lx/ly, this cant be correct what you likely want is mx= (A + (3<<24)) / 3 - (1<<24); my= (B + (3<<24)) / 3 - (1<<24); lx= (A + (3<<24)) % 3; ly= (B + (3<<24)) % 3; also this is a duplication of svq3_mc_dirs THIRDPEL_MODE
@@ -643,15 +639,24 @@ const int width, const int height, int dir) { rv34_mc(r, block_type, xoff, yoff, mv_off, width, height, dir, r->rv30, - r->s.dsp.put_h264_qpel_pixels_tab, r->s.dsp.put_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.put_rv30_qpel_pixels_tab + : r->s.dsp.put_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.put_rv30_chroma_pixels_tab + : r->s.dsp.put_h264_chroma_pixels_tab); }
static void rv34_mc_2mv(RV34DecContext *r, const int block_type) { rv34_mc(r, block_type, 0, 0, 0, 2, 2, 0, r->rv30, - r->s.dsp.put_h264_qpel_pixels_tab, r->s.dsp.put_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.put_rv30_qpel_pixels_tab + : r->s.dsp.put_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.put_rv30_chroma_pixels_tab + : r->s.dsp.put_h264_chroma_pixels_tab); rv34_mc(r, block_type, 0, 0, 0, 2, 2, 1, r->rv30, - r->s.dsp.avg_h264_qpel_pixels_tab, r->s.dsp.avg_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.avg_rv30_qpel_pixels_tab + : r->s.dsp.avg_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.avg_rv30_chroma_pixels_tab + : r->s.dsp.avg_h264_chroma_pixels_tab); }
qpel stands for quarter pel -> 1/4 pel third pel would be tpel :) [...]
+#define op_avg(a, b) a = (((a)+cm[((b) + 32)>>6]+1)>>1) +#define op_put(a, b) a = cm[((b) + 32)>>6] + +static const int coeffs[4] = {8, 5, 3, 0}; + +#define RV30_CHROMA(OPNAME, OP) \ +static void OPNAME ## rv30_chroma_mc4_c(uint8_t *dst/*align 8*/, uint8_t *src/*align 1*/, int stride, int h, int x, int y){\ + const int A=coeffs[ x]*coeffs[ y];\ + const int B=coeffs[3-x]*coeffs[ y];\ + const int C=coeffs[ x]*coeffs[3-y];\ + const int D=coeffs[3-x]*coeffs[3-y];\ + uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;\ + int i;\ +\ + for(i=0; i<h; i++)\ + {\ + OP(dst[0], (A*src[0] + B*src[1] + C*src[stride+0] + D*src[stride+1]));\ + OP(dst[1], (A*src[1] + B*src[2] + C*src[stride+1] + D*src[stride+2]));\ + OP(dst[2], (A*src[2] + B*src[3] + C*src[stride+2] + D*src[stride+3]));\ + OP(dst[3], (A*src[3] + B*src[4] + C*src[stride+3] + D*src[stride+4]));\ + dst+= stride;\ + src+= stride;\ + }\ +}\ +\ +static void OPNAME ## rv30_chroma_mc8_c(uint8_t *dst/*align 8*/, uint8_t *src/*align 1*/, int stride, int h, int x, int y){\ + const int A=coeffs[ x]*coeffs[ y];\ + const int B=coeffs[3-x]*coeffs[ y];\ + const int C=coeffs[ x]*coeffs[3-y];\ + const int D=coeffs[3-x]*coeffs[3-y];\ + uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;\ + int i;\ +\ + for(i=0; i<h; i++)\ + {\ + OP(dst[0], (A*src[0] + B*src[1] + C*src[stride+0] + D*src[stride+1]));\ + OP(dst[1], (A*src[1] + B*src[2] + C*src[stride+1] + D*src[stride+2]));\ + OP(dst[2], (A*src[2] + B*src[3] + C*src[stride+2] + D*src[stride+3]));\ + OP(dst[3], (A*src[3] + B*src[4] + C*src[stride+3] + D*src[stride+4]));\ + OP(dst[4], (A*src[4] + B*src[5] + C*src[stride+4] + D*src[stride+5]));\ + OP(dst[5], (A*src[5] + B*src[6] + C*src[stride+5] + D*src[stride+6]));\ + OP(dst[6], (A*src[6] + B*src[7] + C*src[stride+6] + D*src[stride+7]));\ + OP(dst[7], (A*src[7] + B*src[8] + C*src[stride+7] + D*src[stride+8]));\ + dst+= stride;\ + src+= stride;\ + }\ +}\ +\
this looks like a duplication of the h264 chroma mc functions, you dont need new functions for mapping 1->3, 2->5, 3->8 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates
On Sun, Dec 23, 2007 at 05:36:56PM +0100, Michael Niedermayer wrote:
On Sun, Dec 23, 2007 at 04:33:04PM +0200, Kostya wrote:
Here is my attempt to use RV30 1/3pel MC in the same way as standard H.264 ones.
[MC parameters calculation]
something doesnt look correct on this the mx/my are not rounded the same way as lx/ly, this cant be correct what you likely want is mx= (A + (3<<24)) / 3 - (1<<24); my= (B + (3<<24)) / 3 - (1<<24); lx= (A + (3<<24)) % 3; ly= (B + (3<<24)) % 3;
also this is a duplication of svq3_mc_dirs THIRDPEL_MODE
It is very similar indeed but mx/my calculation in svq3 differs.
@@ -643,15 +639,24 @@ const int width, const int height, int dir) { rv34_mc(r, block_type, xoff, yoff, mv_off, width, height, dir, r->rv30, - r->s.dsp.put_h264_qpel_pixels_tab, r->s.dsp.put_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.put_rv30_qpel_pixels_tab + : r->s.dsp.put_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.put_rv30_chroma_pixels_tab + : r->s.dsp.put_h264_chroma_pixels_tab); }
static void rv34_mc_2mv(RV34DecContext *r, const int block_type) { rv34_mc(r, block_type, 0, 0, 0, 2, 2, 0, r->rv30, - r->s.dsp.put_h264_qpel_pixels_tab, r->s.dsp.put_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.put_rv30_qpel_pixels_tab + : r->s.dsp.put_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.put_rv30_chroma_pixels_tab + : r->s.dsp.put_h264_chroma_pixels_tab); rv34_mc(r, block_type, 0, 0, 0, 2, 2, 1, r->rv30, - r->s.dsp.avg_h264_qpel_pixels_tab, r->s.dsp.avg_h264_chroma_pixels_tab); + r->rv30 ? r->s.dsp.avg_rv30_qpel_pixels_tab + : r->s.dsp.avg_h264_qpel_pixels_tab, + r->rv30 ? r->s.dsp.avg_rv30_chroma_pixels_tab + : r->s.dsp.avg_h264_chroma_pixels_tab); }
qpel stands for quarter pel -> 1/4 pel third pel would be tpel :)
I made it too similar then. [chroma MC]
this looks like a duplication of the h264 chroma mc functions, you dont need new functions for mapping 1->3, 2->5, 3->8
Now it is done along with other parameters calculation.
[...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the % [...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well [...]
+static void OPNAME ## rv30_tpel8_hv_lowpass(uint8_t *dst, uint8_t *src, int dstStride, int srcStride){\ + const int h=8;\ + uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;\ + int i;\ + for(i=0; i<h; i++)\ + {\ + OP2(dst[0], 36*src[0]+54*src[1]+6*src[2]+54*src[0+srcStride]+81*src[1+srcStride]+9*src[2+srcStride]+6*src[0+srcStride*2]+9*src[1+srcStride*2]+src[2+srcStride*2]);\
instead of 9 read, 8 mul (or 5 if factored a little), 8 add, 1 store per pixel this can be implemented as 6 read, 4 multiply, 4 add, 2 store by simply spliting the filter in horizontal and vetical [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
[...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
[...]
+static void OPNAME ## rv30_tpel8_hv_lowpass(uint8_t *dst, uint8_t *src, int dstStride, int srcStride){\ + const int h=8;\ + uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;\ + int i;\ + for(i=0; i<h; i++)\ + {\ + OP2(dst[0], 36*src[0]+54*src[1]+6*src[2]+54*src[0+srcStride]+81*src[1+srcStride]+9*src[2+srcStride]+6*src[0+srcStride*2]+9*src[1+srcStride*2]+src[2+srcStride*2]);\
instead of 9 read, 8 mul (or 5 if factored a little), 8 add, 1 store per pixel this can be implemented as 6 read, 4 multiply, 4 add, 2 store by simply spliting the filter in horizontal and vetical
Here it is.
[...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
On Mon, Dec 31, 2007 at 09:17:59AM +0200, Kostya wrote:
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
ohh yes it can, heres a test program with a simpler variant without the >> ----- static const int chroma_coeffs2[6] = { 8, 8, 5, 5, 3, 3}; main(){ int v; for(v=-10000; v<10000; v++){ int mx = (v + (3 << 24)) / 3 - (1 << 24); int lx = (v + (3 << 24)) % 3; int uvmx = chroma_coeffs[((v + (3 << 24)) >> 1) % 3]; int uvmx2= chroma_coeffs2[3*(mx&1) + lx]; if(uvmx != uvmx2) printf("%d %d %d %d %d\n", v, mx, lx, uvmx, uvmx2); } } -----
[...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
well but the hunk above does change dsputil.h [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
On Wed, Jan 02, 2008 at 11:12:55PM +0100, Michael Niedermayer wrote:
On Mon, Dec 31, 2007 at 09:17:59AM +0200, Kostya wrote:
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
ohh yes it can, heres a test program with a simpler variant without the >> [code skipped]
My test were flawed then. Sorry.
[...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
well but the hunk above does change dsputil.h
Here it is without dsputil stuff.
[...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
On Thu, Jan 03, 2008 at 02:09:15PM +0200, Kostya wrote:
On Wed, Jan 02, 2008 at 11:12:55PM +0100, Michael Niedermayer wrote:
On Mon, Dec 31, 2007 at 09:17:59AM +0200, Kostya wrote:
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
ohh yes it can, heres a test program with a simpler variant without the >> [code skipped]
My test were flawed then. Sorry.
[...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
well but the hunk above does change dsputil.h
Here it is without dsputil stuff.
it wont compile without it [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates
On Thu, Jan 03, 2008 at 09:11:30PM +0100, Michael Niedermayer wrote:
On Thu, Jan 03, 2008 at 02:09:15PM +0200, Kostya wrote:
On Wed, Jan 02, 2008 at 11:12:55PM +0100, Michael Niedermayer wrote:
On Mon, Dec 31, 2007 at 09:17:59AM +0200, Kostya wrote:
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...]
+ mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; + dxy = ly*4 + lx;
+ uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
ohh yes it can, heres a test program with a simpler variant without the >> [code skipped]
My test were flawed then. Sorry.
[...]
@@ -422,6 +422,10 @@ void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, int * range, int * sum, int edges);
+ /* rv30 functions */ + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
well but the hunk above does change dsputil.h
Here it is without dsputil stuff.
it wont compile without it
In the current configuration it will not be compiled at all. I plan to add those dsputil chunks along with decoder enablement.
[...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
On Fri, Jan 04, 2008 at 10:01:55AM +0200, Kostya wrote:
On Thu, Jan 03, 2008 at 09:11:30PM +0100, Michael Niedermayer wrote:
On Thu, Jan 03, 2008 at 02:09:15PM +0200, Kostya wrote:
On Wed, Jan 02, 2008 at 11:12:55PM +0100, Michael Niedermayer wrote:
On Mon, Dec 31, 2007 at 09:17:59AM +0200, Kostya wrote:
On Wed, Dec 26, 2007 at 03:38:35AM +0100, Michael Niedermayer wrote:
On Mon, Dec 24, 2007 at 01:04:00PM +0200, Kostya wrote: [...] > + mx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) / 3 - (1 << 24); > + my = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) / 3 - (1 << 24); > + lx = (s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) % 3; > + ly = (s->current_picture_ptr->motion_val[dir][mv_pos][1] + (3 << 24)) % 3; > + dxy = ly*4 + lx;
> + uvmx = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3]; > + uvmy = chroma_coeffs[((s->current_picture_ptr->motion_val[dir][mv_pos][0] + (3 << 24)) >> 1) % 3];
this is wrong ([0] vs. [1]) also i think (3*(mx&1) + lx)>>1 could be used to avoid the %
Umm, it can't
ohh yes it can, heres a test program with a simpler variant without the >> [code skipped]
My test were flawed then. Sorry.
[...]
> @@ -422,6 +422,10 @@ > void (*x8_setup_spatial_compensation)(uint8_t *src, uint8_t *dst, int linesize, > int * range, int * sum, int edges); > > + /* rv30 functions */ > + qpel_mc_func put_rv30_tpel_pixels_tab[4][16]; > + qpel_mc_func avg_rv30_tpel_pixels_tab[4][16]; > +
this could be put under CONFIG_RV30_DECODER as well
I won't touch dsputil.* until decoder is fully integrated.
well but the hunk above does change dsputil.h
Here it is without dsputil stuff.
it wont compile without it
In the current configuration it will not be compiled at all. I plan to add those dsputil chunks along with decoder enablement.
then the patch is ok [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable
On Fri, Jan 04, 2008 at 12:06:11PM +0100, Michael Niedermayer wrote: [...]
In the current configuration it will not be compiled at all. I plan to add those dsputil chunks along with decoder enablement.
then the patch is ok
committed
[...]
-- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
+#define op_avg(a, b) a = (((a)+cm[((b) + 32)>>6]+1)>>1) +#define op_put(a, b) a = cm[((b) + 32)>>6] + +static const int coeffs[4] = {8, 5, 3, 0}; + +#define RV30_CHROMA(OPNAME, OP) \ +static void OPNAME ## rv30_chroma_mc4_c(uint8_t *dst/*align 8*/, uint8_t *src/*align 1*/, int stride, int h, int x, int y){\ + const int A=coeffs[ x]*coeffs[ y];\ + const int B=coeffs[3-x]*coeffs[ y];\ + const int C=coeffs[ x]*coeffs[3-y];\ + const int D=coeffs[3-x]*coeffs[3-y];\ + uint8_t *cm = ff_cropTbl + MAX_NEG_CROP;\ + int i;\ +\ + for(i=0; i<h; i++)\ + {\ + OP(dst[0], (A*src[0] + B*src[1] + C*src[stride+0] + D*src[stride+1]));\ + OP(dst[1], (A*src[1] + B*src[2] + C*src[stride+1] + D*src[stride+2]));\ + OP(dst[2], (A*src[2] + B*src[3] + C*src[stride+2] + D*src[stride+3]));\ + OP(dst[3], (A*src[3] + B*src[4] + C*src[stride+3] + D*src[stride+4]));\ + dst+= stride;\ + src+= stride;\ + }\ +}\
Why the ff_cropTbl? Bilinear interpolation has no overshoot. --Loren Merritt
participants (3)
-
kostya.shishkov＠gmail.com -
lorenm＠u.washington.edu -
michaelni＠gmx.at