-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow for audio chaining by returning self object from audio effects and audio mixer
#10635
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have seen this kind of chaining more with properties than function calls, but it's neat how they can be stacked.
I think it would be good to document that self is returned for all these cases, using :return: and :rtype:, and probably give an example of chaining for each case
I've got some basic documentation in there. If you have any suggestions on the language used, I'd be happy to update. Thank you for the review! |
|
@todbot You use synthio a lot, and I do not. Would you have any comment? Thanks! |
Looks neat! I remember advocating for something similar with I didn't realize My only concern (as w/ everything in the core modules now) is that the added feature will push out other modules on ports that are at the limit. |
dhalbert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@todbot I'm not worried about size - the code growth should be tiny or non-existent in this case.
The idea of audio effect/mixer chaining was theorized in one of the first audio effects PRs, #9640 (comment), but never actually implemented.
The updates in this PR are relatively simple. Instead of returning
Nonewithinplay(...)methods of audio objects which allow both input and output (effects and mixer), it instead returns a reference to itself.With this minor update, rather than building audio chains the old way:
You can instead directly chain each source into its destination:
Note: The only method that is missing this chaining ability is
audiomixer.MixerVoice.play. I felt that it made sense to allowaudiomixer.Mixer.playto support this but thought it would result in confusing implementation to supportMixerVoice. (ie:dac.play(mixer.voice[0].play(sample))vsdac.play(mixer.play(sample)).