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

JI op corrected to map to volt/octave values #133

Merged
merged 2 commits into from Nov 17, 2017
Merged

Conversation

@trentgill
Copy link
Contributor

@trentgill trentgill commented Nov 17, 2017

What does this PR do?

Corrects the functionality of the JI op. It previously gave raw ratio conversions that didn't relate to the 1volt / octave standard in Eurorack.

Uses a prime-factorization technique, which goes up to 13-limit tuning. Should be sufficient for most musical use-cases. Every extra prime extends worst case search time for invalid ratios.

Presently an invalid ratio (one which cannot be factored with primes of
13 or less) simply returns 0 (the base pitch). All results are scaled
between 0 and V 1.

The weird list of constants is calculated with a logarithmic function, representing some kind of musical fraction of the octave. These constants can simply be composed with addition (numerator) and subtraction (denominator) to find the JI result. Prime factorization is used to break down composite input into it's prime components.

How should this be manually tested?

I tried to add these tests in process_tests.c but they would always fail. Any ideas how? I can push those tests too if I can figure out why they didn't work
"JI 0 0" -> 0
"JI 3 2" -> 958
"JI 1323 -1024" -> 605
"JI -40 39" -> 60
"JI 23 13" -> 0

If the related Github issues aren't referenced in your commits, please link to them here.

#129

I have,

  • updated CHANGELOG.md
  • updated the documentation
    No changes to documentation required.

I also wrote a little script that generates the constants stored in the function. I would normally include them in a block comment immediately above the function, but it didn't seem to fit with the style. Is there somewhere more comments or info are stored about the code?

Here's the function for reference:
int16_t ji_find_prime_constant( uint16_t prime )
{
float r = 1638.0 * logf( (float)prime ) / log( 2.0 );
while( r > 1638.0 ){ r -= 1638.0; }
r *= 16.0;
return( (int16_t)( r + 0.5 ) );
}

trentgill added 2 commits Nov 17, 2017
Uses a prime-factorization technique, which goes up to 13-limit tuning.
Should be sufficient for most musical use-cases. Every extra prime
extends worst case search time for invalid ratios.

Presently an invalid ratio (one which cannot be factored with primes of
13 or less) simply return 0 as the base pitch. All results are scaled
between 0 and V 1.
Copy link
Contributor

@burnsauce burnsauce left a comment

I can't verify the math, but the code looks clean.

@cmcavoy
Copy link
Contributor

@cmcavoy cmcavoy commented Nov 17, 2017

bitmoji

@tehn tehn merged commit fb2ed73 into monome:master Nov 17, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@trentgill trentgill mentioned this pull request Jan 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants