audiofilters: Add Distortion effect and implement LFO ticking

[relic-se]

New audio effects class, audiofilters.Distortion, to distort audio samples using one of the available modes available in the audiofilters.DistortionMode enum.

Todo:

• Reduce floating point computation overhead for DistortionMode.CLIP, DistortionMode.OVERDRIVE and DistortionMode.WAVESHAPE algorithms.
• Test with unsigned and 8 bit sources.

Comments:

• Should tone-shaping of some form be included within this class or require the use of an audiofilters.Filter effect? (ie: filter.play(distortion.play(sample)))
• Any addition mode suggestions would be appreciated. The size overhead of including additional algorithms is very minimal. I'd only want to avoid redundant or unnecessary modes which may cause confusion (hence why I didn't include the ATAN mode within the Godot Engine).
• The pre_gain and post_gain properties are currently measured in decibels which requires the addition of a db_to_linear function to make those values linear. Would it be more ideal to forego decibels altogether and make those properties linear?

Parameters, documentation copy and algorithms are mostly credited to Godot Engine under the MIT License.

• Documentation: https://docs.godotengine.org/en/stable/classes/class_audioeffectdistortion.html.
• Code: https://github.com/godotengine/godot/blob/master/servers/audio/effects/audio_effect_distortion.cpp

Example Code:

import board
import audiobusio
import audiofilters
import audiocore
import digitalio
import adafruit_debouncer

audio = audiobusio.I2SOut(bit_clock=board.GP0, word_select=board.GP1, data=board.GP2)

wave_file = open("StreetChicken.wav", "rb")
wave = audiocore.WaveFile(wave_file)

effect = audiofilters.Distortion(
    buffer_size=1024,
    channel_count=wave.channel_count,
    sample_rate=wave.sample_rate,
    mix=0.0,
    pre_gain=0.0,
    post_gain=-10.0,
    drive=0.5,
)
effect.play(wave, loop=True)
audio.play(effect)

button_mix_pin = digitalio.DigitalInOut(board.GP3)
button_mix_pin.direction = digitalio.Direction.INPUT
button_mix = adafruit_debouncer.Debouncer(button_mix_pin)

button_mode_pin = digitalio.DigitalInOut(board.GP4)
button_mode_pin.direction = digitalio.Direction.INPUT
button_mode = adafruit_debouncer.Debouncer(button_mode_pin)

modes = [
#    (Mode, pre_gain, drive, post_gain),
    (audiofilters.DistortionMode.CLIP, 0.0, 0.5, -10.0),
    (audiofilters.DistortionMode.LOFI, 0.0, 0.9, -2.0),
    (audiofilters.DistortionMode.OVERDRIVE, 20.0, 0.5, -15.0),
    (audiofilters.DistortionMode.WAVESHAPE, 10.0, 0.75, -20.0),
]
mode_index = 0

# Initial state
print("off")
print(effect.mode)

while True:
    button_mix.update()
    if button_mix.rose:
        effect.mix = not effect.mix
        print("on" if effect.mix else "off")
    
    button_mode.update()
    if button_mode.rose:
        mode_index = (mode_index + 1) % len(modes)
        effect.mode, effect.pre_gain, effect.drive, effect.post_gain = modes[mode_index]
        print(effect.mode)

Closes #9872.

[RAWJUNGLE]

I think we need to add more fluff for the full effects package as well.

• Maybe a bitcrasher?

[relic-se]

I think we need to add more fluff for the full effects package as well.

* Maybe a bitcrasher?

I don't know exactly what you mean by "fluff", but the audiofilters.DistortionMode.LOFI is a bitcrusher. In fact, it may need to be renamed to better represent that.

[jepler]

Additionally, the added file(s) in this module need to be separately listed in the UNIX port (ports/unix/variant/coverage/mpconfigvariant.mk). This is the cause of the CI failures that prevent per-board builds from starting.

It would be nice if at least one basic test of the distortion functionality is added to tests/circuitpython so that this functionality is smoke tested during CI.

[relic-se]

Thank you for the review, @jepler ! I'll dig in as soon as I get the chance.

[relic-se]

I've noticed a few areas within audiodelays.Echo and audiofilters.Filter which weren't using the practices you've outlined in your comments, @jepler. I've included those updates in a separate commit within this PR.

[jepler]

[copied from Discord]

I don't know that these other objects need a blocks property. But they need to call shared_bindings_synthio_lfo_tick once per 256 samples(*) to set the global data that causes synthio_block_slot_get to actually advance the LFOs that are used.

blocks is used as a way to tick blocks that aren't actually used by anything. or which aren't necessarily used by anything. So for instance if you wanted to have an LFO that is polled from CircuitPython code in order to set an LED color. Or, if you have an LFO that you want to keep advancing its phase regardless of whether it's associated with a playing note.

For the first case, having some object that is not a synthesizer/audio source at all might be the right solution.

For the second case, I don't know if that is applicable to the e.g., an audio echo effect.

(*) shared_bindings_synthio_lfo_tick could have a "number of samples" parameter added to the C function, so that you could calculate a different number of samples each time. Of course, since the arithmetic is all discrete you could get slightly different results from advancing LFOs by [email protected] units vs advancing them at 256-sample@8kHz or 512-sample@24kHz rates...

[relic-se]

I don't know that these other objects need a blocks property. But they need to call shared_bindings_synthio_lfo_tick once per 256 samples(*) to set the global data that causes synthio_block_slot_get to actually advance the LFOs that are used.

blocks is used as a way to tick blocks that aren't actually used by anything. or which aren't necessarily used by anything. So for instance if you wanted to have an LFO that is polled from CircuitPython code in order to set an LED color. Or, if you have an LFO that you want to keep advancing its phase regardless of whether it's associated with a playing note.

Thank you for the explanation. I don't think I properly understood the purpose of the blocks properly and have misused it in the past. This all makes a lot more sense. I'll update this effect accordingly. If the solution is as simple as it seems, I may also update the other audio effects which use blocks within this PR (same as with the floating point constants).

(*) shared_bindings_synthio_lfo_tick could have a "number of samples" parameter added to the C function, so that you could calculate a different number of samples each time. Of course, since the arithmetic is all discrete you could get slightly different results from advancing LFOs by [email protected] units vs advancing them at 256-sample@8kHz or 512-sample@24kHz rates...

As for your note on the variable number of samples, I think that 256 samples is a generally ideal pace, but if the buffer size is not a factor of 256, it would cause uneven ticking of the blocks. Yet, using the entire buffer size may not be ideal either because the block inputs may not increment at a desirable pace. We may need a separate accumulator within audiofilters_distortion_obj_t to determine when to update the blocks.

[relic-se]

@jepler I've taken some time to look into how the block ticking is actually handled, and I still don't think that the block system is ready to have this update to audioeffects classes.

audiocore.RawSample -> audiodelays.Echo -> audiobusio.I2SOut

In the example layout above, it would make sense to include shared_bindings_synthio_lfo_tick within audiodelays.Echo. Otherwise, synthio.BlockInput values would not be updated.

synthio.Synthesizer -> audiodelays.Echo -> audiobusio.I2SOut

synthio.Synthesizer -> audiomixer.MixerVoice
audiocore.RawSample -> audiodelays.Echo -> audiomixer.MixerVoice
audiomixer.Mixer -> audiobusio.I2SOut

In either of the two examples above, the global block ticking variables (ie: uint8_t synthio_global_tick) would be updated by two separate entities, causing uneven value changes especially when dealing with different buffer sizes. Working with two synthio.Synthesizer objects would likely do the same thing as well.

I think the solution is to store these global variables locally on each object and add them as arguments to synthio_block_slot_get. That should solve this problem. Sharing synthio.BlockInput objects among different audio objects would still be an issue.

That all said, I still don't plan to include this fix here, and it should instead be addressed in a separate PR connected to #9872.

[jepler]

In either of the two examples above, the global block ticking variables (ie: uint8_t synthio_global_tick) would be updated by two separate entities

This is why the rule exists that a block can only be associated with one audio source and associating it with more than one audio source is an undiagnosed error or undefined behavior, however you want to call it.

Here's why (I think that) a single global tick is sufficient, given that rule: synthio_global_tick is only ever compared for equality with last_tick. This means it's not a problem if 2 different audio sources both increment it; both of them will tick their associated blocks once, by one tick, the exact details being determined by the global rate scale (and, for filters, the global W scale).

As a footnote, this also ignores the possibility that synthio_global_tick is ticked exactly 256 times, as it is an 8 bit quantity for no good reason, it uses up 4 bytes of storage anyway:

typedef struct synthio_block_base {
    mp_obj_base_t base;
    uint8_t last_tick;
    mp_float_t value;
} synthio_block_base_t;

[jepler]

so with two synthesizers, a block used in synth A will see synthio_global_tick values like 0, 2, 4, ... and a block used in synth B will see ticks like 1, 3, 5, .... But it doesn't matter, since the tick value is always "different than the last time".

Why does the synthio_global_tick exist at all? So that synthio_block_slot_get knows whether it needs to call a particular block's tick or just return the old value. So for instance if you use the same LFO as the pitch bend of two notes, it still only gets advanced once despite two synthio_block_slot_get calls on it during the same tick. Otherwise, I'd have to explicitly track a lot more information, or not allow two uses of one LFO output.

[relic-se]

@jepler I see what you mean and why the uneven synthio_global_tick values wouldn't make a difference.

I put in some work to separate those three global variables into a struct that is passed through to all necessary block functions: main...relic-se:circuitpython:local_block_ticking. I'm good to scrap this in favor of your approach.

I've also changed my mind on the 256 sample increments, and I think I'll make the audioeffects just tick the block inputs based on the size of the buffer. It'll make for a much simpler update.

[relic-se]

@jepler I've updated shared_bindings_synthio_lfo_tick to support a variable number of samples and all audio effects to implement this call. I see there is a conflict with your PR, #9804 (https://github.com/adafruit/circuitpython/pull/9804/files#diff-8b26aff15abcdd9f17032d54412d3c7eda19e104eb216351be5836a441a97f83R244). Let me know how we'd like to handle that situation (I'm not ticking block biquads within this PR, btw).

I've tested the following script with and without the update and can confirm that it works now:

import audiobusio
import audiocore
import audiodelays
import audiomixer
import board
import synthio

audio = audiobusio.I2SOut(
    bit_clock=board.GP0,
    word_select=board.GP1,
    data=board.GP3,
)

wave_file = open("StreetChicken.wav", "rb")
wave = audiocore.WaveFile(wave_file)

# Use mixer to decrease volume of sample
mixer = audiomixer.Mixer(
    voice_count=1,
    channel_count=wave.channel_count,
    sample_rate=wave.sample_rate,
)
mixer.voice[0].level = 0.5

effect = audiodelays.Echo(
    delay_ms=synthio.LFO(rate=0.2, scale=100, offset=250),
    freq_shift=True,
    channel_count=wave.channel_count,
    sample_rate=wave.sample_rate,
)

audio.play(effect)
effect.play(mixer)
mixer.voice[0].play(wave, loop=True)

Same goes for this test with a synthio.Synthesizer sample. It works with and without the update (since the synth was also handling lfo ticking):

import audiobusio
import audiodelays
import board
import synthio
import time

audio = audiobusio.I2SOut(
    bit_clock=board.GP0,
    word_select=board.GP1,
    data=board.GP3,
)

synth = synthio.Synthesizer(
    channel_count=2,
    sample_rate=22050,
)

effect = audiodelays.Echo(
    delay_ms=synthio.LFO(rate=0.2, scale=100, offset=250),
    freq_shift=True,
    channel_count=2,
    sample_rate=22050,
)

audio.play(effect)
effect.play(synth)

while True:
    synth.press(65)
    time.sleep(0.1)
    synth.release(65)
    time.sleep(0.9)

[relic-se]

If you test the above examples with a large buffer_size on the effect, let's say 16384, you can clearly hear the stepping in the delay time of the output. Though it is very unlikely that a user would use a buffer_size of this amount, it does bring up a valid case for keeping lfo ticking to SYNTHIO_MAX_DUR intervals.

[relic-se]

I've moved the block value calls around in order to force SYNTHIO_MAX_DUR intervals within the audio effects buffer loop. This fixes the large buffer issue. Here's another example to demonstrate this:

import audiobusio
import audiocore
import audiofilters
import board
import synthio

audio = audiobusio.I2SOut(
    bit_clock=board.GP0,
    word_select=board.GP1,
    data=board.GP3,
)

wave_file = open("StreetChicken.wav", "rb")
wave = audiocore.WaveFile(wave_file)

import audiofilters
effect = audiofilters.Distortion(
    mix=synthio.LFO(rate=0.5, offset=0.5, scale=0.5),
    pre_gain=-30,
    drive=0.75,
    channel_count=wave.channel_count,
    sample_rate=wave.sample_rate,
    buffer_size=32768, # large buffer size
)

effect.play(wave, loop=True) # if effect.play is called after audio.play, a delay of over 1s will occur while it fills the first buffer
audio.play(effect)

These changes will like cause merge conflicts with #9876 regarding audiodelays.Echo.

[gamblor21]

Couple small comments on the code. I'll test this on hardware shortly. Sorry for the delay!

[gamblor21]

Could this and line 306 be factored up out of the if/else block? The compiler probably does anyways.

[relic-se]

Good call, and looking deeper at it, this would actually cause an issue with unsigned 16-bit sources due to while (length--) {...} above it.

[relic-se]

On second thought, we rely on the remaining length of the sample to calculate the ticking length (see the n variable below). In this logic branch, the sample is set to NULL and hence we tick the LFO in one fell swoop instead. In order to optimize this, we'd have to calculate n beforehand and add an additional self->sample != NULL check. I don't see this helping too much. If it's still a concern, it's simpler to just remove LFO ticking when there's no sample.

[gamblor21]

We may want to check that delay >= 0. Never did in the original code (my bad) but a LFO could go negative and weird things happen.

[relic-se]

Casting it to an unsigned integer does force it to be >= 0, but a negative result might cause it to wrap around. If it's 0, I believe it will do a minimum of 1 sample of delay. I'll look into it.

[jepler]

maybe it should be synthio_block_slot_get_limited(1, some_calculated_maximum) then?

Technically under the C99 standard, conversion from a negative floating point value to an unsigned integer value is undefined behavior.

When a finite value of real floating type is converted to an integer type other than _Bool,
the fractional part is discarded (i.e., the value is truncated toward zero). If the value of
the integral part cannot be represented by the integer type, the behavior is undefined. (6.3.1.4.1)

[relic-se]

I've taken a different approach here. Storing this value and current_delay_ms as uint32_t was an arbitrary choice, so I've instead reverted both back to floats. I've added in another pre-calculated variable for the length of one audio frame (sample) in milliseconds, and I'm checking against that determine if we need to recalculate delay buffer parameters. The minimum value of the delay is handled within recalculate_delay. Before, delays could only be manipulated by an LFO in millisecond intervals because of this, whereas now they can be controlled with sample-level precision by an LFO. This could be useful for chorus/phaser type effects.

As for some_calculated_maximum, it's difficult to sum that up because that maximum value depends on the freq_shift property. If using the standard delay, it is limited to max_effect_ms, but with freq_shift, I believe it is max_effect_ms*256 because of the 8 sub-bits in the buffer pointer. You lose a lot of audio quality when going that long, though.

I'm tempted to move this logic into recalculate_delay if it needs more refinement.

[jepler]

I didn't try the code and I don't understand the distortion algorithm, but I noted a few things that may benefit from attention.

[jepler]

maybe it should be synthio_block_slot_get_limited(1, some_calculated_maximum) then?

Technically under the C99 standard, conversion from a negative floating point value to an unsigned integer value is undefined behavior.

When a finite value of real floating type is converted to an integer type other than _Bool,
the fractional part is discarded (i.e., the value is truncated toward zero). If the value of
the integral part cannot be represented by the integer type, the behavior is undefined. (6.3.1.4.1)

[jepler]

it's up to your testing that the perf is OK but I remain surprised that it's even feasible to call a transcendental math function for every sample. microcontrollers are a lot faster than they used to be.

[relic-se]

I believe I borrowed the hard/soft clip option from daisyduino. The difference is noticeable depending on the scenario.

Regarding performance, I think it really depends on the extent of the user code. If you are just processing audio through this effect, it seems to run just fine. I haven't tested this effect within a larger application as of yet.

Hard clip is definitely the more performant option. There may need to be some notes on performance within the documentation.

As for the other algorithms, I don't fully understand them either. But I've done my best to refactor them to reduce unnecessary floating point calculations where possible. If anyone can reference less intensive algorithms with similar results, I'd be happy to switch over.

[gamblor21]

I tried it on the RP2350 and seems to work fine for me. No crashes.