On Tue, 2012-12-11 at 09:08 +0100, Mikel Astiz wrote: > Hi Tanu, > > On Tue, Dec 11, 2012 at 6:10 AM, Tanu Kaskinen <tanuk at iki.fi> wrote: > > On Mon, 2012-12-10 at 08:30 +0100, Mikel Astiz wrote: > >> From: Mikel Astiz <mikel.astiz at bmw-carit.de> > >> > >> Move the connection of sink/source-related hooks to module > >> initialization and shutdown, to group all of them together. There is > >> no need to connect them every time the card profile is changed. > > > > sink_state_changed_cb and source_state_changed_cb have to be modified a > > bit, basically just return without doing anything if USE_SCO_OVER_PCM() > > returns false. With the current code this patch can cause an assertion > > crash because of this in the beginning of sco_over_pcm_state_update(): > > > > pa_assert(USE_SCO_OVER_PCM(u)); > > I don' think this scenario is possible since sco_source and sco_sink > should be NULL and thus sco_over_pcm_state_update() would never be > called, due to the comparison in both xxx_state_changed_cb(). Why do you say sco_source and sco_sink should be NULL? They're set during module initialization and never changed, so they definitely can be non-NULL. USE_SCO_OVER_PCM() returns false if the current profile is not HSP, but with your patch xxx_state_changed_cb() can be called also when some other profile is active, and thus crashing may occur. > However, I agree that we should make this more explicit. We could do > what you propose or alternatively (as the previous code does) just > connect the hook only if USE_SCO_OVER_PCM. Any preference? I prefer having the hook always connected, for simplicity. -- Tanu