[pulseaudio-discuss] On replacing ffmpeg resampler with lavr

[Alexander E. Patrakov]

Hello.

Yesterday I discussed the lavr resampler with Peter Meerwald. I am still 
unconvinced that it is worth including. Here are facts and opinions for 
your consideration.

Quality-wise, when implemented properly, this would be identical to the 
current in-tree ffmpeg resampler. I.e., worse than speex-float-1 above 
10 kHz, but better in the low-frequency-range, and better on typical 
music files (but, let me remind, speex-float-1 is already good enough, 
so nobody will notice the difference on music). I have not tested 
performance.

One of the motivations for the addition of the lavr resampler is to 
remove the in-tree copy of the ffmpeg resampler. The supposed benefits 
are that the copy in libavresample is better maintained and has SIMD 
optimizations. What I disagree with is those supposed benefits.

First, let me debunk the "external copy has SIMD optimizations" myth. 
There are indeed architecture-specific directories in ffmpeg source:

https://github.com/FFmpeg/FFmpeg/tree/master/libavresample/aarch64
https://github.com/FFmpeg/FFmpeg/tree/master/libavresample/arm
https://github.com/FFmpeg/FFmpeg/tree/master/libavresample/x86

(and the same for libav). However, the files contained there are not for 
resampling. They are for sample format conversion (e.g. s16 <-> float) 
and channel remixing, which is what we already have SIMD-optimized code 
for (thanks to Peter).

So, we are talking about replacing C code with C code. Let's compare the 
C code for the main resampling function:

https://github.com/FFmpeg/FFmpeg/blob/master/libavresample/resample.c#L316
http://cgit.freedesktop.org/pulseaudio/pulseaudio/tree/src/pulsecore/ffmpeg/resample2.c#n211

FFMpeg code indeed looks cleaner and supports more sample formats 
instead of insisting on s16. However, if one inlines the resample_one() 
function from libavresample/resample_template.c into the if 
(!nearest_neighbour) branch of resample() from libavresample/resample.c, 
he will get almost exactly what we already have in PulseAudio's embedded 
copy of the ffmpeg resampler. So, all the speed benefits (if any) come 
from changing the working sample format and from ffmpeg build system 
that applies optimizations via CFLAGS that are sensible for this kind of 
code.

Now let's talk about the drawbacks of replacing the ffmpeg resampler 
with lavr.

First, this introduces a dependency on a huge body of source code just 
for the tiny (as demonstrated by the existing embedded copy) task of 
resampling. We already have a setup that allows us to copy almost 
verbatim and use thus this tiny portion of the source.

Second, ffmpeg/libav source code (as a whole) is banned in Fedora for 
patent reasons (Peter already told me that he doesn't care) - so they 
will have to disable this resampler because it would rely on a missing 
dependency.

Third, the patch that adds lavr resampler to PulseAudio has to go 
through hoops in order to avoid the channel remixer that is also present 
in libavresample. This is just ugly, and this was never an issue in our 
embedded copy. I admit that ffmpeg's channel remixer may be better than 
ours.

So, based on the above, I'd prefer to keep the status quo. If the 
embedded ffmpeg resampler bothers you, maybe it is better to just remove 
it in favor of speex (or maybe later migrate to opus?) instead of 
migrating to external libavresample?

-- 
Alexander E. Patrakov