Pipe999Attack999LoopCrossFadeLength and Pipe999Release999ReleaseCrossfadeLength

[oleg68]

As discussed in #1724 different attack and rellease samples may have different crossfades, but now Pipe999LoopCrossfadeLength and Pipe999ReleaseCrossfadeLength values are applied to all attack/release samples.

Introducing the new keys Pipe999Attack999LoopCrossFadeLength and Pipe999Release999ReleaseCrossfadeLength would allow to specify different values for different samples.

[larspalo]

As noted in the discussion cited, it's also possible to load a release from the additional attack samples without any separate specification, thus we should also need a Pipe999Attack999ReleaseCrossfadeLength key to be consistent and comprehensive.

[oleg68]

@larspalo

It is quite clear with Pipe999Attack999LoopCrossFadeLength, but it is not so clear with Pipe999Attack999ReleaseCrossfadeLength and Pipe999Release999ReleaseCrossfadeLength.

GrandOrgue uses the ReleaseCrossfadeLength when it switches from one sample to another sample of the same pipe, for example, from AttackXXX to ReleaseYYY or from AttackXXX to AttackYYY.

Suppose, all these samples have ReleaseCrossfadeLength specified: Pipe999AttackXXXReleaseCrossfadeLength, Pipe999AttackYYYReleaseCrossfadeLength and Pipe999ReleaseYYYReleaseCrossfadeLength. Which of them should GO use for switching in each case?

For switching from AttackXXX to ReleaseYYY seems it is right to use Pipe999ReleaseYYYReleaseCrossfadeLength instead of Pipe999AttackXXXReleaseCrossfadeLength, because the actual fade length depends on the attack duration as well as selecting the appropriate release.

But what ReleaseCrossfadeLength should GO use for switching from AttackXXX to AttackYYY, from the source sample or for the target one?

Seems it would be right to choose appropriate Pipe999ReleaseZZZ sample and take the ReleaseCrossfadeLength from it even for switching between AttackXXX and AttackYYY. @larspalo do you agree with this approach?

Another possible option is to stop switching attack samples at all. The reason is that GO does it when the player presses another key referencing the same pipe with another velocity. But nothing happens with a real pipe organ in the same case. Why does GO need to do switch attack samples in this case? May be it is important for emulating another instruments? @larspalo what is your opinion?

[larspalo]

@oleg68 Let us just leave the LoopCrossfadeLength out of the discussion as it's usage is pretty obvious.

The ReleaseCrossfadeLength affects transition from an attack sample into a release sample. The Pipe999ReleaseCrossfadeLength, Pipe999Attack999ReleaseCrossfadeLength and Pipe999Release999ReleaseCrossfadeLength would all be the same thing, and each is used to allow specifying the value for the specific way of loading a release used.

Remember that a release can be loaded from an(y) attack by means of a cue point in that file or a specified cue in the odf (which currently seems to not be working as it should...), not only by having them separately specified like the SP sample sets usually do.

Seems it would be right to choose appropriate Pipe999ReleaseZZZ sample and take the ReleaseCrossfadeLength from it even for switching between AttackXXX and AttackYYY.

Yes, I think that the value specified for the selected target (release) should be used no matter which attack preceded it. The Pipe999Attack999ReleaseCrossfadeLength is only used for the release that's loaded from the attack file of Pipe999Attack999 (which actually is the default, if Pipe999Attack999LoadRelease is not set to false).

Another possible option is to stop switching attack samples at all.

I'm not exactly sure what you mean by switching attack samples in this context. A pipe may have multiple attacks and they are all expected to be versions of the same pipe speaking (even a tracker pipe organ can have very different speech build-up depending on how the attack was performed by the player). There are currently two main methods to decide what attack should be used: randomly or velocity based (and unfortunately still randomly once the velocity threshold is passed - which is a poor implementation of the feature in GO that eventually should be fixed). For selecting attacks it's also possible to specify a time factor that can affect the selection, as I'm sure you know.

However, if a pipe is already speaking and another key would reference the same pipe in the proper way (by REF or rank referencing) it should not trigger a new attack no matter what velocity is used. (if it does it's a bug)

Having multiple attack samples, versions of how the pipe speaks, is definitely an improvement of the level of realism - and that is something we should not remove... But maybe you're referring to something else?

[oleg68]

I'm not exactly sure what you mean by switching attack samples in this context.

I'm talking about GOSoundEngine::SwitchAttackSampler

It is called when a tremulant is switched on or off when a pipe speaking. In this case GO swithes one attack sample to another one.

What CrossfadeLength should we use for transition between the tremulant on and off?

[larspalo]

@oleg68 Ok, so that's when using wave tremulant samples and the tremulant is turned on/off and a key is held to switch between tremmed and un-tremmed samples.

It's indeed a bit odd that it's the release crossfade value that's used in such cases, but why not keep it the way it currently does as it seems to be working fine, and the same kind of alignment is used for tremmed/no-tremmed transitions that the attack/release transition use? It's a corner case anyway...

[oleg68]

It's indeed a bit odd that it's the release crossfade value that's used in such cases, but why not keep it the way it currently does as it seems to be working fine, and the same kind of alignment is used for tremmed/no-tremmed transitions that the attack/release transition use? It's a corner case anyway...

Currently GrandOrgue uses Pipe999ReleaseCrossfadeLength for this transition. Do you propose to continue doing this?

I don't think that using the release crossfade is odd. The sound fade should be the same in both cases: when the key is released and when the tremulant is switched.

So my (1) option is to select a release sample upon the actual play time and use the ReleaseCrossfadeLength from it even this release is not actually played.

[larspalo]

So my (1) option is to select a release sample upon the actual play time and use the ReleaseCrossfadeLength from it even this release is not actually played.

But this is not what actually happens in GOSoundEngine::SwitchAttackSampler. No release is actually used at all here (it just returns if that would be the case). All that happens is a fade in/out from the tremulant (or non-tremulant) sample to the other version selected or vice versa. Thus, the value taken should be from the attack release crossfade length (actually intended for usage with the loaded release from that same attack sample). Depending on what attack is used it should in other words be from Pipe999ReleaseCrossfadeLength or Pipe999Attack999ReleaseCrossfadeLength.

[oleg68]

But this is not what actually happens in GOSoundEngine::SwitchAttackSampler.

Yes, currently we use the single Pipe999ReleaseCrossfadeLength for all kinds of transitions: Attack->Release and Attack->Attack.

Depending on what attack is used it should in other words be from Pipe999ReleaseCrossfadeLength or Pipe999Attack999ReleaseCrossfadeLength.

Suppose, we are switching Pipe999 from Attack001 to Attack002. What should we use: Pipe999Attack001ReleaseCrossfadeLength or Pipe999Attack002ReleaseCrossfadeLength?

No release is actually used at all here

The only parameter the crossfade length depends on is the time the pipe has spoken for: a short note requires a shorter crossfader. The same dependency should exist when the tremulant is switched on/off just after a very short time the key was pressed. It does not depend on the attack sample at all. Because the release sample is selected basing on the press time, and its crossfade length belongs not to this release sample, but to an attack sample was playing before, it would be correct to use the crosfade length from the release sample even this sample is not actually played in Attack->Attack transition.

I do not agree with using Pipe999Attack999ReleaseCrossfadeLength at all unless we load a release from the Pipe999Attack sample. So I'd put a warning in the log window if Pipe999Attack999ReleaseCrossfadeLength is specified together with Pipe999AttackLoadRelease=N and I'd ignore it.

[larspalo]

I do not agree with using Pipe999Attack999ReleaseCrossfadeLength at all unless we load a release from the Pipe999Attack sample. So I'd put a warning in the log window if Pipe999Attack999ReleaseCrossfadeLength is specified together with Pipe999AttackLoadRelease=N and I'd ignore it.

Yes, that's absolutely right. But most of the time that value used to be calculated as a default since it's (was) extremely rare that anyone would actually use any of the CrossfadeLength variants at all in the odf. Though, the Pipe999ReleaseCrossfadeLength should always be the default used for any child if not overridden.

Suppose, we are switching Pipe999 from Attack001 to Attack002. What should we use: Pipe999Attack001ReleaseCrossfadeLength or Pipe999Attack002ReleaseCrossfadeLength?

We have two transition types: attack/release and attack/attack. In the attack/release we use the targeted release crossfade length, I'd suggest that the same is done with the attack/attack transition. With the current implementation this is never an issue as the pipe level is the only possible level to work on and thus the value would be same for any attack/attack (or a calculated default if not specified).

Personally, I don't think this matters that much. Transition time from tremmed attack to un-tremmed attack sample in a held note should ideally be the startup and slowdown time of the tremulant mechanism itself (not how fast you manage to activate/de-activate the tremulant). Such values are of course not used for wave tremulants as they only can be specified for the synth version.

So, if you really ask my opinion, I do think the current behavior should more or less be kept for the attack/attack switching so that either:

• the calculated default is used if nothing is specified
• Pipe999ReleaseCrossfadeLength is used if specified and no other attack has a specified value
• Pipe999Attack999ReleaseCrossfadeLength is used if specified and is targeted (which would give a possibility to "fake" startup time and stop time for a tremulant)

The real attack/release handling is a different thing and I actually think that's much more important since the attack switching is rarely going to be in effect.

[larspalo]

Pipe999Attack999ReleaseCrossfadeLength is specified together with Pipe999AttackLoadRelease=N and I'd ignore it.

Actually, thinking about it I think it should better be re-phrased to:

(if) Pipe999Attack999ReleaseCrossfadeLength is specified together with Pipe999Attack999LoadRelease=N

since this means that there is no release loaded from that specific attack.

Potentially one could have Pipe999LoadRelease=N to not load the release from Pipe999=Sample but still load the release from Pipe999Attack999=Sample without needing to say anything extra (rather you must say Pipe999Attack999LoadRelease=N to not expect a release to be loaded as that property is never inherited)!

[oleg68]

@larspalo

1. Let's introduce some concepts for better understanding.

   1. A pipe.
   2. A sample. One sample may be either an attack sample (containing loops) or a release sample, but not both.
   3. A wave file where the sample is loaded from.

   GO may load one (attack / or release) sample from one wave file or it may load two (attack + release) samples from the same wave file. In the second case let's talk we have one file but two samples.

2. Let's confirm some principles about ReleaseCrossfadeLength:

   1. A Pipe999ReleaseCrossfadeLen key gives a default value for all release samples of this pipe. By default, it is inherited by all release samples , but not inherited by any attack samples, including by one loaded from the wave file referenced by the Pipe999 key.
   2. If a Pipe999ReleaseCrossfadeLength key is not specified then the default crossfade len is calculated from the midi key number.
   3. Pipe999Attack999ReleaseCrossfadeLength and Pipe999Release999ReleaseCrossfadeLength overide the default values coming from (i) or (ii) for one release sample.
   4. An attack sample never has a ReleaseCrossfadeLength.
   5. A key Pipe999Attack999ReleaseCrossfadeLength belongs to the release sample loaded from the Pipe999Attack999 wave file, not to the attack sample itself.
   6. Pipe999Attack999ReleaseCrossfadeLength must not exist if we do not load a release sample from this file.

In the attack/release we use the targeted release crossfade length,

Let's understand the reason "why": because the crossfade length and the actual release sample are chosen based on the same parameter: the time the source attack sample was played for.

I'd suggest that the same is done with the attack/attack transition

But the reason "why" mentioned above does not more work for attack samples.

for the attack/attack switching so that either:

• the calculated default is used if nothing is specified

Agree

• Pipe999ReleaseCrossfadeLength is used if specified and no other attack has a specified value

Agree

• Pipe999Attack999ReleaseCrossfadeLength is used if specified and is targeted (which would give a possibility to "fake" startup time and stop time for a tremulant)

Don't agree because this contradicts the principle that an attack sample must not have a ReleaseCrossfadeLength.

If we really need a possibility to "fake" tremulant startup/stop time we have to introduce some anothes kind of crossfade length rather than to use ReleaseCrossfadeLength for this purpose. But I would defer this enchancement.

The real attack/release handling is a different thing and I actually think that's much more important since the attack switching is rarely going to be in effect.

Agree.

So I propose to simplify this: to take into account only the default crossfade length for attack/attack transition (calculated or from Pipe999ReleaseCrossfadeLength), but not to take the release crossfade length of the release sample loaded from the same wave file that the targer attack sample.

@larspalo if you agree with this approach, give me a sign and I'm starting to implement it.

[larspalo]

if you agree with this approach, give me a sign and I'm starting to implement it.

@oleg68 Yes, I agree. It's making sense as you say. The release crossfade length value was taken from the pipe, not the attack sample.

[oleg68]

@eturpault @larspalo

I implemented Pipe999Attack999LoopCrossfadeLength, Pipe999Attack999ReleaseCrossfadeLength, and Pipe999Release999ReleaseCrossfadeLength in https://github.com/oleg68/GrandOrgue-official/actions/runs/7388669742

You can test them.

It will be submitted to 3.14.0 as a PR.

[eturpault]

@oleg68 : I added in OdfEdit v2.9 the generation of new keys Pipe999Attack999LoopCrossfadeLength and Pipe999Release999ReleaseCrossfadeLength for the Hauptwerk to GrandOrgue ODF convertion (I don't use Pipe999Attack999ReleaseCrossfadeLength as HW ODF does not permit to define a release crossfade length for an attack sample). The generated ODF is loaded without syntax error by GO v3.13.2-1.15, so these two new keys are recognized as expected.

However :

• I still get 5 errors "Error: Unable to load ... .wav: Not enough samples for a crossfade" with the Prytanée sample set as reported in #1724, but with GO v3.13.2-1.15 these errors are no more blocking the loading of the sample set which can be played as it is. As it is not blocking the loading, could this issue generate a warning instead of an error ? Is the entire pipe actually rejected or only the single loop having the issue is rejected as you suggested to do ? Maybe the warning message should mention rather that one loop has not been loaded.

• The first error log is still mentioning the sample file 061-c#_050.wav instead of 061-c#.wav from which the error comes from according to your analysis.

[oleg68]

However :

The two issues you mentioned occured because you tested the version without #1724 fixed.

Please repeat your test with https://github.com/oleg68/GrandOrgue-official/actions/runs/7449689298 . It includes both changes of #1724 and #1760.

[eturpault]

I just did the test with GO 3.13.3-1.2, indeed all is fixed and there are warning logs instead of error logs to indicate ignored loops, thank you.