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

AmpFeature2Client: return fast and slow curves instead of their difference #198

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elgiano
Copy link
Contributor

@elgiano elgiano commented Sep 4, 2022

To better understand how the AmpSlicer works, I find it useful to look at both fast and slow curves instead of only looking at their difference. So I'm making this extra client to get those into a buffer.

  • Single output buffer: numChannels = inputChannels * 2 ([fast, slow] for each input channel)
  • Envelope.hpp: I added a processSampleSeparate method that returns a tuple<double, double>. Original processing code is not repeated, but encapsulated in a private processSampleCommon method.

I'm opening this PR mainly for review of how I wrote the C++, looking forward for style/refactor change requests!
Code is tested (on mono and multi-channel buffers), but not documented yet.

thanks @weefuzzy for the info about custom NRT adaptors!

P.S. I also have the "trivial" SC code for flucoma-sc here. I don't do Max or PD but it should be as trivial :)

purpose: return fast and slow curves instead of their difference. Useful
for understanding and tweaking the differential envelope algorithm
@elgiano
Copy link
Contributor Author

elgiano commented Sep 5, 2022

Afterthought: maybe it would be better, instead of making a new client, to equip AmpFeature with a select mechanism?
I made it here:
dev...elgiano:flucoma-core:feature/ampfeature-select

@jamesb93
Copy link
Member

jamesb93 commented Sep 9, 2022

This is a cool idea! I can't comment on the c++ side of things, but in practice I think that having the object output both curves and the delta would be the easiest way. That way you don't have to do a fancy buffer operation in order to grab the delta and then you could @select the output you want (names unsure). That's just my 2c.

@elgiano
Copy link
Contributor Author

elgiano commented Sep 9, 2022

@jamesb93 I totally agree, and the work is already done on that other branch. The thing is that AmpFeature2 doesn't break the current AmpFeature, while the other branch would. It's possible to set \diff as the only default output for retrocompatibility though. I'm waiting for more comments, but I would actually be happier to close this PR in favor of a new one from dev...elgiano:flucoma-core:feature/ampfeature-select

@weefuzzy
Copy link
Member

Thanks for your patience @elgiano. I think I'm more of a fan of the ampfeature-select branch, but I'll look at both more closely (I seem to vary between preferring more objects vs preferring more attributes / options!).

A next big chunk of work for me is to try and improve the usability of the API for making clients, so this is a really valuable exercise from my perspective!

@elgiano
Copy link
Contributor Author

elgiano commented Sep 12, 2022 via email

@weefuzzy weefuzzy changed the base branch from dev to main February 21, 2023 08:56
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.

3 participants