[FFmpeg-devel] [PATCH v2 1/3] aarch64/vvc: Add w_avg

[Zhao Zhili]

> On Sep 26, 2024, at 20:17, Martin Storsjö <martin at martin.st> wrote:
> 
> On Thu, 26 Sep 2024, Zhao Zhili wrote:
> 
>>      --- a/libavcodec/aarch64/vvc/dsp_init.c
>>      +++ b/libavcodec/aarch64/vvc/dsp_init.c
>>      @@ -52,6 +52,37 @@ void ff_vvc_avg_12_neon(uint8_t *dst,
>>      ptrdiff_t dst_stride,
>>                             const int16_t *src0, const int16_t
>>      *src1, int width,
>>                             int height);
>> 
>>      +void ff_vvc_w_avg_8_neon(uint8_t *_dst, const ptrdiff_t
>>      _dst_stride,
>>      +                         const int16_t *src0, const
>>      int16_t *src1,
>>      +                         const int width, const int
>>      height,
>>      +                         uintptr_t w0_w1, uintptr_t
>>      offset_shift);
>> Including "const" on scalar parameters is entirely redundant, and we
>> don't prescribe use of that elsewhere in ffmpeg, and just makes the
>> whole declaration more noisy.
>> I see these “const” make clang-tidy not happy. They are here to keep
>> consistent with the prototypes
>> in vvc/dsp.h.
> 
> Hmm, I don't quite understand this comment - so you say that clang-tidy, in addition to me, also complain about them? But they are added manually to keep the prototypes exactly in sync? Or does clang-tidy complain about differences here, if we differ on the constness here?

Clang-tidy complains about these “const” be added:

Clang-Tidy: Parameter 'block_w' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions

> 
>> There are three options:
>> 1. Keep “const” as current state
>> 2. Drop “const” only for these new functions
>> 3. Remove “const” from vvc/dsp.h and all implementations
>> I can’t decide which way to go.
> 
> I would go for 3, at least long term.
> 
> If you need to keep the const within the function prototypes here for now to please some tool (I think most compilers wouldn't complain about differences in const on scalar parameters, although I think old MSVC did that), that's ok, but I would remove it from the unnecessary places (the local variables in the function, the parameter/register mappings in assembly).
> 
> Then we can try to do 3 as a later step.
> 
> // Martin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".