2013/11/11 Tanu Kaskinen <tanu.kaskinen at linux.intel.com>: > On Sat, 2013-11-09 at 02:34 +0600, Alexander E. Patrakov wrote: >> Peter Meerwald wrote: >> >> > some resampler implementations (e.g. libsamplerate and ffmpeg) do not consume >> > the entire input buffer; the impl_resample() function now has a return value >> > returning the number of frames in the input buffer not processed >> > these frames must be saved in appropriate buffer and presented together with >> > new input data >> >> (yes I know this is too late, as the patch is already pushed, but...) >> >> First, I slightly object to the commit message. It may be >> misinterpreted as "they don't even look at the entire input buffer". >> While sometimes this is true (e.g. because the output buffer size was >> a limiting factor), there is another important reason why some samples >> need to be presented again. Namely, this happens if they affect both >> the old and the new chunk of output data and the resampler >> implementation relies on the user to housekeep the input buffers. >> >> Second, I consider all resamplers that don't do this (i.e. that handle >> the necessary buffering internally in an opaque manner) as >> fundamentally broken if rewinds are allowed, and would argue for their >> refactoring or deletion. And here is why (yes, I am a perfectionist in >> the bad sense of this word). > > Outright deletion is an option only if we think that the deleted > resampler is worthless or nearly worthless to us. Imperfect rewind > handling alone doesn't make a resampler worthless. You are right - by itself, clicky rewind handling does not negate other benefits of a resampler. The ultimate long-term plan should be, however, to either have one perfect resampler instead of the current choice that looks quite similar to the situation that GNOME had with clocks (even if that would mean writing it from scratch - but I doubt that), or to have a document that clearly explains why a tradeoff is necessary without invoking historical and legal reasons. > I don't understand why making e.g. the ffmpeg resampler handle rewinds > correctly would necessitate the deletion of all resamplers that don't > handle rewinds correctly. That oddity notwithstanding, it sounds like > you have a solid plan for fixing the rewinds at least for a subset of > our resamplers, so can we expect patches coming at some point? As far as I understand, ffmpeg resampler is very small (and thus easy to understand), fast, stateless (which means easy support for rewinds), suitably licensed, and already copied into PulseAudio code base (which means that we can improve it further without prior coordination with upstream). When one adds variable sample rate support, it would become _the_ perfect resampler. By itself, this doesn't mean that everything else should be deleted, but the less things to abstract, the better. I don't think you can realistically expect patches, though, in a short timeframe - but eventually I would really like to do that. I still have the IIR-based module-virtual-surround rewrite sitting on my laptop's SSD, waiting for a releasable and well-commented coefficient-generation script :( -- Alexander E. Patrakov