Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.6k
Fix error of noteToFreq() in p5.sound.js#8040
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Whole note A and B should belongs to the next octave
🎉 Thanks for opening this pull request! For guidance on contributing, check out ourcontributor guidelines and otherresources for contributors! Thank You! |
Hi! A few things to note here:
Regarding the particular issue in question here, it actually looks like thishas been fixed in legacy p5.sound already, four years ago:processing/p5.js-sound#605 @ksen0 it looks like the version in p5.js has just never been updated since 1.0.1: Line 1 in0c2fe51
A test release 1.0.2 was madehttps://github.com/processing/p5.js-sound/releases/tag/v1.0.2 but it's not clear how extensively this was tested. I guess it never made its way out.@ksen0 not sure how risky you feel it is to do a legacy p5.sound update in a minor p5 1.11.x release. (As you can probably see@CharlieELweb, the mixed release process of p5 and p5.sound in 1.x is a little messy, and they have separate release schedules in 2.x to resolve some of that messiness.) |
CharlieELweb commentedAug 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks@davepagurek, just found that the official web editor of p5.js default version 1.11.8 has this problem so sent this pull request. |
we appreciate you noticing the bug! the fact that it was fixed upstream but not actually released is still worth looking into, it's just that the fix is likely more to do with the release process than a code change 🙂 |
Uh oh!
There was an error while loading.Please reload this page.
Changes:
wholeNotes
object's value inp5.sound.js
(please help me updatep5.sound.min.js
as well...)When building a project using the p5.sound.js library, there's an error in thep5.MonoSynth.triggerAttack() function when passing a MIDI value in Note/Octave format (
"C4"
,"Eb3"
, etc.) as the parameter. The frequency of notes"A"
and"B"
(and related notes like"A#"
,"Ab"
) should belong to the next octave. (i.e., when passing"B5"
as the parameter, the result is actually the frequency of"B4"
).This sketch showcases the error:https://editor.p5js.org/CharlieEL/sketches/cit_Ecpdm
By adding 12 to the
A
andB
values of thewholeNotes
object, the bug should be fixed.