Skip to content

Conversation

CharlieELweb
Copy link

@CharlieELweb CharlieELweb commented Aug 21, 2025

Changes:

  • Fixed the wholeNotes object's value in p5.sound.js (please help me update p5.sound.min.js as well...)

When building a project using the p5.sound.js library, there's an error in the p5.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 and B values of the wholeNotes object, the bug should be fixed.

Whole note A and B should belongs to the next octave
Copy link

welcome bot commented Aug 21, 2025

🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors!
🤔 Please ensure that your PR links to an issue, which has been approved for work by a maintainer; otherwise, there might already be someone working on it, or still ongoing discussion about implementation. You are welcome to join the discussion in an Issue if you're not sure!
🌸 Once your PR is merged, be sure to add yourself to the list of contributors on the readme page !

Thank You!

@davepagurek
Copy link
Contributor

Hi! A few things to note here:

Regarding the particular issue in question here, it actually looks like this has 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:

/** [p5.sound] Version: 1.0.1 - 2021-05-25 */

A test release 1.0.2 was made https://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
Copy link
Author

CharlieELweb commented Aug 21, 2025

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.

@davepagurek
Copy link
Contributor

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 🙂

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants