af_aphaser.c: segfault with low values of delay option

[Ganesh Ajjanagadde]

I attempted to apply an -aphaser filter to an aif audio file, with the invocation:
% ffmpeg -report -i sample.aif -af aphaser=delay=0.01 test.aif

ffmpeg crashed with a segfault.
Configuration/info about input file:
ffmpeg version 2.6.2 Copyright (c) 2000-2015 the FFmpeg developers

built with gcc 4.9.2 (GCC) 20150304 (prerelease)
configuration: --prefix=/usr --disable-debug --disable-static --disable-stripping --enable-avisynth --enable-avresample --enable-fontconfig --enable-gnutls --enable-gpl --enable-libass --enable-libbluray --enable-libfreetype --enable-libfribidi --enable-libgsm --enable-libmodplug --enable-libmp3lame --enable-libopencore_amrnb --enable-libopencore_amrwb --enable-libopenjpeg --enable-libopus --enable-libpulse --enable-libschroedinger --enable-libspeex --enable-libssh --enable-libtheora --enable-libv4l2 --enable-libvorbis --enable-libvpx --enable-libx264 --enable-libx265 --enable-libxvid --enable-runtime-cpudetect --enable-shared --enable-swresample --enable-vdpau --enable-version3 --enable-x11grab
libavutil 54. 20.100 / 54. 20.100
libavcodec 56. 26.100 / 56. 26.100
libavformat 56. 25.101 / 56. 25.101
libavdevice 56. 4.100 / 56. 4.100
libavfilter 5. 11.102 / 5. 11.102
libavresample 2. 1. 0 / 2. 1. 0
libswscale 3. 1.101 / 3. 1.101
libswresample 1. 1.100 / 1. 1.100
libpostproc 53. 3.100 / 53. 3.100

Splitting the commandline.
Reading option '-v' ... matched as option 'v' (set logging level) with argument '9'.
Reading option '-loglevel' ... matched as option 'loglevel' (set logging level) with argument '99'.
Reading option '-i' ... matched as input file with argument 'sample.aif'.
Finished splitting the commandline.
Parsing a group of options: global .
Applying option v (set logging level) with argument 9.
Successfully parsed a group of options.
Parsing a group of options: input file sample.aif.
Successfully parsed a group of options.
Opening an input file: sample.aif.
[aiff @ 0x7f70f9ed9860] Format aiff probed with size=2048 and score=100
[aiff @ 0x7f70f9ed9860] Before avformat_find_stream_info() pos: 54 bytes read:32768 seeks:2
[aiff @ 0x7f70f9ed9860] All info found
[aiff @ 0x7f70f9ed9860] After avformat_find_stream_info() pos: 204854 bytes read:262144 seeks:2 frames:50
Guessed Channel Layout for Input Stream #0.0 : stereo
Input #0, aiff, from 'sample.aif':

Duration: 00:01:00.00, start: 0.000000, bitrate: 1411 kb/s

Stream #0:0, 50, 1/44100: Audio: pcm_s16be, 44100 Hz, 2 channels, s16, 1411 kb/s

Successfully opened the file.
At least one output file must be specified
[AVIOContext @ 0x7f70f9ee1f60] Statistics: 262144 bytes read, 2 seeks

I have identified a possible source of the bug:
Lines 208, 209 of source code:

p->delay_buffer_length = p->delay * 0.001 * inlink->sample_rate + 0.5;
p->delay_buffer = av_calloc(p->delay_buffer_length, sizeof(*p->delay_buffer) * inlink->channels);

effectively create a buffer of size zero, with a non-null pointer.
Checks on lines 213, 214:

if (!p->modulation_buffer

!p->delay_buffer)

return AVERROR(ENOMEM);

thus get bypassed, even though the filter should not proceed further due to the zero length buffer.

[Carl Eugen Hoyos]

Please provide a sample that allows to reproduce the crash.

Completely unrelated: --enable-runtime-cpudetect --enable-swresample --enable-vdpau have no effect: Did you compile yourself?

[Ganesh Ajjanagadde]

aphaser segfault

[Ganesh Ajjanagadde]

I did not compile myself, instead used the standard ffmpeg package on Arch Linux.

[Carl Eugen Hoyos]

Do you know who the packager for Arch Linux is? I always wanted to understand why people use --disable-debug --disable-stripping, now at least I know where it comes from...

[Ganesh Ajjanagadde]

A quick check at: ​https://www.archlinux.org/packages/extra/x86_64/ffmpeg/

lists ​https://www.archlinux.org/people/trusted-users/#bpiotrowski

as the last packager for ffmpeg.

[Carl Eugen Hoyos]

$ valgrind ffmpeg_g -i sample.aif -af aphaser=delay=0.01 -f null -
==4308== Memcheck, a memory error detector
==4308== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==4308== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==4308== Command: ffmpeg_g -i sample.aif -af aphaser=delay=0.01 -f null -
==4308==
ffmpeg version N-71997-g5c8809b Copyright (c) 2000-2015 the FFmpeg developers
  built with gcc 4.7 (SUSE Linux)
  configuration: --enable-gpl --enable-gray
  libavutil      54. 23.101 / 54. 23.101
  libavcodec     56. 38.101 / 56. 38.101
  libavformat    56. 32.100 / 56. 32.100
  libavdevice    56.  4.100 / 56.  4.100
  libavfilter     5. 16.101 /  5. 16.101
  libswscale      3.  1.101 /  3.  1.101
  libswresample   1.  1.100 /  1.  1.100
  libpostproc    53.  3.100 / 53.  3.100
Guessed Channel Layout for  Input Stream #0.0 : stereo
Input #0, aiff, from 'sample.aif':
  Duration: 00:00:05.00, start: 0.000000, bitrate: 1411 kb/s
    Stream #0:0: Audio: pcm_s16be, 44100 Hz, 2 channels, s16, 1411 kb/s
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf56.32.100
    Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s
    Metadata:
      encoder         : Lavc56.38.101 pcm_s16le
Stream mapping:
  Stream #0:0 -> #0:0 (pcm_s16be (native) -> pcm_s16le (native))
Press [q] to stop, [?] for help
==4308== Invalid read of size 8
==4308==    at 0x5348A5: phaser_s16 (af_aphaser.c:200)
==4308==    by 0x534FF4: filter_frame (af_aphaser.c:252)
==4308==    by 0x4A124D: ff_filter_frame_framed (avfilter.c:1091)
==4308==    by 0x4A22D8: ff_filter_frame (avfilter.c:1172)
==4308==    by 0x4A6641: request_frame (buffersrc.c:500)
==4308==    by 0x4A68DA: av_buffersrc_add_frame_internal (buffersrc.c:181)
==4308==    by 0x4A6C6C: av_buffersrc_add_frame_flags (buffersrc.c:106)
==4308==    by 0x48A129: decode_audio (ffmpeg.c:1951)
==4308==    by 0x48F8DA: transcode (ffmpeg.c:2225)
==4308==    by 0x471C4A: main (ffmpeg.c:4068)
==4308==  Address 0xb79e960 is 0 bytes inside a block of size 1 alloc'd
==4308==    at 0x4C290FE: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4308==    by 0x4C291A7: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4308==    by 0xF24409: av_malloc (mem.c:95)
==4308==    by 0xF243EB: av_malloc (mem.c:134)
==4308==    by 0xF24742: av_calloc (mem.c:252)
==4308==    by 0x53512B: config_output (af_aphaser.c:209)
==4308==    by 0x4A04B0: avfilter_config_links (avfilter.c:262)
==4308==    by 0x4A0493: avfilter_config_links (avfilter.c:251)
==4308==    by 0x4A483E: avfilter_graph_config (avfiltergraph.c:275)
==4308==    by 0x4826A9: configure_filtergraph (ffmpeg_filter.c:973)
==4308==    by 0x48CB81: transcode_init (ffmpeg.c:2845)
==4308==    by 0x48D8E5: transcode (ffmpeg.c:3857)
==4308==
==4308== Invalid write of size 8
==4308==    at 0x5348BB: phaser_s16 (af_aphaser.c:200)
==4308==    by 0x534FF4: filter_frame (af_aphaser.c:252)
==4308==    by 0x4A124D: ff_filter_frame_framed (avfilter.c:1091)
==4308==    by 0x4A22D8: ff_filter_frame (avfilter.c:1172)
==4308==    by 0x4A6641: request_frame (buffersrc.c:500)
==4308==    by 0x4A68DA: av_buffersrc_add_frame_internal (buffersrc.c:181)
==4308==    by 0x4A6C6C: av_buffersrc_add_frame_flags (buffersrc.c:106)
==4308==    by 0x48A129: decode_audio (ffmpeg.c:1951)
==4308==    by 0x48F8DA: transcode (ffmpeg.c:2225)
==4308==    by 0x471C4A: main (ffmpeg.c:4068)
==4308==  Address 0xb79e970 is 15 bytes after a block of size 1 alloc'd
==4308==    at 0x4C290FE: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4308==    by 0x4C291A7: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==4308==    by 0xF24409: av_malloc (mem.c:95)
==4308==    by 0xF243EB: av_malloc (mem.c:134)
==4308==    by 0xF24742: av_calloc (mem.c:252)
==4308==    by 0x53512B: config_output (af_aphaser.c:209)
==4308==    by 0x4A04B0: avfilter_config_links (avfilter.c:262)
==4308==    by 0x4A0493: avfilter_config_links (avfilter.c:251)
==4308==    by 0x4A483E: avfilter_graph_config (avfiltergraph.c:275)
==4308==    by 0x4826A9: configure_filtergraph (ffmpeg_filter.c:973)
==4308==    by 0x48CB81: transcode_init (ffmpeg.c:2845)
==4308==    by 0x48D8E5: transcode (ffmpeg.c:3857)

[Ganesh Ajjanagadde]

Looking at the source and documentation for aphaser, I see some other things that could be improved. I list them below; please let me know if you want a separate ticket for them.

1. Documentation is somewhat incomplete - it lists the default values, but not the max or the min values.

2. The code has a bunch of "magic" constants representing the max and the min. From my understanding of the source code, many of them are not really fundamental and arbitrarily constrain the user, e.g modulation speed must be between 0.1 and 2. I think some rationale should be offered, e.g as a comment in the code, or if they are not necessary, the constraints should be made more appropriate.

[Ganesh Ajjanagadde]

It appears that the original reported issue (namely the segfault) has been fixed in commit 94e293a83cd7e6b7fa9f3fcd90e9ddae4c274c73

As such, this ticket can be closed, and I will file a new one for my secondary comments.

[Carl Eugen Hoyos]

It looks as if Paul found the issue independently, thank you for testing again.