[FFmpeg-devel] [PATCH] swresample/resample: Properly empty MMX state

[FFmpeg-devel] [PATCH] swresample/resample: Properly empty MMX state

[Andreas Rheinhardt]

There is a x86-32 MMXEXT implementation for resampling
planar 16bit data. multiple_resample() therefore calls
emms_c() if it thinks that this needed. And this is bad:

1. It is a maintenance nightmare because changes to the
x86 resample DSP code would necessitate changes to the check
whether to call emms_c().
2. The return value of av_get_cpu_flags() does not tell
whether the MMX DSP functions are in use, as they could
have been overridden by av_force_cpu_flags().
3. The MMX DSP functions will never be overridden in case of
an x86-32 build with --disable-sse2. In this scenario lots of
resampling tests (like swr-resample_exact_lin_async-s16p-8000-48000)
fail because the cpuflags indicate that SSE2 is available
(presuming that the test is run on a CPU with SSE2).
4. The check includes a call to av_get_cpu_flags(). This is not
optimized away for arches other than x86-32.
5. The check takes about as much time as emms_c() itself,
making it pointless.

This commit therefore removes the check and calls emms_c()
unconditionally (it is a no-op for non-x86).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
The reason I don't add an ARCH_X86_32 check is that I intend
to remove this emms_c() again shortly. I have just updated
my branch [1] that removes obsolete MMX(EXT) by a commit that
removes the MMXEXT resampling functions that are the cause
of this issue. A follow-up commit then removes the emms_c()
completely.

[1]: https://github.com/mkver/FFmpeg/commits/mmx2

 libswresample/resample.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index f1ec77f54b..9c5b7fee72 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -452,9 +452,6 @@ static int set_compensation(ResampleContext *c, int sample_delta, int compensati
 
 static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, AudioData *src, int src_size, int *consumed){
     int i;
-    int av_unused mm_flags = av_get_cpu_flags();
-    int need_emms = c->format == AV_SAMPLE_FMT_S16P && ARCH_X86_32 &&
-                    (mm_flags & (AV_CPU_FLAG_MMX2 | AV_CPU_FLAG_SSE2)) == AV_CPU_FLAG_MMX2;
     int64_t max_src_size = (INT64_MAX/2 / c->phase_count) / c->src_incr;
 
     if (c->compensation_distance)
@@ -500,8 +497,7 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A
         }
     }
 
-    if(need_emms)
-        emms_c();
+    emms_c();
 
     if (c->compensation_distance) {
         c->compensation_distance -= dst_size;
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/5] swresample/x86/audio_convert: Remove obsolete MMX functions

[Andreas Rheinhardt]

x64 always has MMX, MMXEXT, SSE and SSE2 and this means
that some functions for MMX, MMXEXT and 3dnow are always
overridden by other functions (unless one e.g. explicitly
disables SSE2) for x64. So given that the only systems that
benefit from these functions are truely ancient 32bit x86s
they are removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libswresample/x86/audio_convert.asm    | 9 ---------
 libswresample/x86/audio_convert_init.c | 9 +--------
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/libswresample/x86/audio_convert.asm b/libswresample/x86/audio_convert.asm
index d441636d3c..d6d6a81495 100644
--- a/libswresample/x86/audio_convert.asm
+++ b/libswresample/x86/audio_convert.asm
@@ -608,15 +608,6 @@ pack_8ch_%2_to_%1_u_int %+ SUFFIX:
 %macro NOP_N 0-6
 %endmacro
 
-INIT_MMX mmx
-CONV int32, int16, u, 2, 1, INT16_TO_INT32_N, NOP_N
-CONV int32, int16, a, 2, 1, INT16_TO_INT32_N, NOP_N
-CONV int16, int32, u, 1, 2, INT32_TO_INT16_N, NOP_N
-CONV int16, int32, a, 1, 2, INT32_TO_INT16_N, NOP_N
-
-PACK_6CH float, float, u, 2, 2, 0, NOP_N, NOP_N
-PACK_6CH float, float, a, 2, 2, 0, NOP_N, NOP_N
-
 INIT_XMM sse
 PACK_6CH float, float, u, 2, 2, 7, NOP_N, NOP_N
 PACK_6CH float, float, a, 2, 2, 7, NOP_N, NOP_N
diff --git a/libswresample/x86/audio_convert_init.c b/libswresample/x86/audio_convert_init.c
index a7d5ab89f8..f6d36f9ca6 100644
--- a/libswresample/x86/audio_convert_init.c
+++ b/libswresample/x86/audio_convert_init.c
@@ -26,7 +26,7 @@
 #define PROTO(pre, in, out, cap) void ff ## pre ## in## _to_ ##out## _a_ ##cap(uint8_t **dst, const uint8_t **src, int len);
 #define PROTO2(pre, out, cap) PROTO(pre, int16, out, cap) PROTO(pre, int32, out, cap) PROTO(pre, float, out, cap)
 #define PROTO3(pre, cap) PROTO2(pre, int16, cap) PROTO2(pre, int32, cap) PROTO2(pre, float, cap)
-#define PROTO4(pre) PROTO3(pre, mmx) PROTO3(pre, sse) PROTO3(pre, sse2) PROTO3(pre, ssse3) PROTO3(pre, sse4) PROTO3(pre, avx) PROTO3(pre, avx2)
+#define PROTO4(pre) PROTO3(pre, sse) PROTO3(pre, sse2) PROTO3(pre, ssse3) PROTO3(pre, sse4) PROTO3(pre, avx) PROTO3(pre, avx2)
 PROTO4(_)
 PROTO4(_pack_2ch_)
 PROTO4(_pack_6ch_)
@@ -52,15 +52,8 @@ av_cold void swri_audio_convert_init_x86(struct AudioConvert *ac,
             ac->simd_f =  ff_int32_to_int16_a_ ## cap;\
     }
 
-MULTI_CAPS_FUNC(MMX, mmx)
 MULTI_CAPS_FUNC(SSE2, sse2)
 
-    if(EXTERNAL_MMX(mm_flags)) {
-        if(channels == 6) {
-            if(   out_fmt == AV_SAMPLE_FMT_FLT  && in_fmt == AV_SAMPLE_FMT_FLTP || out_fmt == AV_SAMPLE_FMT_S32 && in_fmt == AV_SAMPLE_FMT_S32P)
-                ac->simd_f =  ff_pack_6ch_float_to_float_a_mmx;
-        }
-    }
     if(EXTERNAL_SSE(mm_flags)) {
         if(channels == 6) {
             if(   out_fmt == AV_SAMPLE_FMT_FLT  && in_fmt == AV_SAMPLE_FMT_FLTP || out_fmt == AV_SAMPLE_FMT_S32 && in_fmt == AV_SAMPLE_FMT_S32P)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/5] swresample/x86/rematrix: Remove obsolete MMX functions

[Andreas Rheinhardt]

x64 always has MMX, MMXEXT, SSE and SSE2 and this means
that some functions for MMX, MMXEXT and 3dnow are always
overridden by other functions (unless one e.g. explicitly
disables SSE2) for x64. So given that the only systems that
benefit from these functions are truely ancient 32bit x86s
they are removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libswresample/x86/rematrix.asm    | 6 ------
 libswresample/x86/rematrix_init.c | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/libswresample/x86/rematrix.asm b/libswresample/x86/rematrix.asm
index 7984b9a729..968010701e 100644
--- a/libswresample/x86/rematrix.asm
+++ b/libswresample/x86/rematrix.asm
@@ -223,12 +223,6 @@ mix_2_1_int16_u_int %+ SUFFIX:
 %endmacro
 
 
-INIT_MMX mmx
-MIX1_INT16 u
-MIX1_INT16 a
-MIX2_INT16 u
-MIX2_INT16 a
-
 INIT_XMM sse
 MIX2_FLT u
 MIX2_FLT a
diff --git a/libswresample/x86/rematrix_init.c b/libswresample/x86/rematrix_init.c
index 0608c74e7f..b6ed38bf67 100644
--- a/libswresample/x86/rematrix_init.c
+++ b/libswresample/x86/rematrix_init.c
@@ -28,7 +28,6 @@ mix_2_1_func_type ff_mix_2_1_a_## type ## _ ## simd;
 
 D(float, sse)
 D(float, avx)
-D(int16, mmx)
 D(int16, sse2)
 
 av_cold int swri_rematrix_init_x86(struct SwrContext *s){
@@ -43,10 +42,6 @@ av_cold int swri_rematrix_init_x86(struct SwrContext *s){
     s->mix_2_1_simd = NULL;
 
     if (s->midbuf.fmt == AV_SAMPLE_FMT_S16P){
-        if(EXTERNAL_MMX(mm_flags)) {
-            s->mix_1_1_simd = ff_mix_1_1_a_int16_mmx;
-            s->mix_2_1_simd = ff_mix_2_1_a_int16_mmx;
-        }
         if(EXTERNAL_SSE2(mm_flags)) {
             s->mix_1_1_simd = ff_mix_1_1_a_int16_sse2;
             s->mix_2_1_simd = ff_mix_2_1_a_int16_sse2;
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/5] swresample/x86/resample: Remove obsolete MMXEXT functions

[Andreas Rheinhardt]

x64 always has MMX, MMXEXT, SSE and SSE2 and this means
that some functions for MMX, MMXEXT, SSE and 3dnow are always
overridden by other functions (unless one e.g. explicitly
disables SSE2). So given that the only systems which benefit
from the MMXEXT resamplers (which are overridden by SSE2)
are truely ancient 32bit x86s they are removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libswresample/x86/resample.asm    | 5 -----
 libswresample/x86/resample_init.c | 5 -----
 2 files changed, 10 deletions(-)

diff --git a/libswresample/x86/resample.asm b/libswresample/x86/resample.asm
index 7107cf9d42..6c3dc28703 100644
--- a/libswresample/x86/resample.asm
+++ b/libswresample/x86/resample.asm
@@ -594,11 +594,6 @@ INIT_XMM fma4
 RESAMPLE_FNS float, 4, 2, s, pf_1
 %endif
 
-%if ARCH_X86_32
-INIT_MMX mmxext
-RESAMPLE_FNS int16, 2, 1
-%endif
-
 INIT_XMM sse2
 RESAMPLE_FNS int16, 2, 1
 %if HAVE_XOP_EXTERNAL
diff --git a/libswresample/x86/resample_init.c b/libswresample/x86/resample_init.c
index 32c080ea4c..d13ccd4833 100644
--- a/libswresample/x86/resample_init.c
+++ b/libswresample/x86/resample_init.c
@@ -35,7 +35,6 @@ int ff_resample_common_##type##_##opt(ResampleContext *c, void *dst, \
 int ff_resample_linear_##type##_##opt(ResampleContext *c, void *dst, \
                                       const void *src, int sz, int upd)
 
-RESAMPLE_FUNCS(int16,  mmxext);
 RESAMPLE_FUNCS(int16,  sse2);
 RESAMPLE_FUNCS(int16,  xop);
 RESAMPLE_FUNCS(float,  sse);
@@ -52,10 +51,6 @@ av_cold void swri_resample_dsp_x86_init(ResampleContext *c)
 
     switch(c->format){
     case AV_SAMPLE_FMT_S16P:
-        if (ARCH_X86_32 && EXTERNAL_MMXEXT(mm_flags)) {
-            c->dsp.resample_linear = ff_resample_linear_int16_mmxext;
-            c->dsp.resample_common = ff_resample_common_int16_mmxext;
-        }
         if (EXTERNAL_SSE2(mm_flags)) {
             c->dsp.resample_linear = ff_resample_linear_int16_sse2;
             c->dsp.resample_common = ff_resample_common_int16_sse2;
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/5] swresample/resample: Remove unnecessary emms_c

[Andreas Rheinhardt]

The last MMX code in swresample has just been removed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libswresample/resample.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 9c5b7fee72..8f9efc3f21 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -497,8 +497,6 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A
         }
     }
 
-    emms_c();
-
     if (c->compensation_distance) {
         c->compensation_distance -= dst_size;
         if (!c->compensation_distance) {
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swresample/resample: Properly empty MMX state

[Andreas Rheinhardt]

Andreas Rheinhardt:
> There is a x86-32 MMXEXT implementation for resampling
> planar 16bit data. multiple_resample() therefore calls
> emms_c() if it thinks that this needed. And this is bad:
> 
> 1. It is a maintenance nightmare because changes to the
> x86 resample DSP code would necessitate changes to the check
> whether to call emms_c().
> 2. The return value of av_get_cpu_flags() does not tell
> whether the MMX DSP functions are in use, as they could
> have been overridden by av_force_cpu_flags().
> 3. The MMX DSP functions will never be overridden in case of
> an x86-32 build with --disable-sse2. In this scenario lots of
> resampling tests (like swr-resample_exact_lin_async-s16p-8000-48000)
> fail because the cpuflags indicate that SSE2 is available
> (presuming that the test is run on a CPU with SSE2).
> 4. The check includes a call to av_get_cpu_flags(). This is not
> optimized away for arches other than x86-32.
> 5. The check takes about as much time as emms_c() itself,
> making it pointless.
> 
> This commit therefore removes the check and calls emms_c()
> unconditionally (it is a no-op for non-x86).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> The reason I don't add an ARCH_X86_32 check is that I intend
> to remove this emms_c() again shortly. I have just updated
> my branch [1] that removes obsolete MMX(EXT) by a commit that
> removes the MMXEXT resampling functions that are the cause
> of this issue. A follow-up commit then removes the emms_c()
> completely.
> 
> [1]: https://github.com/mkver/FFmpeg/commits/mmx2
> 
>  libswresample/resample.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/libswresample/resample.c b/libswresample/resample.c
> index f1ec77f54b..9c5b7fee72 100644
> --- a/libswresample/resample.c
> +++ b/libswresample/resample.c
> @@ -452,9 +452,6 @@ static int set_compensation(ResampleContext *c, int sample_delta, int compensati
>  
>  static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, AudioData *src, int src_size, int *consumed){
>      int i;
> -    int av_unused mm_flags = av_get_cpu_flags();
> -    int need_emms = c->format == AV_SAMPLE_FMT_S16P && ARCH_X86_32 &&
> -                    (mm_flags & (AV_CPU_FLAG_MMX2 | AV_CPU_FLAG_SSE2)) == AV_CPU_FLAG_MMX2;
>      int64_t max_src_size = (INT64_MAX/2 / c->phase_count) / c->src_incr;
>  
>      if (c->compensation_distance)
> @@ -500,8 +497,7 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A
>          }
>      }
>  
> -    if(need_emms)
> -        emms_c();
> +    emms_c();
>  
>      if (c->compensation_distance) {
>          c->compensation_distance -= dst_size;

Will apply this patchset tomorrow unless there are objections.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".