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

Waveforms can't have 0ms attack/delay/release values #561

Open
ThePythonator opened this issue Jan 20, 2021 · 5 comments
Open

Waveforms can't have 0ms attack/delay/release values #561

ThePythonator opened this issue Jan 20, 2021 · 5 comments
Milestone

Comments

@ThePythonator
Copy link
Contributor

Calling channels[x].trigger_attack() when channels[x].attack_ms = 0 causes a divide by zero error.
This also happens with decay and release.

The code which causes this error is in audio.hpp.

@ThePythonator
Copy link
Contributor Author

A quick fix (as suggested by @Daft-Freak ) would be to not allow adsr_end_frame to be 0:
adsr_end_frame = std::max(uint32_t(1), adsr_end_frame);

@Daft-Freak
Copy link
Collaborator

Daft-Freak commented Jan 20, 2021

Realised I've been working around this for a while: https://github.com/Daft-Freak/32blit-music-player/blob/a4cffc81413696586c7e1eaec0ab2cfc4389118e/mp3-stream.cpp#L237-L239

Clamping adsr_end_frame to a minimum of 1 would be the simplest fix, but does mean you get a 1-sample duration. It at least seems cleaner than

if(![phase]_ms) {
  adsr = [target];
  trigger_[next_phase]();
  return;
}

In all the trigger_[phase] methods...

Edit: Whoops, posted at the same time!

@Gadgetoid
Copy link
Contributor

Looks like we need the ability to disable/bypass the envelope generator altogether?

The >0ms transition times are generally intentional to avoid pops/clicks from harsh audio transitions, but it would seem that's not always the desired outcome for wave samples?

@Daft-Freak
Copy link
Collaborator

I suppose you might want to use it as a fade-in/out for MP3-based music? Otherwise I just want to either mix a bunch of waves (DOOM) or could do without the entire audio system (emulator)... But as always, I'm slightly misusing everything :)

@Gadgetoid
Copy link
Contributor

Your kind of misuse is the best kind of misuse, though, and anything we can fix to support that is probably a net benefit for the future of 32blit. Getting the audio-engine out of the way certainly isn't the crime of the century. 😆

Been a while since I touched the audio code, but an ADSR::Disabled state that skips the envelope generator and doesn't advance states might work.

@Gadgetoid Gadgetoid added this to the v0.4.0 milestone Feb 22, 2023
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

No branches or pull requests

3 participants