On Fri, 07 Dec 2018 12:52:30 +0100,
Takashi Iwai wrote:
>
> On Fri, 07 Dec 2018 12:45:38 +0100,
> Timo Wischer wrote:
> >
> > On 12/7/18 12:03, Takashi Iwai wrote:
> > > On Thu, 06 Dec 2018 15:31:43 +0100,
> > > <twischer@xxxxxxxxxxxxxx> wrote:
> > >> From: Timo Wischer <twischer@xxxxxxxxxxxxxx>
> > >>
> > >> 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.
>
> Well, if you just need some additional constraints that is specific to
> the explug, keep the constraints linked to slave, but apply the
> additional refine on top of it. That should make things much easier,
> I suppose.
But actually I'd like to see exactly what's failing before going
further. Could you give a simple test case? You can write a dummy
extplug plugin that just shows the behavior, for example.
thanks,
Takashi
>
>
> Takashi
>
> >
> > 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@xxxxxxxxxxxxxx>
> > >>
> > >> 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
> > >>
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
