Hi Pierre,

On Fri, Apr 8, 2011 at 3:42 PM, pl bossart <bossart.nos...@gmail.com> wrote:
>>> I've not looked specifically at the code, but I'd have expected that
>>> jack detection would somehow be built into module-alsa-card or
>>> module-alsa-source/sink rather than shipped as a separate module (tho'
>>> I'm maybe not appreciating some intricacy here)... I'll try and review
>>> the code soon (although others may have beaten me to it anyway :D)
>>>
>>

Jack detect does not use the ALSA kernel subsystem but does instead
use the input subsystem for jack status. It makes sense to create a
new module so we can then use jack detect for non ALSA sound devices.

>> Well the UCM code corresponding on what to do when jack is plug or
>> unplug  its part of module-alsa-card but we had to add first the logic
>> in PA to detect the jack insertation/removel  that it is basically a
>> listener of /dev/input/eventX, that is used by the kernel to report
>> the event. All the logic for jack detection has been added in a new
>> module.
>>
>> I'll wait your feedback/questions related to UCM integration :)
>
> I have pretty much the same feedback as Colin.
> First I cherry-picked the two jack-sensing patch and installed them on
> my laptop (68293cd29b1dcb6b555edeaa5d63110164e5c794 and
> 6c4a7de60040d5dc3d3b44461a7f490e3feba26f). Works fine, the events are
> detected. This is something that we've needed for some time. Thanks!
>
> But then I looked a the flows and was confused by the design.
> - It's not clear why you would create a new thread using pthreads
> rather than pa_create_thread()? Is there any technical reason why the
> abstraction isn't enough

noup, I didn't know about pa_threads.

> - this thread blocks on a read, and whenever a jack insertion/removal
> is detected it fires a Hook which is then used by module-alsa-card to
> actually switch profiles in two callbacks.
> Why not implement this thread as part of module-alsa-card then? What
> if the benefits of using the hook as a communication means between two
> modules? I can see though the benefit of using a hook for, say, an
> effect that is enabled only for a headphone. That is more the intended
> use of a hook I believe.

See the above comment on the reason for the new module, however I'm
not sure if there is a better pulseaudio method than a hook to signal
this sort of event between modules.

> While I am at it, I am not even sure you need a thread for this with
> all the event loops, iochannels and stuff that PulseAudio/glib
> provide.

Ok, I can update to use glib event loop and iochannels.

> We may also want to switch the card profiles even if the card isn't
> supported by UCM.
>

I agree, although this is something that can be done after UCM jack
support is working.

> Last, I looked at the two callbacks you implemented, looks to me that
> the insert and remove parts do the same thing?

yeap, my bad, forgot to fix this in the version that I posted.

> Shouldn't you memorize
> the current device, switch to the headphone upon jack insertion and
> restore the previous device on removal?

I'm adding some code for this.

> Maybe this needs to be linked with how PulseAudio memorizes the
> devices or Colin's device-manager?

I think this is something that would be in the next stage of UCM/Jack
integration as I have to get the basic functionality working first. I
know this is on the agenda for Liam and Colin at the ASoC conference
in May.

Thanks,
Margarita

> -Pierre
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss@mail.0pointer.de
> https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss
>
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

Reply via email to