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

Pitch shift audio effect #10052

Merged
merged 10 commits into from
Mar 4, 2025
Merged

Conversation

relic-se
Copy link

@relic-se relic-se commented Feb 12, 2025

@jepler @gamblor21 @todbot for interest.

Add a pitch shifting effect to the delay library. It uses a circular buffer "window" with a second, smaller circular buffer "overlap" to achieve the effect, so it fits nicely in the delay category.

This algorithm is relatively simple compared to others which rely on FFT or other computationally intensive processes and has been tested on both the RP2040 and RP2350 without any performance concerns. The only room I see for further optimization is in the recalculate_rate function which uses a single pow call. There's potential to implement something in lines with pitch_bend(...) within shared-module/synthio/Note.c, but recalculate_rate is called at most once out of every 256 samples.

Shifting is currently achieved by manipulating the semitones property (or using a synthio.BlockInput). I settled on using just "semitones" rather than "octaves" or "cents" or a combination of the three, but I am open to suggestion on best practice here.

Single channel output is untested and would likely result in unexpected results because buffer indexes are shared among left and right channels.

I am actually quite impressed with the performance of this audio effect despite its simplicity. It may not be "professional" quality, but I think it's highly effective for most applications.

Fifths melody example:

import time
import board
import audiobusio
import synthio
import audiodelays

audio = audiobusio.I2SOut(
    bit_clock=board.GP0,
    word_select=board.GP1,
    data=board.GP2,
)
synth = synthio.Synthesizer(
    channel_count=2,
    sample_rate=44100,
)
pitch_shift = audiodelays.PitchShift(
    semitones=12,
    channel_count=2,
    sample_rate=44100,
    mix=0.5,
)

pitch_shift.play(synth)
audio.play(pitch_shift)

while True:
    for notenum in (60, 64, 67, 71):
        synth.press(notenum-12)
        time.sleep(0.25)
        synth.release_all()

Vibrato chord example:

import time
import board
import audiobusio
import synthio
import audiodelays

audio = audiobusio.I2SOut(
    bit_clock=board.GP0,
    word_select=board.GP1,
    data=board.GP2,
)
synth = synthio.Synthesizer(
    channel_count=2,
    sample_rate=44100,
)
pitch_shift = audiodelays.PitchShift(
    semitones=synthio.LFO(
        scale=0.5,
        rate=0.5,
    ),
    window=4096, # larger window = more accurancy and more latency
    overlap=512, # overlap reduces popping
    channel_count=2,
    sample_rate=44100,
    mix=1.0, # fully wet
)

pitch_shift.play(synth)
audio.play(pitch_shift)

for notenum in (60, 64, 67, 71):
    synth.press(synthio.Note(
        frequency=synthio.midi_to_hz(notenum),
        amplitude=0.25,
    ))

Audio recording of both examples: https://drive.google.com/file/d/1jfBEAkVzs2b72FccknX_ScdzI9fphOpf/view?usp=sharing

@jepler
Copy link
Member

jepler commented Feb 12, 2025

The documentation says I picked octaves for pitch bend in synthio, though it says the range is +-12 which seems like a semitone-related number. And I can't make much sense of the implementation, it looks too clever to read late at night.

It's a pity if I made a bad choice for synthio, but it's also a pity if it doesn't match between synthio & this new functionality.

@jepler
Copy link
Member

jepler commented Feb 12, 2025

Is this using a published algorithm like PSOLA or a bespoke algorithm?

@relic-se
Copy link
Author

It's a pity if I made a bad choice for synthio, but it's also a pity if it doesn't match between synthio & this new functionality.

I do think it's a smart idea to synchronize their implementation. However, I'm not sure if bend is an appropriate property name in this context. And I don't think you made a bad choice on that. The only difference in implementation is to remove the / 12.0 in recalculate_rate.

@relic-se
Copy link
Author

Is this using a published algorithm like PSOLA or a bespoke algorithm?

It's bespoke but based on well-documented principles. My initial inspiration came from this article, http://www.technoblogy.com/show?1L02, and dissecting the Reaper pitch shifter jsfx plugin.

I can explain it in more detail or make a diagram of the algorithm if needed.

@relic-se relic-se marked this pull request as ready for review February 12, 2025 21:19
@tannewt tannewt requested a review from jepler February 13, 2025 18:42
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I have no idea about the performance of the algorithm, and didn't test it on HW.

I do have some notes about the code structure that I'd like to see addressed.

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.

Code looks good to me, I'll try it on hardware later.

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.

Tried on the Pico2 and worked fine for me. A bit warbly with the default settings in my opinion but you can always up the window/overlap to smooth it out if you want to devote the resources.

@relic-se
Copy link
Author

A bit warbly with the default settings in my opinion but you can always up the window/overlap to smooth it out if you want to devote the resources.

The default settings are definitely conservative. If you have a suggestion for a better combo (though more resource intensive), I'd consider changing out the defaults. Otherwise, it's up to the user.

@gamblor21
Copy link
Member

The default settings are definitely conservative. If you have a suggestion for a better combo (though more resource intensive), I'd consider changing out the defaults. Otherwise, it's up to the user.

Yup more just a comment vs a change required. The documentation says how the user can change it if they want.

@gamblor21 gamblor21 requested a review from jepler March 4, 2025 05:13
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

I don't have the technical expertise to review the new pitch shift code, and I didn't test it either, but I reviewed everything else and it looks good. thank you for taking on that cleanup.

@jepler jepler merged commit 6e7baea into adafruit:main Mar 4, 2025
504 checks passed
@relic-se relic-se deleted the audioeffects_pitch_shift branch March 4, 2025 19:54
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.

3 participants