| Message ID | 20190119224839.18909-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 483d02918872fb382e7d630b9ef5b2551283c2f4 |
| Headers | show |
On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavcodec/motion_est.c | 4 +-- > libavcodec/motion_est_template.c | 62 +--------------------------------------- > 2 files changed, 3 insertions(+), 63 deletions(-) please check if the compiler optimizes the size constant out after this change thanks [...]
On Sun, 20 Jan 2019, Michael Niedermayer wrote: > On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavcodec/motion_est.c | 4 +-- >> libavcodec/motion_est_template.c | 62 +--------------------------------------- >> 2 files changed, 3 insertions(+), 63 deletions(-) > > please check if the compiler optimizes the size constant out after this > change The compiler inlines the function before and after the change as well. I can't see notable changes in the disassembly of interlaced_search and h263_mv4_search. Is this enough, or something else should be checked? I am not sure how... Thanks, Marton
On Sun, Jan 20, 2019 at 07:35:18PM +0100, Marton Balint wrote: > > > On Sun, 20 Jan 2019, Michael Niedermayer wrote: > > >On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: > >>Signed-off-by: Marton Balint <cus@passwd.hu> > >>--- > >> libavcodec/motion_est.c | 4 +-- > >> libavcodec/motion_est_template.c | 62 +--------------------------------------- > >> 2 files changed, 3 insertions(+), 63 deletions(-) > > > >please check if the compiler optimizes the size constant out after this > >change > > The compiler inlines the function before and after the change as well. I > can't see notable changes in the disassembly of interlaced_search and > h263_mv4_search. Is this enough, or something else should be checked? I am > not sure how... If the inlined code is used with only one size value then its probably ok. you could also put some marker with asm() in the code where size is used and then look at the .s file generated if the size is optimized out or still read as a variable thanks [...]
On Sun, 20 Jan 2019, Michael Niedermayer wrote: > On Sun, Jan 20, 2019 at 07:35:18PM +0100, Marton Balint wrote: >> >> >> On Sun, 20 Jan 2019, Michael Niedermayer wrote: >> >>> On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: >>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>> --- >>>> libavcodec/motion_est.c | 4 +-- >>>> libavcodec/motion_est_template.c | 62 +--------------------------------------- >>>> 2 files changed, 3 insertions(+), 63 deletions(-) >>> >>> please check if the compiler optimizes the size constant out after this >>> change >> >> The compiler inlines the function before and after the change as well. I >> can't see notable changes in the disassembly of interlaced_search and >> h263_mv4_search. Is this enough, or something else should be checked? I am >> not sure how... > > If the inlined code is used with only one size value then its probably ok. > you could also put some marker with asm() in the code where size is used > and then look at the .s file generated if the size is optimized out or > still read as a variable I checked the .s file and a constant is pushed to the stack as the size parameter when funny_diamond_search is called in h263_mv4_search and interlaced_search. So yes, the size parameter is still optimized as a constant in the touched functions. Regards, Marton
On Thu, Jan 24, 2019 at 10:23:33PM +0100, Marton Balint wrote: > > > On Sun, 20 Jan 2019, Michael Niedermayer wrote: > > >On Sun, Jan 20, 2019 at 07:35:18PM +0100, Marton Balint wrote: > >> > >> > >>On Sun, 20 Jan 2019, Michael Niedermayer wrote: > >> > >>>On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: > >>>>Signed-off-by: Marton Balint <cus@passwd.hu> > >>>>--- > >>>>libavcodec/motion_est.c | 4 +-- > >>>>libavcodec/motion_est_template.c | 62 +--------------------------------------- > >>>>2 files changed, 3 insertions(+), 63 deletions(-) > >>> > >>>please check if the compiler optimizes the size constant out after this > >>>change > >> > >>The compiler inlines the function before and after the change as well. I > >>can't see notable changes in the disassembly of interlaced_search and > >>h263_mv4_search. Is this enough, or something else should be checked? I am > >>not sure how... > > > >If the inlined code is used with only one size value then its probably ok. > >you could also put some marker with asm() in the code where size is used > >and then look at the .s file generated if the size is optimized out or > >still read as a variable > > I checked the .s file and a constant is pushed to the stack as the size > parameter when funny_diamond_search is called in h263_mv4_search and > interlaced_search. > > So yes, the size parameter is still optimized as a constant in the touched > functions. i guess its ok then thanks for checking [...]
On Thu, 24 Jan 2019, Michael Niedermayer wrote: > On Thu, Jan 24, 2019 at 10:23:33PM +0100, Marton Balint wrote: >> >> >> On Sun, 20 Jan 2019, Michael Niedermayer wrote: >> >>> On Sun, Jan 20, 2019 at 07:35:18PM +0100, Marton Balint wrote: >>>> >>>> >>>> On Sun, 20 Jan 2019, Michael Niedermayer wrote: >>>> >>>>> On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: >>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>> --- >>>>>> libavcodec/motion_est.c | 4 +-- >>>>>> libavcodec/motion_est_template.c | 62 +--------------------------------------- >>>>>> 2 files changed, 3 insertions(+), 63 deletions(-) >>>>> >>>>> please check if the compiler optimizes the size constant out after this >>>>> change >>>> >>>> The compiler inlines the function before and after the change as well. I >>>> can't see notable changes in the disassembly of interlaced_search and >>>> h263_mv4_search. Is this enough, or something else should be checked? I am >>>> not sure how... >>> >>> If the inlined code is used with only one size value then its probably ok. >>> you could also put some marker with asm() in the code where size is used >>> and then look at the .s file generated if the size is optimized out or >>> still read as a variable >> >> I checked the .s file and a constant is pushed to the stack as the size >> parameter when funny_diamond_search is called in h263_mv4_search and >> interlaced_search. >> >> So yes, the size parameter is still optimized as a constant in the touched >> functions. > > i guess its ok then Thanks, applied. Regards, Marton
diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c index 8b5ce2117a..759eea479d 100644 --- a/libavcodec/motion_est.c +++ b/libavcodec/motion_est.c @@ -633,7 +633,7 @@ static inline int h263_mv4_search(MpegEncContext *s, int mx, int my, int shift) if(P[i][1] > (c->ymax<<shift)) P[i][1]= (c->ymax<<shift); } - dmin4 = epzs_motion_search4(s, &mx4, &my4, P, block, block, s->p_mv_table, (1<<16)>>shift); + dmin4 = epzs_motion_search2(s, &mx4, &my4, P, block, block, s->p_mv_table, (1<<16)>>shift, 1); dmin4= c->sub_motion_search(s, &mx4, &my4, dmin4, block, block, size, h); @@ -795,7 +795,7 @@ static int interlaced_search(MpegEncContext *s, int ref_index, P_MV1[0]= mx; //FIXME not correct if block != field_select P_MV1[1]= my / 2; - dmin = epzs_motion_search2(s, &mx_i, &my_i, P, block, field_select+ref_index, mv_table, (1<<16)>>1); + dmin = epzs_motion_search2(s, &mx_i, &my_i, P, block, field_select+ref_index, mv_table, (1<<16)>>1, 0); dmin= c->sub_motion_search(s, &mx_i, &my_i, dmin, block, field_select+ref_index, size, h); diff --git a/libavcodec/motion_est_template.c b/libavcodec/motion_est_template.c index 0c21bbfe1a..014038e54f 100644 --- a/libavcodec/motion_est_template.c +++ b/libavcodec/motion_est_template.c @@ -989,76 +989,16 @@ int ff_epzs_motion_search(MpegEncContext *s, int *mx_ptr, int *my_ptr, } } -static int epzs_motion_search4(MpegEncContext * s, - int *mx_ptr, int *my_ptr, int P[10][2], - int src_index, int ref_index, int16_t (*last_mv)[2], - int ref_mv_scale) -{ - MotionEstContext * const c= &s->me; - int best[2]={0, 0}; - int d, dmin; - unsigned map_generation; - const int penalty_factor= c->penalty_factor; - const int size=1; - const int h=8; - const int ref_mv_stride= s->mb_stride; - const int ref_mv_xy= s->mb_x + s->mb_y *ref_mv_stride; - me_cmp_func cmpf, chroma_cmpf; - LOAD_COMMON - int flags= c->flags; - LOAD_COMMON2 - - cmpf = s->mecc.me_cmp[size]; - chroma_cmpf = s->mecc.me_cmp[size + 1]; - - map_generation= update_map_generation(c); - - dmin = 1000000; - - /* first line */ - if (s->first_slice_line) { - CHECK_MV(P_LEFT[0]>>shift, P_LEFT[1]>>shift) - CHECK_CLIPPED_MV((last_mv[ref_mv_xy][0]*ref_mv_scale + (1<<15))>>16, - (last_mv[ref_mv_xy][1]*ref_mv_scale + (1<<15))>>16) - CHECK_MV(P_MV1[0]>>shift, P_MV1[1]>>shift) - }else{ - CHECK_MV(P_MV1[0]>>shift, P_MV1[1]>>shift) - //FIXME try some early stop - CHECK_MV(P_MEDIAN[0]>>shift, P_MEDIAN[1]>>shift) - CHECK_MV(P_LEFT[0]>>shift, P_LEFT[1]>>shift) - CHECK_MV(P_TOP[0]>>shift, P_TOP[1]>>shift) - CHECK_MV(P_TOPRIGHT[0]>>shift, P_TOPRIGHT[1]>>shift) - CHECK_CLIPPED_MV((last_mv[ref_mv_xy][0]*ref_mv_scale + (1<<15))>>16, - (last_mv[ref_mv_xy][1]*ref_mv_scale + (1<<15))>>16) - } - if(dmin>64*4){ - CHECK_CLIPPED_MV((last_mv[ref_mv_xy+1][0]*ref_mv_scale + (1<<15))>>16, - (last_mv[ref_mv_xy+1][1]*ref_mv_scale + (1<<15))>>16) - if(s->mb_y+1<s->end_mb_y) //FIXME replace at least with last_slice_line - CHECK_CLIPPED_MV((last_mv[ref_mv_xy+ref_mv_stride][0]*ref_mv_scale + (1<<15))>>16, - (last_mv[ref_mv_xy+ref_mv_stride][1]*ref_mv_scale + (1<<15))>>16) - } - - dmin= diamond_search(s, best, dmin, src_index, ref_index, penalty_factor, size, h, flags); - - *mx_ptr= best[0]; - *my_ptr= best[1]; - - return dmin; -} - -//try to merge with above FIXME (needs PSNR test) static int epzs_motion_search2(MpegEncContext * s, int *mx_ptr, int *my_ptr, int P[10][2], int src_index, int ref_index, int16_t (*last_mv)[2], - int ref_mv_scale) + int ref_mv_scale, const int size) { MotionEstContext * const c= &s->me; int best[2]={0, 0}; int d, dmin; unsigned map_generation; const int penalty_factor= c->penalty_factor; - const int size=0; //FIXME pass as arg const int h=8; const int ref_mv_stride= s->mb_stride; const int ref_mv_xy= s->mb_x + s->mb_y *ref_mv_stride;
Signed-off-by: Marton Balint <cus@passwd.hu> --- libavcodec/motion_est.c | 4 +-- libavcodec/motion_est_template.c | 62 +--------------------------------------- 2 files changed, 3 insertions(+), 63 deletions(-)