Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
CharlieELweb wants to merge1 commit intoprocessing:main
base:main
Choose a base branch
Loading
fromCharlieELweb:patch-3

Conversation

CharlieELweb
Copy link

@CharlieELwebCharlieELweb commentedAug 21, 2025
edited
Loading

Changes:

  • Fixed thewholeNotes 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 theA andB values of thewholeNotes object, the bug should be fixed.

ksen0 reacted with heart emoji
Whole note A and B should belongs to the next octave
@welcomeWelcome
Copy link

welcomebot commentedAug 21, 2025

🎉 Thanks for opening this pull request! For guidance on contributing, check out ourcontributor guidelines and otherresources 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 toadd yourself to thelist of contributors on the readme page !

Thank You!

@davepagurek
Copy link
Contributor

Hi! A few things to note here:

  • p5.sound.js, in this repo, is pulled in fromhttps://github.com/processing/p5.js-sound as part of the release process, so changes should go in that repo to not be overwritten.
  • The legacy p5.sound.js is also no longer maintained; further updates will go in the new p5.sound library,https://github.com/processing/p5.sound.js. I'm not 100% sure what the status of bug fixes into the legacy p5.sound is,@ksen0 are we still making bug fixes into it or just leaving it as is?

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:

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

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
Copy link
Author

CharlieELweb commentedAug 21, 2025
edited
Loading

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.

ksen0 reacted with heart emoji

@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 🙂

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@CharlieELweb@davepagurek

[8]ページ先頭

©2009-2025 Movatter.jp