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

Improved AudioEffectDelayExternal #433

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

h4yn0nnym0u5e
Copy link
Contributor

@h4yn0nnym0u5e h4yn0nnym0u5e commented Apr 1, 2022

  • implements a few more options for memory type
  • breaks out the memory management into AudioExtMem base class so (future) audio objects can share resources
  • documentation added

Subsequent improvements

  • resilience against SPI initialisation issues
  • speed improvement for SPI-based memory on Teensy 4.x
  • update() code simplified by improvements to AudioExtMem class

@PaulStoffregen
Copy link
Owner

oh no, looks like a conflict with another recent commit

… feature/AudioEffectDelayExternal

# Resolved Conflicts:
#	gui/index.html
@h4yn0nnym0u5e
Copy link
Contributor Author

Think I fixed it - there was a previous PR which corrected a copy-pasta error to do with freeing up delay memory, which hadn't propagated into this one. Or something...

Only briefly tested with a partially populated 64MB memory board (see https://forum.pjrc.com/threads/29276-Limits-of-delay-effect-in-audio-library?p=320254&viewfull=1#post320254)

There was an issue with the undefined initialization order of the delay object(s) compared to the supporting SPI object. This wasn't apparent during previous testing, and may be a result of updates to the C++ compiler. We now "pre-initialise" the object, and only finish configuring SPI memory when the first call to delay() is made. Search for "C++ static initialization order fiasco" for more detail than you want.

Note that for huge amounts of SPI RAM the initialisation time can be quite significant, as the whole block is zeroed out at the start to ensure silence is output, before the first real samples are input to the memory.
@h4yn0nnym0u5e
Copy link
Contributor Author

Note that 0544053 fixes a pretty serious bug that rendered this object almost completely unusable, at least on my Windows 10 / Arduino 1.8.19 system.

@h4yn0nnym0u5e
Copy link
Contributor Author

8ed2b49 reverts by default to the "old scheme" of initialising SPI memory at object construction time, since a fix has been done to the SPI library. Deferred initialisation is still available by setting a third constructor parameter to false (if not supplied it defaults to true): this may be of use to users who have only partially updated their libraries.

See https://forum.pjrc.com/threads/73154-Teensy-4-1-does-not-boot-when-invoking-SPI-from-external-class and PaulStoffregen/SPI@1904fb5

Uses 32-bit accesses for audio transfers where possible, giving about 10% speed boost.
Also implement readWrap() and writeWrap() to simplify the update() code.
Gets rid of compiler warning
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.

None yet

2 participants