[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [alsa-devel] [PATCH 0/2] sndif: add explicit back and front synchronization



On 03/06/2018 06:30 PM, Takashi Iwai wrote:
On Tue, 06 Mar 2018 17:04:41 +0100,
Oleksandr Andrushchenko wrote:
If we decide to negotiate the parameters, then it can't be done
at .open stage as well, as at this moment we don't know stream
parameters yet, e.g. we don't know the number of channels, PCM
format etc., so we cannot explain to the backend what we want.
Thus, it seems that we need to move the negotiation to .hw_params
callback where stream properties are known. But this leaves the
only option to ask the backend if it can handle the requested
buffer/period and other parameters or not... This is what I do now :(
The additional parameter setup can be done via hw_constraints.  The hw
constraint is basically a function call for each parameter change to
narrow down the range of the given parameter.

snd_pcm_hw_constraint_integer() in the above is just an example.
The actual function to adjust values can be freely written.
Yes, this is clear, the question here mostly was not
*how* to set the constraints, but *where* to get those
... and here comes the hw constraint into the play.

For each parameter change, for example, the frontend just passes the
inquiry to the backend.  The basis of the hw constraint is nothing but
to reduce the range of the given parameter.  It's either interval
(range, used for period/buffer size or sample rate) or the list (for
the format).  When any parameter is changed, ALSA PCM core invokes the
corresponding hw constraint function, and the function reduces the
range.  It's repeated until all parameters are set and settled down.

So, for your driver, the frontend just passes the hw constraint for
each of basic 5 parameters to the backend.  For example, at beginning,
the hw constraint for the buffer size will pass the range (1,INTMAX).
Then the backend returns the range like (1024,65536).   This already
gives users the min/max buffer size information.  The similar
procedure will be done for all other parameters.

In addition, you can put the implicit rule like the integer periods,
which makes things easier.

Thank you very much for such a detailed explanation.
Could you please give me an example of ALSA driver which
code I can read in order to understand how it is supposed
to be used, e.g. which meets the expectations we have for
Xen PV sound driver?
This is the most difficult part, apparently :)
There are lots of codes deploying the own hw constraints, but nothing
is similar like your case.

Suppose that we negotiate from the frontend to the backend like

        int query_hw_param(int parm, int *min_p, int *max_p);

so that you can call like
        err = query_hw_param(PARM_RATE, &min_rate, &max_rate);

This assumes that min_rate and max_rate were already filled by the
values requested from frontend user-space.  In query_hw_parm, the
backend receives this range, checks it, and fills again the actually
applicable range that satisfies the given range in return.

In that way, user-space will reduce the configuration space
repeatedly.  And at the last step, the configurator chooses the
optimal values that fit in the given configuration space.

As mentioned in the previous post, in your driver at open, you'd need
to add the hw constraint for each parameter.  That would be like:

        err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
                                  hw_rule_rate, NULL, -1);

and hw_rule_rate() would look like:

static int hw_rule_rate(struct snd_pcm_hw_params *params,
                        struct snd_pcm_hw_rule *rule)
{
        struct snd_interval *p =
                hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
        int min_rate = p->min;
        int max_rate = p->max;
        struct snd_interval t;
        int err;

        err = query_hw_param(PARM_RATE, &min_rate, &max_rate);
        if (err < 0)
                return err;

        t.min = min_rate;
        t.max = max_rate;
        t.openmin = t.openmax = 0;
        t.integer = 1;

        return snd_interval_refine(p, &t);
}

The above is simplified not to allow the open min/max and assume only
integer, which should be enough for your cases, I suppose.

And the above function can be generalized like

static int hw_rule_interval(struct snd_pcm_hw_params *params,
                            struct snd_pcm_hw_rule *rule)
{
        struct snd_interval *p =
                hw_param_interval(params, rule->var);
        int min_val = p->min;
        int max_val = p->max;
        struct snd_interval t;
        int err;

        err = query_hw_param(alsa_parm_to_xen_parm(rule->var),
                        &min_val, &max_val);
        if (err < 0)
                return err;

        t.min = min_val;
        t.max = max_val;
        t.openmin = t.openmax = 0;
        t.integer = 1;

        return snd_interval_refine(p, &t);
}

and registering this via

        err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
                                  hw_rule_interval, NULL, -1);

In the above NULL can be referred in the callback via rule->private,
if you need some closure in the function, too.
Thank you so much for that detailed explanation and code sample!!!
This is really great to see such a comprehensive response.
Meanwhile, I did a yet another change to the protocol (please find
attached) which will be added to those two found in this patch set
already:
In order to provide explicit stream parameter negotiation between
    backend and frontend the following changes are introduced in the protocol:
     - add XENSND_OP_HW_PARAM_SET request to set one of the stream
       parameters: frame rate, sample rate, number of channels,
       buffer and period sizes
     - add XENSND_OP_HW_PARAM_QUERY request to read a reduced
       configuration space for the parameter given: in the response
       to this request return min/max interval for the parameter
       given
     - add minimum buffer size to XenStore configuration

With this change:
1. Frontend sends XENSND_OP_HW_PARAM_SET to the backend in response
to user space's snd_pcm_hw_params_set_XXX calls, using XenStore entries
as initial configuration space (this is what returned on snd_pcm_hw_params_any)
2. Frontend uses snd_pcm_hw_rule_add to set the rules (for sample rate,
format, number of channels, buffer and period sizes) as you described
above: querying is done with XENSND_OP_HW_PARAM_QUERY request
3. Finally, frontend issues XENSND_OP_OPEN request with all the negotiated
configuration values

Questions:

1. For XENSND_OP_HW_PARAM_SET I will need a hook in the frontend driver
so I can intercept snd_pcm_hw_params_set_XXX calls - is this available in ALSA?

2. From backend side, if it runs as ALSA client, it is almost 1:1
mapping for XENSND_OP_HW_PARAM_SET/snd_pcm_hw_params_set_XXX, so I can imagine
how to do that. But what do I do if I run the backend as PulseAudio client?

3. Period size rules will not allow the check you mentioned before, e.g.
require that buffer_size % period_size == 0). Can frontend driver assume
that on its own? So, I simply add the rule regardless of what backend can?

4. Do you think the attached change together with the previous one (
which adds sync event) makes the protocol look good? Do we need any other change?

Takashi
Thank you very much for helping with this,
Oleksandr

Attachment: 0001-sndif-add-explicit-back-and-front-parameter-negotiat.patch
Description: Text Data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.