Hey there,
I have recently switched to using FFmpeg for conversions of 24bit stereo WAV to
16bit stereo WAV (with dithering).
For some very large files, I occasionally encountered a segmentation fault in
_sum2_float. Unfortunately, I was not able to reproduce the issue in a small
test setting, but only in a quite large environment.
Debugging showed that the issue was caused in function swr_convert_internal()
in swresample.c, specifically in line 681
s->mix_2_1_f(
conv_src->ch[ch] + off, // out array
preout->ch[ch] + off, // in1 array
s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos + off +
len1, // in2 array
s->native_one, // coefficients
0, // coefficient index 1
0, // coefficient index 2
out_count - len1 // length
);
(https://github.com/FFmpeg/FFmpeg/blob/53688b62ca96ad9a3b0e7d201caca61c79a68648/libswresample/swresample.c#L681)
where dithering is applied. Here, s->mix_2_1_f() is the same as sum2_float().
The in2 array pointer is too large.
I was able to log values before one of the crashs and found:
out_count=682
len1=672
(out_count - len1)=10
off=2688
s->dither.noise.bps =4
s->dither.noise_pos =130262
s->dither.noise.count=131072
s->dither.noise.bps * s->dither.noise_pos + off + len1 = 524408 // in2 start
address offset greater than buffer size!!!
The buffer count has a total size of s->dither.noise.count *
s->dither.noise.bps = 524288
Therefore the start address for the in2 array (3rd argument for mix_2_1_f(...))
is already outside of the buffer.
I was not able to find a reason for the “+len1” in “s->dither.noise.bps *
s->dither.noise_pos + off + len1”. It looks out of place to me. Without it the
buffer overrun does not occur.
“make fate” worked like a charm.
-hendrik
tagtraum industries incorporated
724 Nash Drive
Raleigh, NC 27608
USA
+1 (919) 809-7797
http://www.tagtraum.com/
http://www.beatunes.com/
From ef79e9286c4b8b694715e978f48abe1601956c0e Mon Sep 17 00:00:00 2001
From: Hendrik Schreiber <h...@tagtraum.com>
Date: Thu, 5 Apr 2018 13:58:37 +0200
Subject: [PATCH] Fix for seg fault in swr_convert_internal() -> sum2_float
during dithering. Removed +len1 in call to s->mix_2_1_f() as I found no
logical explanation for it. After removal, problem was gone.
Signed-off-by: Hendrik Schreiber <h...@tagtraum.com>
---
libswresample/swresample.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libswresample/swresample.c b/libswresample/swresample.c
index f076b6c8cf..5bd39caac4 100644
--- a/libswresample/swresample.c
+++ b/libswresample/swresample.c
@@ -678,7 +678,7 @@ static int swr_convert_internal(struct SwrContext *s,
AudioData *out, int out_co
s->mix_2_1_simd(conv_src->ch[ch], preout->ch[ch],
s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos,
s->native_simd_one, 0, 0, len1);
if(out_count != len1)
for(ch=0; ch<preout->ch_count; ch++)
- s->mix_2_1_f(conv_src->ch[ch] + off,
preout->ch[ch] + off, s->dither.noise.ch[ch] + s->dither.noise.bps *
s->dither.noise_pos + off + len1, s->native_one, 0, 0, out_count - len1);
+ s->mix_2_1_f(conv_src->ch[ch] + off,
preout->ch[ch] + off, s->dither.noise.ch[ch] + s->dither.noise.bps *
s->dither.noise_pos + off, s->native_one, 0, 0, out_count - len1);
} else {
for(ch=0; ch<preout->ch_count; ch++)
s->mix_2_1_f(conv_src->ch[ch], preout->ch[ch],
s->dither.noise.ch[ch] + s->dither.noise.bps * s->dither.noise_pos,
s->native_one, 0, 0, out_count);
--
2.15.1 (Apple Git-101)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
