[plugins] EnvGen: fix initialization #6175
Open
+101
−36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
d604626
to
564317a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
46aa6c9
to
2c6bb59
Compare
This was referenced Jan 5, 2024
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
2c6bb59
to
838ac4d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
2 times, most recently
from
97e8a83
to
79c3099
Compare
mtmccrea
changed the title
mtmccrea
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
79c3099
to
035ba2f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
035ba2f
to
b276450
Compare
|
Should we have some more tests for envelope? |
|
@mtmccrea thanks for this! Are you considering adding tests for this or would should we pull this in as is? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
mtmccrea
changed the title
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
b276450
to
8089323
Compare
|
I've now added a few tests for the initialization of wrt the env duration bug, #6180, this PR mostly fixes it, though there's an edge case regarding the last sample of the last segment, which should be handled in a separate PR, since this PR is primarily concerned with proper initialization. |
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
4 times, most recently
from
c459080
to
83b6f54
Compare
Delay1.kr hew behavior as of supercollider#6110, in which the predelay sample needs to be specified if not equal to the first input value.
Previously, this value didn't matter because the erroneous UGen initialization swallowed it and it was never output. Now that that's fixed, it gets output for a single sample on account of the construction of the `first0Then1` initial time mechanism. So better to start at (and repeat) the first level, rather than arbitrarily start at `0.0`, which could be destructive if the SynthDef graph isn't designed around the env having a value of 0 (e.g. a freq arg.)
mtmccrea
force-pushed
the
topic/init-sample-lfugens-pt3
branch
from
83b6f54
to
316a306
Compare
|
There was a spurious test timeout failure in the pitch tracker (macOS), which I can't recreate on my machine and doesn't fail after re-running, so I think it's ok... Otherwise I think I've addressed all the issues raised by commenters so far and it's ready for review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose and Motivation
This fixes the initialization of
EnvGenas well as improves (but doesn't fully fix) a segment duration bug, #6180.Additionally:
Env.circlestartup behaviorThis PR is part of an ongoing effort to fix UGen initializations, the progress of which is tracked here.
Note: I did some cleanup of extraneous print statements and stray comments, as a standalone commit. If this isn't preferred, I can remove it. I think it's helpful to clean up as we go.
Types of changes
To-do list
the state of the CoreUGens unit test is the same as reported in Fix UGen initialization: LFUGens pt. 1/3, Trig, Trig1 #6035 (all relevant tests passing)TestEnvGen_server:test_shapeHold_setEndValuefails but is fixed by fixed by Tests: TestEnvGen_server: fix 2 tests #6185