[pulseaudio-discuss] [PATCH 2/2, v2] loopback: Reset sink input rate when source or sink changes

[Georg Chini]

On 10.04.2017 16:36, Tanu Kaskinen wrote:
> On Mon, 2017-04-10 at 16:27 +0200, Georg Chini wrote:
>> On 10.04.2017 16:23, Tanu Kaskinen wrote:
>>> On Sat, 2017-04-08 at 20:34 +0200, Georg Chini wrote:
>>>> If source or sink are changed, the current sink input rate may be different
>>>> from the default rate. Switch sink input rate back to default to avoid the
>>>> influence of the previous combination of source and sink.
>>>> ---
>>>>    src/modules/module-loopback.c | 18 +++++++++++++++---
>>>>    1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/modules/module-loopback.c b/src/modules/module-loopback.c
>>>> index 2242c62c..535c27d2 100644
>>>> --- a/src/modules/module-loopback.c
>>>> +++ b/src/modules/module-loopback.c
>>>> @@ -639,11 +639,18 @@ static void source_output_moving_cb(pa_source_output *o, pa_source *dest) {
>>>>    
>>>>        /* Send a mesage to the output thread that the source has changed.
>>>>         * If the sink is invalid here during a profile switching situation
>>>> -     * we can safely set push_called to false directly. */
>>>> -    if (u->sink_input->sink)
>>>> +     * we can safely set push_called to false directly.  Also, the current
>>>> +     * sampling rate may be far away from the default rate if we are still
>>>> +     * recovering from a previous source or sink change, so reset rate to
>>>> +     * default before moving the source. If the sink is not valid, only
>>>> +     * update the sink input sample spec.*/
>>>> +    if (u->sink_input->sink) {
>>>>            pa_asyncmsgq_send(u->sink_input->sink->asyncmsgq, PA_MSGOBJECT(u->sink_input), SINK_INPUT_MESSAGE_SOURCE_CHANGED, NULL, 0, NULL);
>>>> -    else
>>>> +        pa_sink_input_set_rate(u->sink_input, u->source_output->sample_spec.rate);
>>>> +    } else {
>>>>            u->output_thread_info.push_called = false;
>>>> +        u->sink_input->sample_spec.rate = u->source_output->sample_spec.rate;
>>>> +    }
>>> pa_sink_input_set_rate() does other things too than just set
>>> sample_spec.rate. I think you should always call
>>> pa_sink_input_set_rate(), but since pa_sink_input_set_rate() currently
>>> doesn't expect to be called in the middle of a move, it needs to be
>>> fixed first.
>>>
>> I don't understand why it should not be enough to just set the sink
>> input rate.
> Did you look at the code in pa_sink_input_set_rate()? It updates
> thread_info.sample_spec.rate, calls pa_resampler_set_input_rate() and
> sends a subscription event. Do you think it's ok to skip these things?

OK, but I still think I do not need to fix anything. Since there is no 
output thread
at that point in time, I can directly do what is done in 
PA_SINK_INPUT_MESSAGE_SET_RATE.
The subscription event will be sent by pa_sink_input_finish_move(). I wonder
if we could simply put the update of the in_thread sample rate and the 
resampler
in pa_sink_input_update_rate(), then my patch would need no change and 
we would
also allow other callers to update the rate easily.

>
>> This is also what is done before the sink input is put, so it should be
>> sufficient.
> Which code are you referring to? Searching for "sample_spec.rate = " in
> module-loopback.c doesn't yield any results except the two assignments
> you added in this patch.
>
It's "ss.rate=" in pa__init(). But it probably does not count because it is
set before pa_sink_input_new_data_init()