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

Set the pitch key center from sample #362

Merged
merged 6 commits into from
Sep 22, 2020
Merged

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented Aug 13, 2020

Add pitch_keycenter=sample. Works with wav and flac files.

#357 @kinwie

@jpcima jpcima requested a review from paulfd August 13, 2020 19:10
@alcomposer
Copy link
Collaborator

Does keyN= override this opcode, as per ARIA? What ARIA does here makes complete sense.

@jpcima
Copy link
Collaborator Author

jpcima commented Aug 14, 2020

No, this opcode has complete priority when present.
it's possible changing it trivially though.

@kinwie
Copy link

kinwie commented Aug 14, 2020

@alcomposer
I'm pretty sure, overrided by key= will lose its reason this opcode was created. I would say this is a special case, like jpcima explain, it has complete priority. It's kind of the 'Lock Root Key' feature common used by samplers. So when it's present, the key= will always function as lokey and hikey. One logical usage for this, is to shift the key-mapping but the root key intact. This shifted mapping then can be used as 'fake round-robin'

@alcomposer
Copy link
Collaborator

Wait... What does ARIA / Sforzando do?

@kinwie
Copy link

kinwie commented Aug 14, 2020

In ARIA, this opcode will lose priority even it placed after key=N. It won't affect the pitch_keycenter set by key=

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

Probably would make sense for pitch_keycenter=sample to only loose priority if key= is after, not before?

In order to maintain SFZ conventions.

@kinwie
Copy link

kinwie commented Aug 14, 2020

Yes if that follow the sfz standard behavior. But the benefit to use pitch_keycenter=sample is by placing it at or upper header level, so it is before the key= which placed at . So it needs to take complete priority no matter where it placed

@kinwie
Copy link

kinwie commented Aug 14, 2020

"In order to maintain SFZ conventions."

Yes of course. But a special case like this, we can documented and explain it at the page

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

Sounds like keyjust sets:
lokey
hikey
pitch_keycenter

So if key is after any of them it will override the previous assignment?

But if key is before any or all of them it will also set/override all three?

@kinwie
Copy link

kinwie commented Aug 14, 2020

OTOH, Aria also did this similar behavior for pitch_keycenter=N when used with key=

<region>
sample=snare.wav
pitch_keycenter=62
key=50
<region>
sample=snare.wav
key=50
pitch_keycenter=62

Both will sound the same in ARIA. pitch_keycenter will always 62 even it is before key=50, which mean pitch_keycenter=N has the complete priority in Aria.

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

So key can't override pitch_keycenter but will override lokey & hikey?

Personally I feel that the SFZ language should convey meaning, which allows a user to understand how to use Opcodes in a universal manner.

If a user did want to achieve the first example: why not write:

<region>
sample=snare.wav
pitch_keycenter=62
lokey=50
hikey=50

Or alternatively have a new opcode:
lohikey=??

@kinwie
Copy link

kinwie commented Aug 14, 2020

"So if key is after any of them it will override the previous assignment?"

  • yes

"But if key is before any or all of them it will also set/override all three?"

  • no

@kinwie
Copy link

kinwie commented Aug 14, 2020

key= can override pitch_keycenter if placed after it.
What I said can't, is only in ARIA

@kinwie
Copy link

kinwie commented Aug 14, 2020

"Probably would make sense for pitch_keycenter=sample to only loose priority if key= is after, not before?"

Yes this is the standard sfz behavior, But if this is the condition, then no we no need to use pitch_keycenter=sample along with key, but can just just pitch_keycenter=N instead at the regions

@alcomposer
Copy link
Collaborator

Sounds like this example of ARIA logic breaks a fundamental concept in SFZ. Sorry, not sorry.

@paulfd
Copy link
Member

paulfd commented Aug 14, 2020

From a usability pov it makes total sense though. Setting pitch_keycenter=sample at the top of a group and setting keys in each region is nice. One could even argue for a key=sample case.

@kinwie
Copy link

kinwie commented Aug 14, 2020

Okay! key=sample that is brilliant!

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

@paulfd Possibly a way to allow such behaviour would be to make it a SFZ convention?

A lock prefix (for all opcodes) could allow them to be locked, and the set value (or values) immutable unless set via another lock.

<group>
lock_sample=*sine
<region>
sample=*saw
<region>
lock_sample=*tri

In the above example:

  • region 1 would play *sine
  • region 2 would play *tri

So- this would work (pertaining to the pitch_keycenter opcode) and give ARIA style locking:

<group>
lock_pitch_keycenter=sample
<region>
sample=sample.wav
key=51

With the above:

  • lokey = 51
  • hikey = 51
  • pitch_keycenter = sample

Simply becuase key is a convenience opcode that sets: lokey hikey & pitch_keycenter.
We are locking the pitch keycenter to the sample metadata, unless:

<group>
lock_pitch_keycenter=sample
<region>
sample=sample.wav
lock_key=51

With the above:

  • lokey = 51
  • hikey = 51
  • pitch_keycenter = 51

Alternatively:
Only the targets of convenience opcodes needs a lock prefix?
lock_lokey
lock_hikey
lock_pitch_keycenter

Because key sets all three values, there is no consistent way to stop the values from being altered.

@kinwie
Copy link

kinwie commented Aug 14, 2020

And then, this simply will works :

<group>
key=sample

<region> sample=Piano_A0.wav
<region> sample=Piano_A#0.wav
<region> sample=Piano_B0.wav
<region> sample=Piano_C1.wav
<region> sample=Piano_C#1.wav
etc..

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

@kinwie But your example of offsetting samples for RR wont?

And while key=sample is a good idea, it would set lokey, hikey & pitch_keycenter to the same metadata in the sample (if there is metadata).

I also think lokey and hikey should get sample as well then.

@kinwie
Copy link

kinwie commented Aug 14, 2020

@alcomposer
Assuming all the piano samples have been well edited and trimmed. My previous examples was for existing free samples at freesound.org, which is raw cutted

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

@kinwie sorry- I was talking about your RR example on Discord

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

To allow a sfizz user freedom to lock the pitch_keycenter=sample without breaking SFZ logic we would need:
lock_lokey
lock_hikey
lock_pitch_keycenter

lock_key (which would override all of the above lock values, or set the values as immutable without a lock prefix)

@redtide
Copy link
Member

redtide commented Aug 14, 2020

it was documented yesterday at https://sfzformat.com/opcodes/pitch_keycenter, don't tell me we need to degrade the CW original behavior because ARIA?

@alcomposer
Copy link
Collaborator

alcomposer commented Aug 14, 2020

@redtide, it's much more serious than behaviour. If pitch_keycenter=sample becomes immutable, nothing can override it.

So a lib maker would have no way to override this setting. Which is a basic concept of SFZ opcodes. (All Opcodes can be overridden)

@paulfd
Copy link
Member

paulfd commented Aug 17, 2020

I'm a bit lost now. What does ARIA do exactly with this? Which software's behavior do we emulate here?

@jpcima
Copy link
Collaborator Author

jpcima commented Aug 17, 2020

At this moment, probably neither, the behavior is very complicated in presence of edge cases.
I haven't checked in detail, but @kinwie has identified some.
Ie. does the redefinition by key matter, does the order matter, does the imbrication matter, etc

@kinwie
Copy link

kinwie commented Aug 18, 2020

I have to repeat this:
I'm pretty sure, overrided by key= will lose its reason this opcode was created. I would say this is a special case, like jpcima explain, it has complete priority. It's kind of the 'Lock Root Key' feature common used by samplers. So when it's present, the key= will always function as lokey and hikey. Otherwise, it will have no advantage over normal pitch_keycenter=N. One logical usage for this, is to shift the key-mapping but keep the root key intact. This shifted mapping then can be used as 'fake round-robin' for example

@@ -215,6 +215,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode)
setRangeStartFromOpcode(opcode, keyRange, Default::keyRange);
setRangeEndFromOpcode(opcode, keyRange, Default::keyRange);
setValueFromOpcode(opcode, pitchKeycenter, Default::keyRange);
pitchKeycenterFromSample = false;
Copy link
Member

Choose a reason for hiding this comment

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

So if I follow what would be the most useful would be to have

if (!pitchKeycenterFromSample)
    setValueFromOpcode(opcode, pitchKeycenter, Default::keyRange);

This would mean that to override pitch_keycenter=sample you need to explicitely have a pitch_keycenter=... opcode. Having key=... would just set the low and high key.

Copy link
Member

Choose a reason for hiding this comment

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

Bump on this one. I think the above solution is the sensible way to go for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Like this? c5a1d04

@kinwie
Copy link

kinwie commented Aug 19, 2020

This would mean that to override pitch_keycenter=sample you need to explicitely have a pitch_keycenter=... opcode. Having key=... would just set the low and high key.

Yes, I think this is the best solution for this moment

@jpcima jpcima merged commit b0d66f0 into sfztools:develop Sep 22, 2020
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.

5 participants