[alsa-devel] [PATCH - extplug refinement from hw_params 1/1] pcm: extplug: Update slave PCM if extplug slave params were changed
Timo Wischer
twischer at de.adit-jv.com
Fri Dec 7 12:45:38 CET 2018
On 12/7/18 12:03, Takashi Iwai wrote:
> On Thu, 06 Dec 2018 15:31:43 +0100,
> <twischer at de.adit-jv.com> wrote:
>> From: Timo Wischer <twischer at de.adit-jv.com>
>>
>> from extplug hw_params() callback function.
>>
>> This feature is for example required in the following use case:
>> - A filter plugin supports only 1-8 channels. Therefore it calls
>> snd_pcm_extplug_set_slave_param_minmax(SND_PCM_EXTPLUG_HW_CHANNELS, 1,
>> 8) to provide the salve PCM parameters limited to 1-8 channels to the
>> user application
> Which condition is supposed for this constraint (1-8 channels)? If
> this is about the input to the extplug itself, it should be set via
> snd_pcm_extplug_set_param_minmax() instead.
>
> The snd_pcm_extplug_set_slave_param_minmax() is used to restrict the
> output from extplug (i.e. the condition to be passed to the slave).
> This is needed only when the slave PCM can't have the proper input
> constraints. So usually this sets to a single condition or alike.
When using snd_pcm_extplug_set_param_minmax() it will ignore the
capabilities of the slave device. For example the salve device supports
only 2 channels but the extplug supports 1-8 channels and uses
snd_pcm_extplug_set_param_minmax(CHANNELS, 1, 8).
So the user application can open the device with 8 channels and the
extplug has to down mix to 2 channels. But I do not want to support down
mixing in my extplug. I only want to limit the capabilities of the slave
device with the limitations of my extplug and forward this to the user
application.
PS: currently I am investigating in an issue of this patch in case of
MMAP access where snd_pcm_hw_params() fails with EBUSY because
snd_pcm_mmap() is called twice. Therefore please do not merge this
version of the patch.
But anyway I am interested whether you are fine with this approach, now
or I have to find another approach.
Best regards
Timo
>
>
> Takashi
>
>> - The user application requests 2 channels. But in this case the slave
>> PCM will be configured with 1 channel because it is the first valid
>> configuration
>> - To update the salve PCM
>> snd_pcm_extplug_set_slave_param_minmax(SND_PCM_EXTPLUG_HW_CHANNELS, 2,
>> 2) could be called from the extplug hw_params() callback function
>>
>> Without this patch the call to snd_pcm_extplug_set_slave_param_minmax()
>> would not have any effect. With this patch the slave device will also be
>> configured with 2 channels and no down mixing has to be done by the
>> extplug.
>>
>> This new feature can also be used for some specific dependencies. For
>> example a down mixing extplug supports only 2to1 and 4to2 channel down
>> mixing. Therefore the salve PCM has to be opened with 2 channels if the
>> user application requests 4 channels.
>>
>> Signed-off-by: Timo Wischer <twischer at de.adit-jv.com>
>>
>> diff --git a/src/pcm/pcm_extplug.c b/src/pcm/pcm_extplug.c
>> index 1f887c5..44afadb 100644
>> --- a/src/pcm/pcm_extplug.c
>> +++ b/src/pcm/pcm_extplug.c
>> @@ -26,6 +26,7 @@
>> *
>> */
>>
>> +#include <stdbool.h>
>> #include "pcm_local.h"
>> #include "pcm_plugin.h"
>> #include "pcm_extplug.h"
>> @@ -43,6 +44,7 @@ typedef struct snd_pcm_extplug_priv {
>> snd_pcm_extplug_t *data;
>> struct snd_ext_parm params[SND_PCM_EXTPLUG_HW_PARAMS];
>> struct snd_ext_parm sparams[SND_PCM_EXTPLUG_HW_PARAMS];
>> + bool sparams_changed;
>> } extplug_priv_t;
>>
>> static const int hw_params_type[SND_PCM_EXTPLUG_HW_PARAMS] = {
>> @@ -60,18 +62,49 @@ static const unsigned int excl_parbits[SND_PCM_EXTPLUG_HW_PARAMS] = {
>> SND_PCM_HW_PARBIT_FRAME_BITS),
>> };
>>
>> +static bool snd_ext_parm_equal(const struct snd_ext_parm * const a,
>> + const struct snd_ext_parm * const b)
>> +{
>> + if (a != b || a->active != b->active || a->num_list != b->num_list)
>> + return false;
>> +
>> + if (a->num_list > 0) {
>> + for (unsigned int i = 0; i++; i < a->num_list) {
>> + if (a->list[i] != b->list[i])
>> + return false;
>> + }
>> + } else {
>> + if (a->min != b->min || a->max != b->max)
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static int snd_ext_parm_set(struct snd_ext_parm *parm, unsigned int min,
>> + unsigned int max, unsigned int num_list,
>> + unsigned int *list)
>> +{
>> + const struct snd_ext_parm new_parm = {
>> + .min = min,
>> + .max = max,
>> + .num_list = num_list,
>> + .list = list,
>> + .active = 1,
>> + };
>> + const int changed = snd_ext_parm_equal(parm, &new_parm) ? 0 : 1;
>> +
>> + free(parm->list);
>> + *parm = new_parm;
>> + return changed;
>> +}
>> +
>> /*
>> * set min/max values for the given parameter
>> */
>> int snd_ext_parm_set_minmax(struct snd_ext_parm *parm, unsigned int min, unsigned int max)
>> {
>> - parm->num_list = 0;
>> - free(parm->list);
>> - parm->list = NULL;
>> - parm->min = min;
>> - parm->max = max;
>> - parm->active = 1;
>> - return 0;
>> + return snd_ext_parm_set(parm, min, max, 0, NULL);
>> }
>>
>> /*
>> @@ -92,11 +125,7 @@ int snd_ext_parm_set_list(struct snd_ext_parm *parm, unsigned int num_list, cons
>> memcpy(new_list, list, sizeof(*new_list) * num_list);
>> qsort(new_list, num_list, sizeof(*new_list), val_compar);
>>
>> - free(parm->list);
>> - parm->num_list = num_list;
>> - parm->list = new_list;
>> - parm->active = 1;
>> - return 0;
>> + return snd_ext_parm_set(parm, 0, 0, num_list, new_list);
>> }
>>
>> void snd_ext_parm_clear(struct snd_ext_parm *parm)
>> @@ -291,29 +320,37 @@ static int snd_pcm_extplug_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params
>> */
>> static int snd_pcm_extplug_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t *params)
>> {
>> -
>> extplug_priv_t *ext = pcm->private_data;
>> snd_pcm_t *slave = ext->plug.gen.slave;
>> - int err = snd_pcm_hw_params_slave(pcm, params,
>> - snd_pcm_extplug_hw_refine_cchange,
>> - snd_pcm_extplug_hw_refine_sprepare,
>> - snd_pcm_extplug_hw_refine_schange,
>> - snd_pcm_generic_hw_params);
>> - if (err < 0)
>> - return err;
>> - ext->data->slave_format = slave->format;
>> - ext->data->slave_subformat = slave->subformat;
>> - ext->data->slave_channels = slave->channels;
>> - ext->data->rate = slave->rate;
>> - INTERNAL(snd_pcm_hw_params_get_format)(params, &ext->data->format);
>> - INTERNAL(snd_pcm_hw_params_get_subformat)(params, &ext->data->subformat);
>> - INTERNAL(snd_pcm_hw_params_get_channels)(params, &ext->data->channels);
>> -
>> - if (ext->data->callback->hw_params) {
>> - err = ext->data->callback->hw_params(ext->data, params);
>> +
>> + do {
>> + ext->sparams_changed = false;
>> +
>> + int err = snd_pcm_hw_params_slave(pcm, params,
>> + snd_pcm_extplug_hw_refine_cchange,
>> + snd_pcm_extplug_hw_refine_sprepare,
>> + snd_pcm_extplug_hw_refine_schange,
>> + snd_pcm_generic_hw_params);
>> if (err < 0)
>> return err;
>> - }
>> + ext->data->slave_format = slave->format;
>> + ext->data->slave_subformat = slave->subformat;
>> + ext->data->slave_channels = slave->channels;
>> + ext->data->rate = slave->rate;
>> + INTERNAL(snd_pcm_hw_params_get_format)(
>> + params, &ext->data->format);
>> + INTERNAL(snd_pcm_hw_params_get_subformat)(
>> + params, &ext->data->subformat);
>> + INTERNAL(snd_pcm_hw_params_get_channels)(
>> + params, &ext->data->channels);
>> +
>> + if (ext->data->callback->hw_params) {
>> + err = ext->data->callback->hw_params(ext->data, params);
>> + if (err < 0)
>> + return err;
>> + }
>> + } while (ext->sparams_changed);
>> +
>> return 0;
>> }
>>
>> @@ -411,6 +448,7 @@ static void clear_ext_params(extplug_priv_t *ext)
>> int i;
>> for (i = 0; i < SND_PCM_EXTPLUG_HW_PARAMS; i++) {
>> snd_ext_parm_clear(&ext->params[i]);
>> + ext->sparams_changed |= ext->sparams[i].active;
>> snd_ext_parm_clear(&ext->sparams[i]);
>> }
>> }
>> @@ -637,7 +675,9 @@ parameters are sample format and channels.
>> To define the constraints of the slave PCM configuration, use
>> either #snd_pcm_extplug_set_slave_param_minmax() and
>> #snd_pcm_extplug_set_slave_param_list(). The arguments are as same
>> -as former functions.
>> +as former functions. Both functions can also be called from the hw_params
>> +callback. This can be used to further limit the configuration of the slave
>> +device depending on the configuration of the user device.
>>
>> To clear the parameter constraints, call #snd_pcm_extplug_params_reset()
>> function.
>> @@ -768,11 +808,17 @@ void snd_pcm_extplug_params_reset(snd_pcm_extplug_t *extplug)
>> int snd_pcm_extplug_set_slave_param_list(snd_pcm_extplug_t *extplug, int type, unsigned int num_list, const unsigned int *list)
>> {
>> extplug_priv_t *ext = extplug->pcm->private_data;
>> + int changed = 0;
>> +
>> if (type < 0 || type >= SND_PCM_EXTPLUG_HW_PARAMS) {
>> SNDERR("EXTPLUG: invalid parameter type %d", type);
>> return -EINVAL;
>> }
>> - return snd_ext_parm_set_list(&ext->sparams[type], num_list, list);
>> + changed = snd_ext_parm_set_list(&ext->sparams[type], num_list, list);
>> + if (changed > 0)
>> + ext->sparams_changed = true;
>> +
>> + return changed;
>> }
>>
>> /**
>> @@ -790,6 +836,8 @@ int snd_pcm_extplug_set_slave_param_list(snd_pcm_extplug_t *extplug, int type, u
>> int snd_pcm_extplug_set_slave_param_minmax(snd_pcm_extplug_t *extplug, int type, unsigned int min, unsigned int max)
>> {
>> extplug_priv_t *ext = extplug->pcm->private_data;
>> + int changed = 0;
>> +
>> if (type < 0 || type >= SND_PCM_EXTPLUG_HW_PARAMS) {
>> SNDERR("EXTPLUG: invalid parameter type %d", type);
>> return -EINVAL;
>> @@ -798,7 +846,11 @@ int snd_pcm_extplug_set_slave_param_minmax(snd_pcm_extplug_t *extplug, int type,
>> SNDERR("EXTPLUG: invalid parameter type %d", type);
>> return -EINVAL;
>> }
>> - return snd_ext_parm_set_minmax(&ext->sparams[type], min, max);
>> + changed = snd_ext_parm_set_minmax(&ext->sparams[type], min, max);
>> + if (changed > 0)
>> + ext->sparams_changed = true;
>> +
>> + return changed;
>> }
>>
>> /**
>> --
>> 2.7.4
>>
More information about the Alsa-devel
mailing list