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

APU Note Mode (alternative version of #334) #706

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

JerwuQu
Copy link
Contributor

@JerwuQu JerwuQu commented Apr 17, 2024

This is another go at #333, but with a slightly different approach from #334.

Big thanks to @marler8997 for the detailed proposal and the other PR!

Has been tested working on both web and native.

Reasoning

Since frequencies sent to tone are integers, we lose quite a bit of precision when it comes to playing lower notes. This PR implements a variant of #333 to make tone capable of playing MIDI notes. We have 16 bits per frequency, which is split up into 8 bits for the note, and 8 bits for optional pitch bend, as to not be limited to pure notes.

It turns out the implementation for this is very simple (JS implementation required less than 15 lines!), and since the flag is new and optional it doesn't affect any prior carts. In my opinion this feature is overdue.

Usage

Instead of calling tone with frequencies, you supply it with notes. The list of MIDI notes is easily googlable and easier to use than frequencies: https://inspiredacoustics.com/en/MIDI_note_numbers_and_center_frequencies

// Play A4 (440Hz)
tone(69, 0, 0, TONE_NOTE_MODE)
// Play A4 bent halfway up to the next note (A#4)
tone(69 | 128 << 8, 0, 0, TONE_NOTE_MODE)

One could compare the pitch bends to cents between semitones in music, where 128 bend would be the equivalent to 50 cents.

Compared to #334 in short:

  • TONE_NOTE_MODE flag for tone rather than a system flag.
  • Puts the main note in the lower bits rather than the upper bits (i.e. tone(note | bend << 8, ...) rather than tone(bend | note << 8, ...).)

@marler8997
Copy link

marler8997 commented Apr 17, 2024

Modifications to my original proposal seem fine to me, Id be happy with this or the original.

Copy link
Owner

@aduros aduros left a comment

Choose a reason for hiding this comment

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

This feels really clean, I like the small adjustments over the other PR 🙂

Could you also add a short tip somewhere to the doc page here? https://wasm4.org/docs/guides/audio

runtimes/native/src/apu.c Outdated Show resolved Hide resolved
runtimes/native/src/apu.c Outdated Show resolved Hide resolved
@JerwuQu JerwuQu force-pushed the jwq_apu-midi-mode branch 3 times, most recently from 3e53ff5 to 132759d Compare April 18, 2024 16:40
@JerwuQu
Copy link
Contributor Author

JerwuQu commented Apr 18, 2024

A broken link and accidentally rebasing onto the wrong branch later and it should be ready for review again :)

@JerwuQu JerwuQu requested a review from aduros April 18, 2024 16:46
@aduros aduros merged commit 6227e90 into aduros:main Apr 18, 2024
5 checks passed
@JerwuQu JerwuQu deleted the jwq_apu-midi-mode branch April 18, 2024 17:07
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.

None yet

3 participants