| Message ID | 20211016173552.49482-1-jdek@itanimul.li |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] lavc/arm: dont assign hevc_qpel non-multiple of 8 width stubs | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Sat, 16 Oct 2021, J. Dekker wrote: > Signed-off-by: J. Dekker <jdek@itanimul.li> > --- > libavcodec/arm/hevcdsp_init_neon.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/arm/hevcdsp_init_neon.c b/libavcodec/arm/hevcdsp_init_neon.c > index 201a088dac..112edb5edd 100644 > --- a/libavcodec/arm/hevcdsp_init_neon.c > +++ b/libavcodec/arm/hevcdsp_init_neon.c > @@ -270,7 +270,8 @@ av_cold void ff_hevc_dsp_init_neon(HEVCDSPContext *c, const int bit_depth) > put_hevc_qpel_uw_neon[3][1] = ff_hevc_put_qpel_uw_h1v3_neon_8; > put_hevc_qpel_uw_neon[3][2] = ff_hevc_put_qpel_uw_h2v3_neon_8; > put_hevc_qpel_uw_neon[3][3] = ff_hevc_put_qpel_uw_h3v3_neon_8; > - for (x = 0; x < 10; x++) { > + for (x = 3; x < 10; x++) { > + if (x == 4) continue; The mapping between indices and sizes is quite non-obvious, would it be good to have a comment here explaining why? E.g. "The NEON functions assume that the width is a multiple of 8", and e.g. "x==4 -> width = 12" as comment for the continue statement? This still doesn't explain why this hasn't been an issue in practice before the checkasm test. Do none of the fate samples ever trigger calling these functions with x<3 or x==4? Is that a limitation in our samples, or is it at all possible to make the decoder call those cases? I.e. should we try to find a sample to add that triggers it? The commit message is a bit hard to read too. What about this? ---8<--- lavc/arm: dont assign hevc_qpel functions for non-multiple of 8 widths The assembly is written assuming that the width is a multiple of 8. ---8<--- // Martin
diff --git a/libavcodec/arm/hevcdsp_init_neon.c b/libavcodec/arm/hevcdsp_init_neon.c index 201a088dac..112edb5edd 100644 --- a/libavcodec/arm/hevcdsp_init_neon.c +++ b/libavcodec/arm/hevcdsp_init_neon.c @@ -270,7 +270,8 @@ av_cold void ff_hevc_dsp_init_neon(HEVCDSPContext *c, const int bit_depth) put_hevc_qpel_uw_neon[3][1] = ff_hevc_put_qpel_uw_h1v3_neon_8; put_hevc_qpel_uw_neon[3][2] = ff_hevc_put_qpel_uw_h2v3_neon_8; put_hevc_qpel_uw_neon[3][3] = ff_hevc_put_qpel_uw_h3v3_neon_8; - for (x = 0; x < 10; x++) { + for (x = 3; x < 10; x++) { + if (x == 4) continue; c->put_hevc_qpel[x][1][0] = ff_hevc_put_qpel_neon_wrapper; c->put_hevc_qpel[x][0][1] = ff_hevc_put_qpel_neon_wrapper; c->put_hevc_qpel[x][1][1] = ff_hevc_put_qpel_neon_wrapper;
Signed-off-by: J. Dekker <jdek@itanimul.li> --- libavcodec/arm/hevcdsp_init_neon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)