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
