[pulseaudio-discuss] [PATCH next v1 7/7] bluetooth: Connect all hooks during module load/unload

[Mikel Astiz]

Hi Tanu,

On Tue, Dec 11, 2012 at 10:20 AM, Tanu Kaskinen <tanuk at iki.fi> wrote:
> 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.

You're right, I didn't actually realize this.

As a general comment, it'd be good if someone could test the
SCO_OVER_PCM part since I'm not able to do it and there's been a lot
changes. It's likely that we have some regressions, I'm afraid.

>> 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.

I will send v2 with the change you suggest.

Cheers,
Mikel