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
