Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Cc: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Subject: [FFmpeg-devel] [PATCH] swresample/resample: Properly empty MMX state
Date: Sun, 12 Jun 2022 00:25:02 +0200
Message-ID: <DB6PR0101MB2214AB1CB783BCA7DED878B88FA99@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com> (raw)

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".

             reply	other threads:[~2022-06-11 22:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-11 22:25 Andreas Rheinhardt [this message]
2022-06-12  0:29 ` [FFmpeg-devel] [PATCH 2/5] swresample/x86/audio_convert: Remove obsolete MMX functions Andreas Rheinhardt
2022-06-12  0:29 ` [FFmpeg-devel] [PATCH 3/5] swresample/x86/rematrix: " Andreas Rheinhardt
2022-06-12  0:29 ` [FFmpeg-devel] [PATCH 4/5] swresample/x86/resample: Remove obsolete MMXEXT functions Andreas Rheinhardt
2022-06-12  0:29 ` [FFmpeg-devel] [PATCH 5/5] swresample/resample: Remove unnecessary emms_c Andreas Rheinhardt
2022-06-12 20:02 ` [FFmpeg-devel] [PATCH] swresample/resample: Properly empty MMX state Andreas Rheinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DB6PR0101MB2214AB1CB783BCA7DED878B88FA99@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com \
    --to=andreas.rheinhardt@outlook.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git