Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CLAP 1.1.10 #359
CLAP 1.1.10 #359
Changes from 26 commits
5f6121a
db714f0
858a09f
bd4809d
5820476
e5f86fb
cde87e2
8bc0fae
08c43d6
0efc795
aa8c410
8b7a452
47f1b44
c61906e
555c99d
a401a2f
241899d
d1eb56d
05b1d87
b89953c
c82516c
a8e6a06
92217e0
3929e26
b8e5375
420a542
de9aa5d
5b24f80
5651cd6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
For the
value_to_text()
function, I think it would be useful if the host could know when conversion from the doublevalue
to the string inout_buffer
is trivial (i.e. 1.0 --> "1.0").I could compare the string
std::to_string(value)
with the string returned fromvalue_to_text()
, but they might still be different strings due to precision or locale differences in floating point to string conversions. (for example, "1.0000000" and "1.0" compare as false). This makes it difficult and inefficient for hosts to know whether the parameter value to string conversion was trivial.In LMMS (and presumably any other DAWs where users work with plain integers or floats when editing parameter automation), this often leads to me having to display two nearly identical strings to users alongside each other - "1.0" (the parameter's internal value which users use to automate the parameter) and "1.0000" (the supposedly fancy parameter text from
value_to_text()
). In these cases, I would like to be able to only display one.To fix this issue, maybe the return value of
value_to_text()
could befalse
when an error occurred OR the conversion to string would be trivial? This would be a recommendation for plugins to returnfalse
to let the host do a trivial double-to-string conversion instead of doing that same conversion themselves.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.
Why do your users edit plain floats? To edit a parameter they should type in a string which the host hands to string to value and let the parameter handle it no?
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.
LMMS uses plain float or integer values in the automation editor and parameter editing dialog boxes, not custom strings like those returned from
value_to_text()
. The parameter/automation system could potentially be reworked in the future, but would require a lot of work just to better support CLAP.Until that happens, LMMS and any other hosts that do not (yet) support CLAP's recommended practice of hiding internal parameter values from users and only exposing strings from
value_to_text()
will have an unnecessarily rough time which I believe could be alleviated if CLAP provided better guidance to plugin devs on how to implementvalue_to_text()
.That is, use a return value of
false
to indicate that a particular parameter has no special way of presenting itself and can just be trivially converted to a string. This is similar to how leavingvalue_to_text()
unimplemented indicates that none of the parameters have a special way of presenting themselves.I understand if this is not a change you'd like to make, but to me it is better to specify the meaning of the
false
return value AND make it useful for certain hosts than it is to leave it unspecified and allow diverging implementations in plugins.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.
yeah i view 'false' as in 'i am unable to convert this value to a string'. for instance if you send an out of range value or haven't implemented the clap api. I don't think we should overload it to mean "I did nothing"; but rather it means "don't trust the value of the text buffer as being correctly populated". We could definitely add this to do the doc if people agree, though.
I understand and am sympathetic to the work in LMMS to rework to use strings through the editor. But I'm not that thrilled about changing the base API for that (although others may disagree). Having the param value <> string be bidirectional and both as a requirement of the plugin developer and a guide for the host author seems good.
Perhaps one solution would be to offer an LMMS specific extension where you could advertise this fact?
But also: taking a quick look at surge and the conduit plugins, I think none of our few hundred params would return true - maybe the 8 macros would but I think even those we convert to % * 100 at the edge. I'm not sure how other plugin authors would score.
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.
(Also I'm very excited for LMMS to get CLAP support; many of our users ask for quality electronic open source DAWs which can support Surge and since we don't ship a VST2 etc... if I can help separately from this issue please always do tag me in)
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.
Just to be clear, this isn't a roadblock to implementing CLAP support in LMMS - it's just (in my opinion) a "nice to have" enhancement. If the
value_to_text()
function remains the same as it is, that's fine with me. I just figured it would be worth a shot suggesting a way to change it.I also appreciate your offer to help regarding Surge support in LMMS. In the next few weeks, I'll be trying to get Surge to an operational state in LMMS's CLAP host, so I'll let you know if any issues come up that I don't know how to handle.