[pulseaudio-discuss] [PATCH 1/2] ladspa: D-Bus interface for setting algorithm parameters on-the-fly.

[Tanu Kaskinen]

On Fri, 2012-06-15 at 17:35 +0300, ismo.puustinen at intel.com wrote:
> From: Ismo Puustinen <ismo.puustinen at intel.com>
> 
> A new external D-Bus interface is registered and LADSPA algorithm
> control parameters are exposed as a D-Bus property with setter and
> getter support.

Thanks! I've pushed this now, with the cosmetic changes described below.

> +static pa_dbus_interface_info ladspa_info={

Still missing spaces around '=' here.

> @@ -469,6 +707,276 @@ static void sink_input_mute_changed_cb(pa_sink_input *i) {
>      pa_sink_mute_changed(u->sink, i->muted);
>  }
>  
> +static int parse_control_parameters(struct userdata *u, const char *cdata, double *read_values, pa_bool_t *use_default) {
> +

Extra empty line.

> +static int validate_control_parameters(struct userdata *u, double *control_values, pa_bool_t *use_default) {
> +    unsigned long p = 0, h = 0;
> +    const LADSPA_Descriptor *d;
> +    pa_sample_spec ss;
> +
> +    pa_assert(control_values);
> +    pa_assert(use_default);
> +    pa_assert(u);
> +    ss = u->ss;
> +    d = u->descriptor;
> +    pa_assert(d);

Changed the three last lines to

    pa_assert_se(d = u->description);

    ss = u->ss;

I did that to have better grouping of the input-validating assertions.

> +
> +    /* Iterate over all ports. Check for every control port that 1) it
> +     * supports default values if a default value is provided and 2) the
> +     * provided value is within the limits specified in the plugin. */
> +
> +    for (p = 0; p < d->PortCount; p++) {
> +        LADSPA_PortRangeHintDescriptor hint = d->PortRangeHints[p].HintDescriptor;
> +
> +        if (!LADSPA_IS_PORT_CONTROL(d->PortDescriptors[p]))
> +            continue;
> +
> +        if (LADSPA_IS_PORT_OUTPUT(d->PortDescriptors[p]))
> +            continue;
> +
> +        if (use_default[h]) {
> +            /* User wants to use default value. Check if the plugin
> +             * provides it. */
> +            if (!LADSPA_IS_HINT_HAS_DEFAULT(hint)) {
> +                pa_log_warn("Control port value left empty but plugin defines no default.");
> +                return -1;
> +            }
> +        }
> +        else {
> +            /* Check if the user-provided value is within the bounds. */
> +            LADSPA_Data lower = d->PortRangeHints[p].LowerBound;
> +            LADSPA_Data upper = d->PortRangeHints[p].UpperBound;
> +
> +            if (LADSPA_IS_HINT_SAMPLE_RATE(hint)) {
> +                upper *= (LADSPA_Data) ss.rate;
> +                lower *= (LADSPA_Data) ss.rate;
> +            }
> +
> +            if (LADSPA_IS_HINT_BOUNDED_ABOVE(hint)) {
> +                if (control_values[h] > upper) {
> +                    pa_log_warn("Control value %lu over upper bound: %f / %f", h, control_values[h], upper);
> +                    return -1;
> +                }
> +            }
> +            if (LADSPA_IS_HINT_BOUNDED_BELOW(hint)) {
> +                if (control_values[h] < lower) {
> +                    pa_log_warn("Control value below lower bound: %f / %f", control_values[h], lower);

I added the h value to this log message to be consistent with the
previous log message. I also replaced the "%f / %f" with "%f (lower
bound: %f)" (likewise for the upper bound). 

> +                    return -1;
> +                }
> +            }
> +        }
> +
> +        h++;
> +    }
> +
> +    return 0;
> +}
> +
> +static int write_control_parameters(struct userdata *u, double *control_values, pa_bool_t *use_default) {
> +    unsigned long p = 0, h = 0, c;
> +    const LADSPA_Descriptor *d;
> +    pa_sample_spec ss;
> +
> +    pa_assert(control_values);
> +    pa_assert(use_default);
> +    pa_assert(u);
> +    ss = u->ss;
> +    d = u->descriptor;
> +    pa_assert(d);

I did the same rearrangement as in validate_control_parameters().

> +
> +    if (validate_control_parameters(u, control_values, use_default) < 0) {
> +        return -1;
> +    }

Redundant braces.

-- 
Tanu