Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sample decimation bug on audiodelays.Echo when freq_shift=True #10017

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

relic-se
Copy link

@relic-se relic-se commented Jan 30, 2025

I've discovered that the frequency-shifting echo algorithm has a functionality bug in regards to how it fills the echo buffer during playback.

Currently, it pulls a sample from the echo buffer, calculates the decay, mixes in the input sample, and then iterates over the buffer from its current position to its next position (using the delay rate with bit shifting). If delay_ms is shorter than max_delay_ms, this loop may iterate over 2+ elements of the buffer. If there is data already in the echo buffer (when decay>0), it will make each element equivalent and lose the remaining echo data after the first element.

The following example demonstrates this problem and the audible effect on the output: https://gist.github.com/relic-se/f125dc56574d895f30eee52ec611e16f.

A before and after audio demonstration of the above example can be found here: https://drive.google.com/file/d/14L_8cUwpy_833RL01Nu4C3MQVEwN3zMq/view?usp=drive_link.

The fix for this problem requires that the echo decay and mix calculations be performed for each element of the buffer regardless of rate. With this update, the input sample will still be added at a faster rate, but all echo samples will be retained.

The downside of this fix is that if max_delay_ms is a large value and delay_ms is much smaller, the fast rate will require that many elements in the buffer will be iterated over, each requiring calculation. Since this module is limited to the boards featuring the RP2350, I don't think the performance impact will be too great.

@relic-se relic-se marked this pull request as ready for review January 30, 2025 04:10
@dhalbert dhalbert requested a review from jepler January 30, 2025 20:22
@relic-se relic-se marked this pull request as draft January 31, 2025 21:31
@relic-se relic-se marked this pull request as ready for review January 31, 2025 21:46
Copy link
Member

@gamblor21 gamblor21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and looks good to me.

@gamblor21 gamblor21 merged commit 0a53eb8 into adafruit:main Feb 1, 2025
44 checks passed
@relic-se relic-se deleted the audiodelays_buffer_fix branch February 3, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants