On Thu, 2013-07-11 at 17:42 +0600, Alexander E. Patrakov wrote: > 2013/7/11 Tanu Kaskinen <tanu.kaskinen at linux.intel.com>: > > The first is that I believe simple reset results in some inaccuracy at > > least for the next sample that the resampler produces, because a reset > > means that the resampler pretends that the previous audio was all > > silence, which is not true. That said, we have been doing this forever, > > and nobody has complained, and I would guess that the error is rarely > > audible. > > Please count this mail as a complaint, then. The error is audible (as > a click), I guess nobody reported this because in the most common > rewind case (on stream volume adjustment) one simply takes clicks as > something expected. Complaint noted. > > The second issue is that if the resampler does any buffering (like the > > wrapper that you perhaps plan to do, or swr_convert()), and the buffered > > data is dropped on rewind, the dropped data should be accounted for in > > the next component in the rewind chain - the dropped data should be sort > > of pushed back to where it originally came from, otherwise that data is > > completely lost, so the audio skips (very little, but perhaps audibly). > > I don't quite understand this part. I think that, if a resampler > "eats" the beginning of the incoming audio data, it would have done so > during the previous piece of sound (that existed before the rewind), > too. So the end result would be not that some data is dropped due to a > rewind, but that the boundary between the old and amended data would > be off by some samples. You don't understand me, and I don't understand you... What is the result of the boundary being off? Does audio get skipped, duplicated, or is there no error at all? I'll try to clarify this (also to myself) with an example. Let's have a sink, and a stream with a resampler in between. For simplicity, let's assume that the resampler doesn't actually do any resampling, so when the sink asks for 10 samples, the resampler reads from the stream 10 samples. Let's say that the write index of the sink is N, and the resampler has one sample buffered. Due to the buffered sample, the read index of the stream is N+1. Now the sink is rewound by 10 samples. This means that the sink will want the next written sample to be from index N-10. The resampler drops the buffered sample, and the read index of the stream moves back by 10 samples to N-9. The sample at N-10 got lost, the user hears audio skipping by one sample. The amount of dropped audio in the resampler buffer should have been added to the amount by which the stream read index was moved back. > Your third comment about the latency is spot > on. If the resampler allows one to set the initial phase (and that > conflicts with any form of the reset() api) and the amended sound is > the same as the original one, there will be no error. "The initial phase" means the last output sample relative to the new position, right? > > The third issue is that if the resampler does any buffering, that > > affects the latency, and that is not currently taken into account in the > > latency calculations. The added latency is perhaps small enough not to > > matter at all. > > It is typically less than 1 ms. > > OTOH, the above discussion makes me think that the API for resamplers, > as defined in src/pulsecore/resampler.h, is unsuitable if rewinds are > allowed and have to be processed absolutely correctly. Also I think > that the correct API (that allows pulseaudio to tell resamplers > explicitly about any rewinds, so that they adjust the phase > accordingly and access old samples directly as needed) is not > implementable on top of any stock resampler if it is treated as a > black box. It might be feasible to add the required functionality to the stock resamplers. When we discussed the filter rewinding, you mentioned the idea of maintaining a history of filter state snapshots. Taking a snapshot only requires a function for copying the filter state, and I would guess that adding such function to the stock resamplers could very well be done. -- Tanu
