| Message ID | 1b353c0b4af069072a12974e1d273171138590f3.1547408762.git.pross@xvid.org |
|---|---|
| State | New |
| Headers | show |
2019-01-13 21:02 GMT+01:00, Peter Ross <pross@xvid.org>: > --- > libavcodec/vp56.c | 10 ++++++++++ > libavcodec/vp56.h | 1 + > libavcodec/vp56dsp.c | 19 ------------------- > 3 files changed, 11 insertions(+), 19 deletions(-) > > diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c > index 27b4b8b944..c5c5a9fb65 100644 > --- a/libavcodec/vp56.c > +++ b/libavcodec/vp56.c > @@ -33,6 +33,8 @@ > > void ff_vp56_init_dequant(VP56Context *s, int quantizer) > { > + if (s->quantizer != quantizer) > + ff_vp3dsp_set_bounding_values(s->bounding_values_array, > ff_vp56_filter_threshold[quantizer]); > s->quantizer = quantizer; > s->dequant_dc = ff_vp56_dc_dequant[quantizer] << 2; > s->dequant_ac = ff_vp56_ac_dequant[quantizer] << 2; > @@ -320,9 +322,17 @@ static void vp56_add_predictors_dc(VP56Context *s, > VP56Frame ref_frame) > static void vp56_deblock_filter(VP56Context *s, uint8_t *yuv, > ptrdiff_t stride, int dx, int dy) > { > + if (s->avctx->codec->id == AV_CODEC_ID_VP5) { > int t = ff_vp56_filter_threshold[s->quantizer]; > if (dx) s->vp56dsp.edge_filter_hor(yuv + 10-dx , stride, t); > if (dy) s->vp56dsp.edge_filter_ver(yuv + stride*(10-dy), stride, t); > + } else { > + int * bounding_values = s->bounding_values_array + 127; > + if (dx) > + ff_vp4_h_loop_filter_12_c(yuv + 10-dx, stride, > bounding_values); > + if (dy) > + ff_vp4_v_loop_filter_12_c(yuv + stride*(10-dy), stride, > bounding_values); > + } > } This is your code now but I wonder why it helps not to merge this with 1/6. > static void vp56_mc(VP56Context *s, int b, int plane, uint8_t *src, > diff --git a/libavcodec/vp56.h b/libavcodec/vp56.h > index 70e1d38a83..9b3036895c 100644 > --- a/libavcodec/vp56.h > +++ b/libavcodec/vp56.h > @@ -170,6 +170,7 @@ struct vp56_context { > int filter_mode; > int max_vector_length; > int sample_variance_threshold; > + DECLARE_ALIGNED(8, int, bounding_values_array)[256]; > > uint8_t coeff_ctx[4][64]; /* used in vp5 only */ > uint8_t coeff_ctx_last[4]; /* used in vp5 only */ > diff --git a/libavcodec/vp56dsp.c b/libavcodec/vp56dsp.c > index 9f299dc60f..e8d93d6680 100644 > --- a/libavcodec/vp56dsp.c > +++ b/libavcodec/vp56dsp.c > @@ -72,27 +72,8 @@ av_cold void ff_vp5dsp_init(VP56DSPContext *s) > #endif /* CONFIG_VP5_DECODER */ > > #if CONFIG_VP6_DECODER > -static int vp6_adjust(int v, int t) > -{ > - int V = v, s = v >> 31; > - V ^= s; > - V -= s; > - if (V-t-1 >= (unsigned)(t-1)) > - return v; > - V = 2*t - V; > - V += s; > - V ^= s; > - return V; > -} > - > -VP56_EDGE_FILTER(vp6, hor, 1, stride) > -VP56_EDGE_FILTER(vp6, ver, stride, 1) > - > av_cold void ff_vp6dsp_init(VP56DSPContext *s) > { > - s->edge_filter_hor = vp6_edge_filter_hor; > - s->edge_filter_ver = vp6_edge_filter_ver; And why are the new functions called "_c" but are not part of the context? (Or do I misread the code?) Carl Eugen
On Mon, Jan 14, 2019 at 07:02:47AM +1100, Peter Ross wrote: > --- > libavcodec/vp56.c | 10 ++++++++++ > libavcodec/vp56.h | 1 + > libavcodec/vp56dsp.c | 19 ------------------- > 3 files changed, 11 insertions(+), 19 deletions(-) > > diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c > index 27b4b8b944..c5c5a9fb65 100644 > --- a/libavcodec/vp56.c > +++ b/libavcodec/vp56.c > @@ -33,6 +33,8 @@ > > void ff_vp56_init_dequant(VP56Context *s, int quantizer) > { > + if (s->quantizer != quantizer) > + ff_vp3dsp_set_bounding_values(s->bounding_values_array, ff_vp56_filter_threshold[quantizer]); maybe ive missed something but this fails to build libavcodec/vp56.c: In function ‘ff_vp56_init_dequant’: libavcodec/vp56.c:37:9: error: implicit declaration of function ‘ff_vp3dsp_set_bounding_values’ [-Werror=implicit-function-declaration] ff_vp3dsp_set_bounding_values(s->bounding_values_array, ff_vp56_filter_threshold[quantizer]); ^ cc1: some warnings being treated as errors make: *** [libavcodec/vp56.o] Error 1 make: *** Waiting for unfinished jobs.... [...]
On Sun, Jan 13, 2019 at 09:39:57PM +0100, Carl Eugen Hoyos wrote: > 2019-01-13 21:02 GMT+01:00, Peter Ross <pross@xvid.org>: > > --- > > libavcodec/vp56.c | 10 ++++++++++ > > libavcodec/vp56.h | 1 + > > libavcodec/vp56dsp.c | 19 ------------------- > > 3 files changed, 11 insertions(+), 19 deletions(-) > > + if (dx) > > + ff_vp4_h_loop_filter_12_c(yuv + 10-dx, stride, > > bounding_values); > > + if (dy) > > + ff_vp4_v_loop_filter_12_c(yuv + stride*(10-dy), stride, > > bounding_values); > > + } > > } > > This is your code now but I wonder why it helps not to merge > this with 1/6. fair enough. > > - s->edge_filter_hor = vp6_edge_filter_hor; > > - s->edge_filter_ver = vp6_edge_filter_ver; > > And why are the new functions called "_c" but > are not part of the context? > (Or do I misread the code?) i will drop the _c. and for consisistency they probably should be ff_vp3dsp_xxx. they are not part of the context, because they there are are no asm alternative functions (yet). it seems like a waste creating context function pointers that only every point to the one function. -- Peter (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
2019-01-14 9:11 GMT+01:00, Peter Ross <pross@xvid.org>: > On Sun, Jan 13, 2019 at 09:39:57PM +0100, Carl Eugen Hoyos wrote: >> 2019-01-13 21:02 GMT+01:00, Peter Ross <pross@xvid.org>: >> > --- >> > libavcodec/vp56.c | 10 ++++++++++ >> > libavcodec/vp56.h | 1 + >> > libavcodec/vp56dsp.c | 19 ------------------- >> > 3 files changed, 11 insertions(+), 19 deletions(-) > >> > + if (dx) >> > + ff_vp4_h_loop_filter_12_c(yuv + 10-dx, stride, >> > bounding_values); >> > + if (dy) >> > + ff_vp4_v_loop_filter_12_c(yuv + stride*(10-dy), stride, >> > bounding_values); >> > + } >> > } >> >> This is your code now but I wonder why it helps not to merge >> this with 1/6. > > fair enough. > >> > - s->edge_filter_hor = vp6_edge_filter_hor; >> > - s->edge_filter_ver = vp6_edge_filter_ver; >> >> And why are the new functions called "_c" but >> are not part of the context? >> (Or do I misread the code?) > > i will drop the _c. > > and for consisistency they probably should be ff_vp3dsp_xxx. > > they are not part of the context, because they there are are no > asm alternative functions (yet). it seems like a waste creating > context function pointers that only every point to the one function. If you plan to add asm functions (or if you believe they should be added) you could leave the function pointers as they are. But that's all your decision, Carl Eugen
diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c index 27b4b8b944..c5c5a9fb65 100644 --- a/libavcodec/vp56.c +++ b/libavcodec/vp56.c @@ -33,6 +33,8 @@ void ff_vp56_init_dequant(VP56Context *s, int quantizer) { + if (s->quantizer != quantizer) + ff_vp3dsp_set_bounding_values(s->bounding_values_array, ff_vp56_filter_threshold[quantizer]); s->quantizer = quantizer; s->dequant_dc = ff_vp56_dc_dequant[quantizer] << 2; s->dequant_ac = ff_vp56_ac_dequant[quantizer] << 2; @@ -320,9 +322,17 @@ static void vp56_add_predictors_dc(VP56Context *s, VP56Frame ref_frame) static void vp56_deblock_filter(VP56Context *s, uint8_t *yuv, ptrdiff_t stride, int dx, int dy) { + if (s->avctx->codec->id == AV_CODEC_ID_VP5) { int t = ff_vp56_filter_threshold[s->quantizer]; if (dx) s->vp56dsp.edge_filter_hor(yuv + 10-dx , stride, t); if (dy) s->vp56dsp.edge_filter_ver(yuv + stride*(10-dy), stride, t); + } else { + int * bounding_values = s->bounding_values_array + 127; + if (dx) + ff_vp4_h_loop_filter_12_c(yuv + 10-dx, stride, bounding_values); + if (dy) + ff_vp4_v_loop_filter_12_c(yuv + stride*(10-dy), stride, bounding_values); + } } static void vp56_mc(VP56Context *s, int b, int plane, uint8_t *src, diff --git a/libavcodec/vp56.h b/libavcodec/vp56.h index 70e1d38a83..9b3036895c 100644 --- a/libavcodec/vp56.h +++ b/libavcodec/vp56.h @@ -170,6 +170,7 @@ struct vp56_context { int filter_mode; int max_vector_length; int sample_variance_threshold; + DECLARE_ALIGNED(8, int, bounding_values_array)[256]; uint8_t coeff_ctx[4][64]; /* used in vp5 only */ uint8_t coeff_ctx_last[4]; /* used in vp5 only */ diff --git a/libavcodec/vp56dsp.c b/libavcodec/vp56dsp.c index 9f299dc60f..e8d93d6680 100644 --- a/libavcodec/vp56dsp.c +++ b/libavcodec/vp56dsp.c @@ -72,27 +72,8 @@ av_cold void ff_vp5dsp_init(VP56DSPContext *s) #endif /* CONFIG_VP5_DECODER */ #if CONFIG_VP6_DECODER -static int vp6_adjust(int v, int t) -{ - int V = v, s = v >> 31; - V ^= s; - V -= s; - if (V-t-1 >= (unsigned)(t-1)) - return v; - V = 2*t - V; - V += s; - V ^= s; - return V; -} - -VP56_EDGE_FILTER(vp6, hor, 1, stride) -VP56_EDGE_FILTER(vp6, ver, stride, 1) - av_cold void ff_vp6dsp_init(VP56DSPContext *s) { - s->edge_filter_hor = vp6_edge_filter_hor; - s->edge_filter_ver = vp6_edge_filter_ver; - s->vp6_filter_diag4 = ff_vp6_filter_diag4_c; if (ARCH_ARM)