wwmm / easyeffects

Limiter, compressor, convolver, equalizer and auto volume and many other plugins for PipeWire applications
GNU General Public License v3.0
6.58k stars 270 forks source link

Clamp values also for LV2 wrapper #2684

Open Digitalone1 opened 1 year ago

Digitalone1 commented 1 year ago

Since it has been made for Ladspa, I tried to do it also for LV2 and I got interesting results.

Declared the defaults in the Port structure.

  float value = 0.0F;  // Control value (if applicable)
  float min = -std::numeric_limits<float>::infinity();
  float max = std::numeric_limits<float>::infinity();

Retrieve min and max values with lilv_plugin_get_port_ranges_float.

Then check bounds when applying the new value in set_control_port_value:

      ui_port_event(p.index, value);

      // Check port bounds
      if (value < p.min) {
        util::warning(plugin_uri + " value out of minimum limit for port " + p.symbol + " (" + p.name + ")");

        p.value = p.min;
      } else if (value > p.max) {
        util::warning(plugin_uri + " value out of maximum limit for port " + p.symbol + " (" + p.name + ")");

        p.value = p.max;
      } else {
        p.value = value;
      }

      found = true;

      break;

I didn't test it on all plugins, but I started to get warnings on knee values whenever I set -24 in compressors: (easyeffects:17913): easyeffects-WARNING **: 15:10:26.376: lv2_wrapper.cpp:375 http://lsp-plug.in/plugins/lv2/sc_compressor_stereo value out of minimum limit for port kn (Knee)

Which is related to #2675

But I suspect it's happening also on other controls for other plugins. I should test it more later.

@wwmm Is it necessary to print this warning? Or should we silently set to min/max when out of bounds?

Digitalone1 commented 1 year ago

Tested a bit more. It happens on lots of db values of various plugins, but only on the minimum limit. At the moment the only one which triggers the warning on both min and max limits is the ALR knee of Limiter plugin.

wwmm commented 1 year ago

Is it necessary to print this warning? Or should we silently set to min/max when out of bounds?

I'm in favor of silently ignoring the warnings by default. But it is useful for development reasons to have an easy way to see these warnings when needed while working in the code.

Digitalone1 commented 1 year ago

I'm in favor of silently ignoring the warnings by default. But it is useful for development reasons to have an easy way to see these warnings when needed while working in the code.

Sure. Later I will make the PR and you can add a developer flag to show the warnings only on testing builds.

Digitalone1 commented 1 year ago

I leave this open to show warnings in set_control_port_value only on development builds.