>> 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) >> > > 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 - 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. 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. We may also want to switch the card profiles even if the card isn't supported by UCM. Last, I looked at the two callbacks you implemented, looks to me that the insert and remove parts do the same thing? Shouldn't you memorize the current device, switch to the headphone upon jack insertion and restore the previous device on removal? Maybe this needs to be linked with how PulseAudio memorizes the devices or Colin's device-manager? -Pierre
