Hi Jean-Marc, Thanks for taking a look. I've added an example program in the patches that changes the rate frequently. You can run test-resample2 >test.raw and open in audacity or so to look at the spectrum etc. I've attached a before/after screenshot. In theory, depending on the current phase and the rate changes that are applied, the error can be audible as a pop when changing rates. I've tested this with a sine wave, it might not be as significant with 'normal' audio. The speex resampler is used in pulseaudio to do resampling when matching rates between cards etc. In this case, the rate would change quite frequently. We would want to enable something more accurate but maybe as you say, we should use a resampler more suited for dynamic rate changes... With interpolated sinc table, a more accurate phase calculation should not have any performance impact. For the full sinc table, it might be possible that more phases need to be calculated (more memory + slower startup). I quite like the speex resampler and its optimizations. I have not tested it but it looks like SRC would need some optimizations before it can be used as a replacement, especially compared to the interpolated mode. Wim On 4 February 2016 at 15:38, Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:> Hi Wim, > > I had a quick look at your patch and I wanted to check what issue you > were trying to solve. Do you have an example where changing the rate > really causes bad artefacts in practice? If so, does that happen on a > single change or only on continuous changes? The reason I ask is that > one of the fundamental design assumptions of this resampler is that rate > changes are relatively infrequent. For continuous rate changes, Erik de > Castro Lopo's Secret Rabbit Code (http://www.mega-nerd.com/SRC/) becomes > more computationally efficient anyway. > > I'm not against being more careful with the phase, but that would > somehow have to be carefully evaluated since having to reduce the > quality setting to improve infrequent rate changes may not be optimal. > Alternatively, this could be a run-time option similar to the quality > setting. > > Cheers, > > Jean-Marc > > On 02/04/2016 07:17 AM, Wim Taymans wrote: > > Hi, > > > > These patches improve the resampler set_rate function. > > > > The first 2 patches do some cleanups in the GCD calculation and avoid an > > overflow when calculating the new phase. This patch could probably be > > simplified > > if we allowed 64bits operations in speexdsp. > > > > The 3rd patch avoid rounding errors in the phase calculation. The problem > > is that the new rate is calculated with the reduced rate, which can cause > > large rounding errors and audible artifacts. > > > > Patch 4 demonstrates the effect of patch 3. Compare the resampler output > > before and after patch 3. > > > > Patch 5 attempts to strike a balance between accuracy and memory usage by > > allowing a small rounding error in the phase. > > > > Let me know what you think, > > Wim > > > > > > > > _______________________________________________ > > Speex-dev mailing list > > Speex-dev at xiph.org > > http://lists.xiph.org/mailman/listinfo/speex-dev > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/speex-dev/attachments/20160204/43e3c0ee/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: Screenshot from 2016-02-04 16-06-50.png Type: image/png Size: 535795 bytes Desc: not available URL: <http://lists.xiph.org/pipermail/speex-dev/attachments/20160204/43e3c0ee/attachment-0001.png>
Hi Wim, On 02/04/2016 10:14 AM, Wim Taymans wrote:> I've added an example program in the patches that changes the rate > frequently. You can run test-resample2 >test.raw and open in audacity or so > to look at the spectrum etc. I've attached a before/after screenshot.I'll have a closer look at your test program.> In theory, depending on the current phase and the rate changes that are > applied, the error can be audible as a pop when changing rates. I've > tested this with a sine wave, it might not be as significant with > 'normal' audio.Just curious, have you tried just doing proper rounding (rather than truncation as is the case now) to see if it improves?> The speex resampler is used in pulseaudio to do resampling when matching > rates between cards etc. In this case, the rate would change quite > frequently. We would want to enable something more accurate but maybe as > you say, we should use a resampler more suited for dynamic rate changes...I can see how that would cause multiple rate changes. At the same time, since this would be using the interpolated code (unless you're really lucky with the rate), the cost shouldn't be too high. BTW, do you know how often the rate gets updated?> With interpolated sinc table, a more accurate phase calculation should > not have any performance impact.Agreed.> For the full sinc table, it might be > possible that more phases need to be calculated (more memory + slower > startup).Well, my main concern is that this would make the code switch from the "direct" resampler to the interpolated one, which is about 4x slower.> I quite like the speex resampler and its optimizations. I have not > tested it but it looks like SRC would need some optimizations before it > can be used as a replacement, especially compared to the interpolated mode.Well, the two designs are very different, with different tradeoffs. In terms of algorithm alone (not looking at implementation efficiency), the Speex resampler has less work to do than SRC for each sample it processes. On the other hand, it has a fixed cost for re-computing sinc tables every time the rate changes, whereas (AFAIK) SRC incurs no extra cost if you change rate on every sample. So in terms of frequency of rate changes, there's a threshold at which the two algorithms are about the same, but I've never attempted to measure that threshold (it would depend on optimizations, SIMD, and a bunch of other things anyway). Cheers, Jean-Marc
04.02.2016 22:05, Jean-Marc Valin ?????:> I can see how that would cause multiple rate changes. At the same time, > since this would be using the interpolated code (unless you're really > lucky with the rate), the cost shouldn't be too high. BTW, do you know > how often the rate gets updated?The rate is, by default, updated every 10 seconds. And sometimes (especially with the not-so-recent unreviewed patches) we do get lucky and resample from 44100 to 44100 Hz, and the next 10 seconds from 44100 to 44101 Hz. -- Alexander E. Patrakov