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

Solve some issues with DSmolken's drumkits #172

Merged
merged 6 commits into from
Apr 14, 2020
Merged

Conversation

paulfd
Copy link
Member

@paulfd paulfd commented Apr 7, 2020

  • sample=*silence table was not initialized and could output crazy values
  • The free-running envelopes seem to play out the decay in ARIA; now we go directly to the release stage in sfizz. What does Dimension do?

If Dimension does the same as ARIA, we may need to add a counter to the decay stage on top of the rate. There is an example below.
Example.zip

Note also that loop_mode=one_shot does not free-run on generator regions in ARIA, but it does on sample regions.

Sorry, something went wrong.

@paulfd paulfd requested a review from jpcima April 7, 2020 20:34
@jpcima
Copy link
Collaborator

jpcima commented Apr 7, 2020

Some recordings of above sfz in cakewalk
eg_decay.tar.gz

@jpcima
Copy link
Collaborator

jpcima commented Apr 8, 2020

This is at the generation of the decay stage.
while (count < size && (currentValue *= decayRate) > sustain)

The top of the attack stage is 100% level, and so is sustain level in this case.
The condition currentValue > sustain never holds, and the decay generator is never entered.

However, when using devtools/CaptureEG, it's observed that both Cakewalk and ARIA have some very long decays. (~18 seconds long)

@paulfd
Copy link
Member Author

paulfd commented Apr 8, 2020

So it seems that the envelope is not really free running in fact. The "release" happens when the sample has ended. The free running logic we have atm seems flawed, I added the one shot loop mode check for one of Alex's example that seemed reasonable but apparently ARIA does not follow this. What does Cakewalk do if you replace the sample by a generator?

@paulfd
Copy link
Member Author

paulfd commented Apr 8, 2020

Changing this check

freeRunning = (
    (region.trigger == SfzTrigger::release)
    || (region.trigger == SfzTrigger::release_key)
    || (region.loopMode == SfzLoopMode::one_shot)
);

into

freeRunning = (
    (region.trigger == SfzTrigger::release)
    || (region.trigger == SfzTrigger::release_key)
    || (region.loopMode == SfzLoopMode::one_shot && (region.isGenerator() || region.oscillator))
);

could be a way but this boolean is quite ugly.

Edit: it seems to work out though, tried it quickly.

@paulfd paulfd marked this pull request as ready for review April 8, 2020 12:47
@jpcima
Copy link
Collaborator

jpcima commented Apr 8, 2020

This is another of ARIA. This has AHDS but no R phase.
The whole envelope is longer that the audio file which is 11 seconds.

 ampeg_attack=1.0 ampeg_hold=1.0 ampeg_decay=1.0 ampeg_release=30.0
ampeg_sustain=50
 loop_mode=one_shot

Capture du 2020-04-08 15-21-11

@paulfd
Copy link
Member Author

paulfd commented Apr 8, 2020

But there is an R phase in the file. You mean that the release phase is never active on the EG?

This fits with "loop_mode=one_shot and a sample file -> only play the EG up to the sustain and ignore the release" logic.

@jpcima
Copy link
Collaborator

jpcima commented Apr 14, 2020

Correction on what is said above, it's correct.
There was a problem of the capturing program that created the plot above. It was not prepared to support loop mode, so the playing key had impact on the length and envelope.

It's good.
Capture du 2020-04-14 14-22-11

@paulfd
Copy link
Member Author

paulfd commented Apr 14, 2020

Note that working on the smoothers, I suspect ARIA has a tiny "gain smoother" as I'm thinking of implemeting (#153 #181), which can explain the difference in the release and decay envelopes.

@jpcima
Copy link
Collaborator

jpcima commented Apr 14, 2020

I'm not sure to understand immediately what this is, will check.
Given this one is validated, I will merge it.

@jpcima jpcima merged commit 4c6febe into sfztools:develop Apr 14, 2020
@paulfd paulfd deleted the eg-issues branch January 21, 2022 10:24
essej pushed a commit to essej/sfizz that referenced this pull request Sep 23, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Issues:   Update bug_report.yml to include an example unit test (sfztools#166)

* Benchmarks:   Added a benchmark for looping ContainerClips

* CombiningNode:   Improved the speed of CombingNode::addInput, particularly with large numbers of clips, by using std::partition

* TempoSequence:   Fixed updating sequences multiple times

* Time:   Added some functions to EditTime & EditTimeRange

* CombiningNode:   Fixed an issue where nodes could be processed without being prepared

* Container Clip:   Refactored to flatten contained clips in to the main Edit. Has some limitations at the moment

* Node:   Made createNodeMap recursive to find nested internal Nodes

* ContainerClip:   Make sure each WaveNode gets a unique ID

* Tests:   Added UBsan to macOS tests

* ContainerClip:   Started adding dynamic offset to WaveNodeRealTime

* ContainerClip:   Started adding support for USE_DYNAMIC_OFFSET_CONTAINER_CLIP (to avoid flattening container clips)

* Container Clip:   Fixed dynamic offset for changing graphs and non-zero loop starts

* Container Clip:   Added a DynamicallyOffsettableNodeBase class and used this to support contained clip fades

* Container Clip:   Avoided hiding DynamicallyOffsettableNodeBase functions

* Container Clip:   Fixed compiler errors

* Container Clip:   Fixed some namespace conflicts

* Container Clip:   Added processing of looped ranges

* Container Clip:   Ensure offset change triggers a wave crossfade

* ContainerClip:   Avoided clicks introduced by chunking loop boundaries

* ContainerClip:   Fixed slightly incorrect loop point by using a local ProcessState

* ContainerClip:   Removed an unused variable

* Container Clip:   Added some tests

* Container Clip:   Worked around a Windows warning

* Container Clip:   Commented out a line that will be eventually removed

* Utils:   Improved createGraphDescription to remove duplicate edges and include Nodes that use memory

* Player:   Added a check for cycles in a graph to prepareToPlay

* Container Clip:   Avoided processing 0 samples

* ContainerClip:   Removed the flattening implementation

* Nodes:   Fixed a potential memory corruption caused by claiming incorrect nodes

* ContainerClip:   Added an additional assertion
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.

None yet

2 participants