[pulseaudio-discuss] [PATCH v4] Make module loopback honor requested latency

[Tanu Kaskinen]

On Mon, 2015-02-23 at 18:56 +0100, Georg Chini wrote:
> On 23.02.2015 08:02, Georg Chini wrote:
> > On 22.02.2015 23:25, Alexander E. Patrakov wrote:
> >
> >> Anyway, the original submission (i.e. the patch that I am replying 
> >> to) has a bug: it crashes PulseAudio in the Bluetooth A2DP -> HDA 
> >> test if one changes the sink profile from Stereo Output to 2.1 
> >> Surround Output while the loopback is active. Before the patch, there 
> >> was no such crash.
> >>
> >
> > I do not think that is a bug in the module. It looks like the sink has 
> > already changed when
> > pa_sink_input_may_move_to_cb() is called and that should definitely 
> > not be the case.
> > The original version does not show that problem because it never 
> > resets the rate when source
> > or sink changes.
> >
> > Not too sure, but maybe this code in card_set_profile in 
> > module_alsa_card.c, line 223 already frees
> > the sink before it should?
> >
> >             sink_inputs = pa_sink_move_all_start(am->sink, sink_inputs);
> >             pa_alsa_sink_free(am->sink);
> >             am->sink = NULL;
> >
> >
> I have looked into this somewhat more, and it seems like the 
> may_move_to() function of the
> sink input cannot hold what it promises. In sink-input.h the description is:
> 
>      /* If non-NULL this function is called before this sink input is
>       * move to a sink and if it returns false the move will not
>       * be allowed */
>      bool (*may_move_to) (pa_sink_input *i, pa_sink *s); /* may be NULL */
> 
> When the profile changes, the old sink does no longer exist when 
> may_move_to() is called,
> because it must be destroyed to create a new sink with the different 
> profile. So it is obviously
> too late to cancel the move here (and module-loopback cannot reset the 
> rate).
> Unfortunately in this situation there is also no way to call 
> may_move_to() before the old sink is
> destroyed because the new sink is not known before it is created.
> 
> So my question: Should there be a function may_move() that is called 
> before anything is done?
> 
>      /* If non-NULL this function is called before this sink input is
>       * moved anywhere and if it returns false the move will not
>       * be allowed */
>       bool (*may_move) (pa_sink_input *i); /* may be NULL */
> 
> The call to this function could be added to pa_sink_input_may_move().
> 
> The same probably applies for the source output.

Why is sink_input_may_move_to_cb() in module-loopback.c calling
pa_sink_input_set_rate()? That doesn't sound like a proper place to do
such a thing.

If there was the kind of function that you propose, would it return
false in this scenario? If so, that would just make module-alsa-card.c
kill the sink input, because it's not going to cancel the profile change
just because some sink input refuses to be moved.

-- 
Tanu