Neutone / neutone_sdk

Join the community on Discord for more discussions around Neutone! https://discord.gg/VHSMzb8Wqp
GNU Lesser General Public License v2.1
470 stars 22 forks source link

[cm] Expose default params and warn about freezing #20

Closed christhetree closed 1 year ago

christhetree commented 1 year ago

This PR exposes the default params to the VST so that we can load them for local models. It also makes freeze harder to use accidentally. Keep in mind the tensor that is returned is of shape (4, 1), I can make another convenience method that returns a list of four floats if that's easier.

cc: @bogdanteleaga

bogdanteleaga commented 1 year ago

Just a bit confused why this is needed. Is it just a helper function because parsing the metadata is inconvenient? The default values for the parameters should be present in the neutone_parameters variable already, while the dry/wet etc have their own entries in the metadata.

christhetree commented 1 year ago

Yeah this is just a method to make it as easy as possible to access the default parameter values in C++ (similar to how default wet / dry / gain values each have their own dedicated method, although they weren't being exposed to the VST until this PR when I added the decorator). If it's easy for @Fyfe93 to access them via the metadata in the C++ then we can get rid of this method.

bogdanteleaga commented 1 year ago

Ok, exposing the methods is likely fine. Could you remove the new default_param_values field from the metadata itself? That one is redundant as we have the values written in neutone_parameters anyway.

The registered buffer is also directly accessible so now we have two ways of accessing the list without parsing the metadata: the buffer and the now exposed get_default_param_values method.

christhetree commented 1 year ago

Good catch I forget it's in the neutone params. I just removed it. I think I remember Andrew mentioning that it's more annoying to access a field in the model than a method on the VST side, at least now there are both options and it's consistent with the other default params.

bogdanteleaga commented 1 year ago

Okay, looks good to me!