| Message ID | 20231122194913.9856-3-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] x86/ac3dsp: reduce instruction count inside the float_to_fixed24 loop | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Le keskiviikkona 22. marraskuuta 2023, 21.49.13 EET James Almer a écrit : > Should simplify asm implementations, and prevent UB on at least win64. > > Signed-off-by: James Almer <jamrial@gmail.com> This one looks good to me, but I am utterly incompetent for the previous two.
On Wed, Nov 22, 2023 at 04:49:13PM -0300, James Almer wrote: > Should simplify asm implementations, and prevent UB on at least win64. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/ac3dsp.c | 2 +- > libavcodec/ac3dsp.h | 2 +- > libavcodec/arm/ac3dsp_init_arm.c | 2 +- > libavcodec/mips/ac3dsp_mips.c | 2 +- > libavcodec/x86/ac3dsp_init.c | 4 ++-- > 5 files changed, 6 insertions(+), 6 deletions(-) In file included from src/libavcodec/mips/ac3dsp_mips.c:57:0: src/libavcodec/ac3dsp.h:56:62: error: unknown type name ‘size_t’; did you mean ‘__ssize_t’? void (*float_to_fixed24)(int32_t *dst, const float *src, size_t len); ^~~~~~ __ssize_t src/ffbuild/common.mak:81: recipe for target 'libavcodec/mips/ac3dsp_mips.o' failed make: *** [libavcodec/mips/ac3dsp_mips.o] Error 1 make: *** Waiting for unfinished jobs.... mips FAIL [...]
On 11/24/2023 6:01 PM, Michael Niedermayer wrote: > On Wed, Nov 22, 2023 at 04:49:13PM -0300, James Almer wrote: >> Should simplify asm implementations, and prevent UB on at least win64. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/ac3dsp.c | 2 +- >> libavcodec/ac3dsp.h | 2 +- >> libavcodec/arm/ac3dsp_init_arm.c | 2 +- >> libavcodec/mips/ac3dsp_mips.c | 2 +- >> libavcodec/x86/ac3dsp_init.c | 4 ++-- >> 5 files changed, 6 insertions(+), 6 deletions(-) > > In file included from src/libavcodec/mips/ac3dsp_mips.c:57:0: > src/libavcodec/ac3dsp.h:56:62: error: unknown type name ‘size_t’; did you mean ‘__ssize_t’? > void (*float_to_fixed24)(int32_t *dst, const float *src, size_t len); > ^~~~~~ > __ssize_t > src/ffbuild/common.mak:81: recipe for target 'libavcodec/mips/ac3dsp_mips.o' failed > make: *** [libavcodec/mips/ac3dsp_mips.o] Error 1 > make: *** Waiting for unfinished jobs.... > mips FAIL Do you know where is size_t defined for this target? stddef.h?
On Fri, Nov 24, 2023 at 06:03:29PM -0300, James Almer wrote: > On 11/24/2023 6:01 PM, Michael Niedermayer wrote: > > On Wed, Nov 22, 2023 at 04:49:13PM -0300, James Almer wrote: > > > Should simplify asm implementations, and prevent UB on at least win64. > > > > > > Signed-off-by: James Almer <jamrial@gmail.com> > > > --- > > > libavcodec/ac3dsp.c | 2 +- > > > libavcodec/ac3dsp.h | 2 +- > > > libavcodec/arm/ac3dsp_init_arm.c | 2 +- > > > libavcodec/mips/ac3dsp_mips.c | 2 +- > > > libavcodec/x86/ac3dsp_init.c | 4 ++-- > > > 5 files changed, 6 insertions(+), 6 deletions(-) > > > > In file included from src/libavcodec/mips/ac3dsp_mips.c:57:0: > > src/libavcodec/ac3dsp.h:56:62: error: unknown type name ‘size_t’; did you mean ‘__ssize_t’? > > void (*float_to_fixed24)(int32_t *dst, const float *src, size_t len); > > ^~~~~~ > > __ssize_t > > src/ffbuild/common.mak:81: recipe for target 'libavcodec/mips/ac3dsp_mips.o' failed > > make: *** [libavcodec/mips/ac3dsp_mips.o] Error 1 > > make: *** Waiting for unfinished jobs.... > > mips FAIL > > Do you know where is size_t defined for this target? stddef.h? stddef.h fixes it, yes! thx [...]
Quoting James Almer (2023-11-24 22:03:29) > On 11/24/2023 6:01 PM, Michael Niedermayer wrote: > > On Wed, Nov 22, 2023 at 04:49:13PM -0300, James Almer wrote: > >> Should simplify asm implementations, and prevent UB on at least win64. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavcodec/ac3dsp.c | 2 +- > >> libavcodec/ac3dsp.h | 2 +- > >> libavcodec/arm/ac3dsp_init_arm.c | 2 +- > >> libavcodec/mips/ac3dsp_mips.c | 2 +- > >> libavcodec/x86/ac3dsp_init.c | 4 ++-- > >> 5 files changed, 6 insertions(+), 6 deletions(-) > > > > In file included from src/libavcodec/mips/ac3dsp_mips.c:57:0: > > src/libavcodec/ac3dsp.h:56:62: error: unknown type name ‘size_t’; did you mean ‘__ssize_t’? > > void (*float_to_fixed24)(int32_t *dst, const float *src, size_t len); > > ^~~~~~ > > __ssize_t > > src/ffbuild/common.mak:81: recipe for target 'libavcodec/mips/ac3dsp_mips.o' failed > > make: *** [libavcodec/mips/ac3dsp_mips.o] Error 1 > > make: *** Waiting for unfinished jobs.... > > mips FAIL > > Do you know where is size_t defined for this target? stddef.h? size_t should _always_ be defined in stddef.h, as per C99 7.17.2
diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c index 302b786b15..8397e03d32 100644 --- a/libavcodec/ac3dsp.c +++ b/libavcodec/ac3dsp.c @@ -54,7 +54,7 @@ static void ac3_exponent_min_c(uint8_t *exp, int num_reuse_blocks, int nb_coefs) } } -static void float_to_fixed24_c(int32_t *dst, const float *src, unsigned int len) +static void float_to_fixed24_c(int32_t *dst, const float *src, size_t len) { const float scale = 1 << 24; do { diff --git a/libavcodec/ac3dsp.h b/libavcodec/ac3dsp.h index 25341f3396..ec2f598451 100644 --- a/libavcodec/ac3dsp.h +++ b/libavcodec/ac3dsp.h @@ -53,7 +53,7 @@ typedef struct AC3DSPContext { * @param len number of elements to convert. * constraints: multiple of 32 greater than zero */ - void (*float_to_fixed24)(int32_t *dst, const float *src, unsigned int len); + void (*float_to_fixed24)(int32_t *dst, const float *src, size_t len); /** * Calculate bit allocation pointers. diff --git a/libavcodec/arm/ac3dsp_init_arm.c b/libavcodec/arm/ac3dsp_init_arm.c index a64aa6ae82..ae989069c9 100644 --- a/libavcodec/arm/ac3dsp_init_arm.c +++ b/libavcodec/arm/ac3dsp_init_arm.c @@ -26,7 +26,7 @@ #include "config.h" void ff_ac3_exponent_min_neon(uint8_t *exp, int num_reuse_blocks, int nb_coefs); -void ff_float_to_fixed24_neon(int32_t *dst, const float *src, unsigned int len); +void ff_float_to_fixed24_neon(int32_t *dst, const float *src, size_t len); void ff_ac3_extract_exponents_neon(uint8_t *exp, int32_t *coef, int nb_coefs); void ff_ac3_sum_square_butterfly_int32_neon(int64_t sum[4], const int32_t *coef0, diff --git a/libavcodec/mips/ac3dsp_mips.c b/libavcodec/mips/ac3dsp_mips.c index a5eaaf8eb2..3ea3acc185 100644 --- a/libavcodec/mips/ac3dsp_mips.c +++ b/libavcodec/mips/ac3dsp_mips.c @@ -203,7 +203,7 @@ static void ac3_update_bap_counts_mips(uint16_t mant_cnt[16], uint8_t *bap, #if HAVE_MIPSFPU #if !HAVE_MIPS32R6 && !HAVE_MIPS64R6 -static void float_to_fixed24_mips(int32_t *dst, const float *src, unsigned int len) +static void float_to_fixed24_mips(int32_t *dst, const float *src, size_t len) { const float scale = 1 << 24; float src0, src1, src2, src3, src4, src5, src6, src7; diff --git a/libavcodec/x86/ac3dsp_init.c b/libavcodec/x86/ac3dsp_init.c index 106121b5b9..baa2bea3a4 100644 --- a/libavcodec/x86/ac3dsp_init.c +++ b/libavcodec/x86/ac3dsp_init.c @@ -26,8 +26,8 @@ void ff_ac3_exponent_min_sse2 (uint8_t *exp, int num_reuse_blocks, int nb_coefs); -void ff_float_to_fixed24_sse2 (int32_t *dst, const float *src, unsigned int len); -void ff_float_to_fixed24_avx2 (int32_t *dst, const float *src, unsigned int len); +void ff_float_to_fixed24_sse2 (int32_t *dst, const float *src, size_t len); +void ff_float_to_fixed24_avx2 (int32_t *dst, const float *src, size_t len); int ff_ac3_compute_mantissa_size_sse2(uint16_t mant_cnt[6][16]);
Should simplify asm implementations, and prevent UB on at least win64. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/ac3dsp.c | 2 +- libavcodec/ac3dsp.h | 2 +- libavcodec/arm/ac3dsp_init_arm.c | 2 +- libavcodec/mips/ac3dsp_mips.c | 2 +- libavcodec/x86/ac3dsp_init.c | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-)