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

adding ext/draft/gain-reduction.h #440

Merged
merged 3 commits into from
Jan 17, 2025
Merged

Conversation

schwaaa
Copy link
Contributor

@schwaaa schwaaa commented Jan 8, 2025

Basic extension for plugin to report gain reduction to the host.

typedef struct clap_plugin_gain_reduction {
// reports the current gain reduction in dB. the value is intended
// for informational display only, for example in a host meter or tooltip.
double(CLAP_ABI *current_gain_reduction_db)(const clap_plugin_t *plugin);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it called on the audio thread after the process call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think either a main thread only or audio thread only model is reasonable, but my suggestion would be to explicitly document it as threadsafe. The processing time associated with the returned value is whatever the plugin thinks "current" means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably best to document as well that the returned value is negative if the gain is reduced. Note that as written, this extension is essentially a copy of https://github.com/fenderdigital/presonus-plugin-extensions/blob/main/ipslgainreduction.h.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that the logical place to query that value is after the process call (audio-thread). Then the host can carry the value to its GUI itself.

You should add that this interface should only be implemented if the gain reduction is relevant for this plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about renaming current_gain_reduction_db() to simply get()?

Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to mention that while the plugin isn't processing, the value can be assumed to be 0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that stereo gain reduction is probably not important, we can keep it mono and see if anyone complains.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we name the extension clap_plugin_gain_reduction_feedback? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the extension name is fine, but if you want to change it, I would suggest clap_plugin_gain_reduction_reporting rather than _feedback.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think the extension name is fine.

@Trinitou
Copy link
Contributor

Trinitou commented Jan 8, 2025

Should there be a fixed min/max dB value defined in a comment so that it is clear for the host what the min range needs to be in the case of drawing a gain reduction meter?

@Trinitou
Copy link
Contributor

Trinitou commented Jan 8, 2025

If a plugin can change its type dynamically (e.g. if it is a compressor or an eq), should there be a mechanism to report that to the host?

@schwaaa
Copy link
Contributor Author

schwaaa commented Jan 8, 2025 via email

@Bremmers
Copy link

Bremmers commented Jan 9, 2025

Not sure positive values should be allowed.
In our UI 0dB is on the right if it's a gain reduction meter. If it can boost too, 0dB would be at center position. But with the current design we don't know what kind of meter it is.
If positive values are allowed hosts are more or less forced to do something with it, or it won't work right with some plugins.
It would be easier to just stick to reduction.

@abique
Copy link
Contributor

abique commented Jan 13, 2025

Let's say it is a gain feedback instead.
So +3, means +3dB and -3 means -3dB.
In Bitwig, if the gain is negative, we could paint something in blue from top to bottom; if the gain is positive something in red from bottom to top.

That way the feedback would have a clear center value (0 for 0dB); it works both for reduction and expansion.
The extension would need to be renamed to gain_feedback or something like that to remove the possible confusion regarding the interpretation of the value direction.
What do you think?

@schwaaa
Copy link
Contributor Author

schwaaa commented Jan 13, 2025

I think it's useful to support reporting positive gain adjustments. I think the name gain_reduction is fine even if positive adjustments are supported, as long as the documentation is clear, which I think it is currently. If the name is changed, I would suggest gain_adjustment or gain_adjustment_reporting instead of feedback, which imo has too many other meanings in an audio context.

@Bremmers
Copy link

Bremmers commented Jan 13, 2025

With the presonus (reduction only) idea it's clear it's about the dynamic reduction without any make-up gain. That's how I understand it at least. Because if the user turns make-up gain all the way up the level would be > 0dB all the time, and the meter wouldn't work.

If gain is allowed too, some plugins may include make-up gain. Now you're no longer sure what the meter means exactly. And the CLAP version of a plugin can be different than the VST3 version.

Perhaps we can add a comment like "the value is supposed to represent dynamic compression/expansion, not static gain like make-up gain."
Or "0dB means the signal is neither compressed nor expanded"

@schwaaa
Copy link
Contributor Author

schwaaa commented Jan 13, 2025

Good point about make-up gain. I've added to the documentation/comment in the header to clarify this.

@abique
Copy link
Contributor

abique commented Jan 17, 2025

Thanks 👍

@abique abique merged commit 4e95590 into free-audio:next Jan 17, 2025
4 checks passed
abique pushed a commit that referenced this pull request Jan 17, 2025
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.

4 participants