[pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions

Tanu Kaskinen tanuk at iki.fi
Fri Apr 26 08:56:22 UTC 2019

On Tue, 2019-04-23 at 21:20 +0200, Georg Chini wrote:
> On 22.04.19 09:34, Tanu Kaskinen wrote:
> > On Sat, 2019-04-20 at 11:38 +0200, Georg Chini wrote:
> > > PA uses malloc() in the IO-thread, so are we doing things wrong?
> > > I think using malloc() when a parameter changes is not interfering
> > > with real-time operation because the filter must be reset after
> > > a parameter change anyway.
> Am I allowed to use free() from the I/O-thread?

No. To my knowledge a lot of the memory management work may actually
happen in free() rather than malloc().

> The current scheme for 
> updating
> parameters I have in mind should work for any of the existing filters 
> and relies on
> passing around parameter structures:
>      /* The following functions can be provided to set and get 
> parameters. The parameter
>       * structure is defined by the filter and should contain all 
> parameters that are
>       * configurable by the user. The library code makes no assumption 
> on the contents
>       * of the structure but only passes references. The library 
> implements a message
>       * handler which supports the following messages that use the 
> functions below:
>       * get_parameter - Retrieve a single parameter.
>       * set_parameter - Set a single parameter.
>       * get_all_parameters - Retrieve all parameters as a comma 
> separated list.
>       * set_all_parameters - Set all parameters simultaneously.
>       * get_parameter_description - Returns a list that describes all 
> parameters.
>       *                             Format of the list elements is:
>       * {{Identifier}{Type}{default}{min}{max}}
>       * The last message can be used to get information about the filter 
> or to implement
>       * a filter control GUI that is independent of the filter type. */
>      /* Get the value of the parameter described by identifier. The 
> value shall be returned
>       * as a string in value. The identifier may be any string that the 
> filter recognizes
>       * like a name or index, depending of the implementation of this 
> function. */
>      int (*parameter_get)(const char *identifier, char **value, void 
> *userdata);
>      /* Sets the value of the parameter described by identifier. The 
> value is expected as
>       * a string. The identifier may be any string that the filter 
> recognizes like a name
>       * or index. Returns a parameter structure filled with all current 
> parameter values,
>       * reflecting the updated parameter, so that the structure can be 
> passed to
>       * update_filter_parameter(). update_filter_parameter() will 
> replace the current
>       * parameter set with the new structure. */
>      void *(*parameter_set)(const char *identifier, const char *value, 
> void *userdata);
>      /* Returns a comma separated list of the values of all filter 
> parameters. */
>      char *(*parameter_get_all)(void *userdata);
>      /* Expects a comma separated list of all filter parameter values 
> and returns a parameter
>       * structure that can be passed to update_filter_parameters(). See 
> set_parameter(). */
>      void *(*parameter_set_all(const char *all_params, void *userdata);
>      /* Returns a parameter description string as described above. */

Some comments on the interface:

"Parameters as a comma separated list" sounds potentially problematic
if the specification extends to the public API as well, unless all
possible parameter types are defined to not contain commas. Why is the
list not encoded in the same way other lists are encoded in the message

"{{Identifier}{Type}{default}{min}{max}}" is probably too strict for
the parameter description specification, if it extends to the public
API as well. Min/max only works for range values, but I can imagine
some filter having a "choose one from a list of non-numeric options". I
think the structure after {default} should be dependent on the
parameter type.

> This would allocate the memory in the main thread but would require that
> the I/O-thread frees the old parameter structure when it is replaced. 
> Alternatively
> the function that replaces the old structure with the new one could return a
> pointer to the old structure, so that it can be freed in the main thread.

That should work. The structures probably have fixed size, though, in
which case malloc()/free() can be avoided altogether: the filters can
allocate two instances of the internal structures during intialization 
and switch between them when parameters are updated.



More information about the pulseaudio-discuss mailing list