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

Quantizer v2 #26

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Quantizer v2 #26

wants to merge 16 commits into from

Conversation

aquaticsarah
Copy link

Follow up to #25 . Replaces the simple logic in that patch, which can only handle 12-tone equal temperament, with a table-based scheme which can handle any user-specified scale.

@aquaticsarah aquaticsarah force-pushed the quantizer-v2 branch 2 times, most recently from 9245555 to 8b61bd8 Compare March 14, 2022 00:18
Note: For now, the scale cannot be chosen. The input is always quantized
to the nearest 12-tone equal temperament step.
The version of the description added in the previous patch was
marked up wrong, resulting in the text appearing in two separate
boxes rather than one box with two paragraphs inside it.

This patch fixes that issue.
theacodes and others added 12 commits April 23, 2022 21:21
Note: For testing purposes, these sysexes currently affect the live config
directly. A later commit will change them to affect the config in flash.

As the config struct is about 2KB in size, and we end up needing ~2 copies
on the stack in cmd_0x1A_read_quantizer_config_(), we need to increase the
stack size to 8KB to prevent overflow. This can likely be reduced in future.
The previous commits needed more stack size than was previously allocated,
and changed the STACK_SIZE variable in configure.py accordingly.
Strangely, however, the linker still only allocated 2KB to the stack.
On further investigation, the line

    STACK_SIZE = DEFINED(__stack_size__) ? __stack_size__ : 0x800;

doesn't work, it always sets STACK_SIZE to 0x800. But changing it to

    STACK_SIZE = __stack_size__

works perfectly
It is important to ensure that the addresses and lengths of the existing
NVM sections (settings and lut) are not changed by this patch.
I have manually verified this as follows:

* Build the firmware before applying this commit
* Run:
    $ objdump -t build/gemini-firmware.elf | grep _nvm
* Then apply the commit and rebuild
* Run objdump again
* Compare the two sets of outputs. The relevant columns are the first
  (symbol value) and last (symbol name)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants