Skip to content

Fix/pitch shift - #166

Merged
stephenhensley merged 4 commits into
masterfrom
fix/pitch_shift
Sep 28, 2026
Merged

stephenhensley merged 4 commits into
masterfrom
fix/pitch_shift

Conversation

@stephenhensley

Copy link
Copy Markdown
Collaborator

This should end up fixing #162

The negative transposition was almost 2x the ratio that it was supposed to. So that none of the semitones were accurate when set below 0. This has been fixed.

Making the phasor accept negative frequency let's us remove any of the weird "is it going up or down" logic, and just have the phasor flip when the ratio is derived from a negative transposition.

This does remove the pre-generated lookup for the semitones. We can replace it with a more robust lut at some point to get this to be a bit more efficient.

Also moved the one global define into the class as a member constexpr

And added a secondary argument (defaulting to false) to lock transpositions to semitones.

@stephenhensley
stephenhensley marked this pull request as draft October 1, 2021 23:13
@stephenhensley

stephenhensley commented Oct 1, 2021 •

Copy link
Copy Markdown
Collaborator Author

This isn't quite ready. Checked the "fun" parameter, and it is quite fun when tranpsosing up, but when transposing down it just adds a bunch of noise. So we probably need to flip the direction the modulation happens in when transposing below 0

It also does nothing when set to original pitch,

…added some auto-recalc for transposition at/near zero.
@xconverge

Copy link
Copy Markdown
Contributor

This would also fix #201

@xconverge

Copy link
Copy Markdown
Contributor

@stephenhensley is there more work to be done for this? I wanted to open a PR to templatize or parameterize the buffer size but want it based off of this PR. I have run this PR a decent amount and it seems pretty solid

@stephenhensley

Copy link
Copy Markdown
Collaborator Author

I don't think there's anything immediate left to do here.
At the time, I think there was still some potential clean up, or testing I wanted to do.
But I have used this in a few projects without noticing any problems, and it does resolve #201

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