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

Free/PauseSelf: restate state of Ctor after init sample calc #5914

Merged
merged 2 commits into from
Dec 4, 2022

Conversation

mtmccrea
Copy link
Member

@mtmccrea mtmccrea commented Nov 21, 2022

Reset state of PauseSelf and FreeSelf after init sample calculation, otherwise a trigger on the first sample will be missed.

Note the FreeSelf change fixes an edge case where a UGen would free the synth on the first sample.

This example doesn't currently work, but works with this proposed fix.

(
x = {
	Silent.ar.poll(4, label: "I am not paused.");
	// FreeSelf.kr(Impulse.kr(0)); // immediately free
	FreeSelf.kr(Impulse.kr(1, phase: 0.5)); // free after 0.5 sec
}.play
)

Purpose and Motivation

Fixes #5910, and hopefully SuperDirt #277

Types of changes

  • Bug fix

To-do list

  • Code is tested
  • All tests are passing
  • Updated documentation
  • This PR is ready for review

Copy link
Member

@telephon telephon left a comment

Choose a reason for hiding this comment

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

Just a question: why is this second assignment to zero necessary here and not in other cases, like SetResetFF_Ctor?

@mtmccrea
Copy link
Member Author

mtmccrea commented Nov 21, 2022

It likely will be necessary, I was just fixing the problem with PauseSelf in response to #5910, and went ahead and fixed FreeSelf as well since its behavior is essentially the same so testing both was convenient. The rest will have to follow as part of the long term init sample fix effort.

@dyfer dyfer changed the base branch from develop to 3.13 November 22, 2022 23:01
@dyfer
Copy link
Member

dyfer commented Nov 22, 2022

@mtmccrea could you please rebase this on the 3.13 branch? It should go into the next RC, thanks!

@mtmccrea mtmccrea force-pushed the topic/fix-pauseself-ctor branch from 7aa4f21 to 190ae24 Compare November 27, 2022 11:09
@mtmccrea
Copy link
Member Author

Thanks @dyfer does this look ok?

@dyfer
Copy link
Member

dyfer commented Nov 27, 2022

Hm, the version change commit shouldn't be there. Maybe I messed up something with the branches...?

@telephon
Copy link
Member

I had exactly the same when rebasing. I hand reverted that commit. So yes, maybe there is something wrong in 3.13

@dyfer
Copy link
Member

dyfer commented Nov 28, 2022

@mtmccrea can you do interactive rebase and remove the version change commit?

Sorry the branch state is messy

@mtmccrea mtmccrea force-pushed the topic/fix-pauseself-ctor branch from 190ae24 to 1802580 Compare November 29, 2022 17:18
@mtmccrea
Copy link
Member Author

Thanks @dyfer and @telephon for clarifying, I thought it was strange that that version change commit slipped in.

I think I've resolved it, but just let me know if I need to change anything else.

@dyfer dyfer mentioned this pull request Dec 4, 2022
4 tasks
@dyfer dyfer merged commit 457636b into supercollider:3.13 Dec 4, 2022
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.

Reports of high CPU for 3.13 release candidates
3 participants