[pulseaudio-discuss] Jack detection - from the other side

[David Henningsson]

On 2011-07-05 19:26, Tanu Kaskinen wrote:
> On Mon, 2011-07-04 at 16:32 +0200, David Henningsson wrote:
>> It's time to get a little momentum on the Jack detection again. I hope
>> to be able to work with it this week.
>>
>> As you might know, my udev patch turned controversial [1] [2], so I
>> decided to attack the problem from another angle. Here's a preliminary
>> idea of how the API could look like from the "volume control UI"
>> perspective. As Tanuk originally suggested, every port now has an
>> "available" property which can be yes/no/unknown. Also imagine the same
>> thing implemented for sources as now being presented for sinks.
>
> I would much prefer if patches would be sent with git send-email unless
> they are perfect, because commenting is much easier with inline
> patches...

Ok, I'll see if I can set it up to next time.

>> diff --git a/src/pulse/introspect.h b/src/pulse/introspect.h
>> index 196d44d..6f62497 100644
>> --- a/src/pulse/introspect.h
>> +++ b/src/pulse/introspect.h
>> @@ -193,6 +193,10 @@
>>
>>   PA_C_DECL_BEGIN
>>
>> +#define PA_PORT_AVAILABLE_UNKNOWN (0)   /**<  This port does not support jack detection \since MERGE_OF_JACK_DETECTION */
>> +#define PA_PORT_AVAILABLE_NO (1)        /**<  This port is not available, likely because the jack is not plugged in. \since MERGE_OF_JACK_DETECTION */
>> +#define PA_PORT_AVAILABLE_YES (2)       /**<  This port is available, likely because the jack is plugged in. \since MERGE_OF_JACK_DETECTION */
>
> Should these be in pulse/def.h? Glancing around quickly it looks like
> there are no other constants defined in introspect.h. I'd also introduce
> an enum type for these.

Ok, I can change that.

>
>> +    int switch_on_avail;                /**<  Boolean indicating that when "available" changes to PA_PORT_AVAILABLE_YES, streams should move to this sink. \since MERGE_OF_JACK_DETECTION */
>
> Why is this needed? Automatic moving of streams on jack events is
> policy, and thus it's something that the core shouldn't be involved in,
> IMO.

I understand your thoughts, but if you had such a policy module (instead 
of having it in the core), how would you easily expose the same thing to 
a volume control UI? Could you give an example?

>> +/** Change the "switch_on_available" attribute of a port of a sink. \since MERGE_OF_JACK_DETECTION */
>> +pa_operation* pa_context_set_sink_port_switch_on_avail_by_index(pa_context *c, uint32_t idx, const char*port, int switch_on_available, pa_context_success_cb_t cb, void *userdata);
>
> Ah, this is what it was needed for. I still have my opinion - this
> function doesn't belong to the core API. I think we should have a module
> that implements the move-on-jack-insert policy and provides the client
> API if that's needed.
>
>> +int pa_sink_set_port_available(pa_sink *s, const char *name, int available);
>
> I guess this is left without implementation on purpose?

Well, by announcing the interface first I can avoid wasting work on the 
implementation should the interface be disputed.

> The function is put under the callable-by-everyone section. I think it
> would be logical to put it under the
> only-callable-by-sink-implementation section, but maybe there are
> reasons why that can't be done (like the jack detection being
> implemented outside the alsa modules).

That was my thought.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic