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

Tuning #253

Merged
merged 15 commits into from
Jun 10, 2020
Merged

Tuning #253

merged 15 commits into from
Jun 10, 2020

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented May 21, 2020

This adds API support and LV2 controls for tuning.

  • scala tuning file
  • scala root key
  • reference 440 frequency

It's similar to Sforzando but with a small difference.
The Sforzando root key designates the top note of the Scala, whereas Scala itself designates the lower note. (and so does sfizz)

It's not perfect because the parameters root key and tuning frequency are not currently RT-safe.
It's because the Surge tuning library implements the mapping change by std::string and IO streams.
It can be resolved by making some light changes into the library.

Other note: the implementation into sfizz is by retuning a note as it starts.
It's the same as taking the frequency, retuning, and remapping it into MIDI keys.
Then it proceeds with this fractional key (a real), used in relation to the pitch keycenter.

For example: under 432 Hz tuning
A4 note 69 -> fractional note 68.68

@jpcima jpcima marked this pull request as draft May 21, 2020 13:10
@jpcima jpcima marked this pull request as ready for review May 21, 2020 13:33
@jpcima
Copy link
Collaborator Author

jpcima commented May 21, 2020

RT safety problems resolved, except some functions we don't use. (eg. evenDivisionOfSpanByM)

@paulfd
Copy link
Member

paulfd commented May 29, 2020

Unlike the SFZ file, we could use the LV2 abstract_path extension to save the scala file in a portable way. I can add this if you wish.

I wonder if it would be also nice to have a "reload check" also on the scala tuning? If people want to tweak it on the fly. In this case, using the abstract_path may not be a good idea, as it would copy the original file in the session data.

@jpcima
Copy link
Collaborator Author

jpcima commented May 29, 2020

This could mean auto-reloading will be only valid for the current session where the file was picked, and then no longer after reloading, unless the user would to re-select it.

A thing: isn't the file with virtual path that Ardour keeps just a symlink to the real file?
In this case it should not cause any problems.

@paulfd
Copy link
Member

paulfd commented May 29, 2020

Actually it seems like it, but then I don't understand the point? If you move the session on disk or to another computer, the symlink will break?

@paulfd
Copy link
Member

paulfd commented May 29, 2020

I'm more in favor of auto-reloading and not using the abstract path thing.

@jpcima
Copy link
Collaborator Author

jpcima commented May 29, 2020

I'm more in favor of auto-reloading and not using the abstract path thing.

Fine for me

@paulfd
Copy link
Member

paulfd commented May 29, 2020

I added the auto-reloading and corrected a couple of small bugs in the LV2 in the process! Looks OK to me now, except I have these warnings:

/home/paul/source/sfizz-jpcima/src/sfizz/Tuning.cpp: In member function ‘bool sfz::Tuning::loadScalaFile(const std::filesystem::__cxx11::path&)’:
/home/paul/source/sfizz-jpcima/src/sfizz/Tuning.cpp:164:36: warning: variable ‘kbm’ set but not used [-Wunused-but-set-variable]
  164 |     const Tunings::KeyboardMapping kbm = impl_->tuning().keyboardMapping;

They're indeed unused, just want to check with you that they're not supposed to be used ;)

@jpcima
Copy link
Collaborator Author

jpcima commented May 29, 2020

Yes it's a leftofver of how I updated the scale which keeping the previoud kbm.
This has been moved into Impl when I figured I needed a more complex logic.
OK to remove it.

…codepathsReactivate the periodic file checking after loading a scala file
src/sfizz/Tuning.cpp Outdated Show resolved Hide resolved
@jpcima
Copy link
Collaborator Author

jpcima commented May 29, 2020

It appears to be good overall.
@alcomposer it's done, feel free to give this a test

I don't have yet a determined a parametric formula for the stretch tuning that seems really adequate, it will be an extra for another PR.

@jpcima jpcima merged commit a369d61 into sfztools:develop Jun 10, 2020
@paulfd paulfd linked an issue Jun 15, 2020 that may be closed by this pull request
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.

Micro-tuning
2 participants