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

Rework Max wrapper to allow multiple inputs and different IO sizes fo… #397

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

AlexHarker
Copy link
Contributor

…r control objects

This work supersedes other work towards making the voice allocator object work

What it does

  • allows the wrapper to use multiple inputs for different things (this work was in a voice allocator branch, but it is presented here in an edited form
  • allows for two different ways of setting input size, one of which is dependent on input size and one of which is not - the difference is determinable at compile time / without a client (this relies on a related PR in core (Add a mechanism for communicating streaming size behaviour at compile… flucoma-core#248)

Changes

  • I removed the deferral from high priority thread for resizing based on input. There are other places in max (the app) where allocation happens in high priority and it seemed overly fussy/complicated - the restriction with buffer~s is c74 imposed,
  • even in the wrapper already I can see places where high priority allocation can occur, so it seemed weird to insist on not doing it in this one place (given all the added indirection and logic required to do so).

Notes:

  • functionality for both the bufstats and voice allocator objects should be correct, and @tremblap and I have determined that these should be the only affected objects.
  • similar changes are required in other repos (at least pd, although SC and CLI should be considered also).

Sorry to spam @weefuzzy but I suspect that the removal of the deferral stuff is most contentious here. Also apologies that the PR isn't easier to follow, but there was no obvious way to break this work up.

Copy link
Member

@weefuzzy weefuzzy left a comment

Choose a reason for hiding this comment

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

Looks ok, thanks for trudging through the sludge. Still not a fan of allocating outwith the main thread, but I'm not so impassioned that I'm actually going to argue about it

@AlexHarker
Copy link
Contributor Author

It's not ideal to allocate on a high priority thread, but I think in the max source you would likely find loads of places where it happens (e..g. attribute getters) all the time, so it seems churlish to worry about it here.

Likewise it happens in outputMessage() [several if not all variants] and invokeMessageImpl(), because the memory for std::vector will be on the heap, even though the vectors themselves are local....

@tremblap tremblap merged commit 94337ab into flucoma:main Aug 23, 2023
jamesb93 pushed a commit to jamesb93/flucoma-max that referenced this pull request Aug 27, 2023
…s-fixes

Rework Max wrapper to allow multiple inputs and different IO sizes fo…
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