[FFmpeg-devel] [PATCH 1/2] lavc/arm: dont assign hevc_qpel non-multiple of 8 width stubs

[Martin Storsjö]

On Sat, 16 Oct 2021, J. Dekker wrote:

> Signed-off-by: J. Dekker <jdek at 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