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

[Georg Chini]

On 10.04.2017 20:51, Georg Chini wrote:
> 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. 
It seems to be easy to fix sink_input_set_rate() however, so I'll go 
ahead and do that ...