[pulseaudio-discuss] [PATCH] reserve-monitor: Don't trigger on our own events

[David Henningsson]

On 01/12/2013 04:46 PM, Tanu Kaskinen wrote:
> On Sat, 2013-01-12 at 10:10 +0100, David Henningsson wrote:
>> On 01/11/2013 08:18 PM, Tanu Kaskinen wrote:
>>> On Fri, 2013-01-11 at 14:04 +0100, David Henningsson wrote:
>>>> This fixes a bug where pulseaudio would give up the device (due to
>>>> a request from JACK), but then immediately grab it again because
>>>> the monitor callback fired, telling that the device is now available.
>>>>
>>>> (Note: the protocol does not specify a timeout, i e if pulseaudio
>>>> is requested to give its device up but JACK does not grab the dbus name,
>>>> at what point is PulseAudio allowed to re-grab it?)
>>>>
>>>> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
>>>> ---
>>>>    src/modules/reserve-monitor.c |   30 +++++++++++++++++++-----------
>>>>    1 file changed, 19 insertions(+), 11 deletions(-)
>>>>
>>>> Will commit this to stable-3.x and master in a few days if there are
>>>> no objections.
>>>>
>>>> @Lennart, would you mind committing this to the upstream reserve.git repo as well?
>>>
>>> This seems pretty equivalent to a patch[1] that I sent earlier, with the
>>> difference that with your patch change_cb() is called also in "busy ->
>>> busy" transitions (i.e. when the bus name changes owner, and neither old
>>> or new owner is pulseaudio).
>>>
>>> [1] http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/15053
>>
>> There is the problem of busy not being correctly initialized, i e, the
>> initial value of busy does not take ourselves into account. Explicitly
>> checking is_really_busy(old) works around this issue. (Properly
>> initializing busy is a more elegant solution though.)
>
> Good point. So, there are two bugs: m->busy isn't initialized properly,
> and change_cb() is sometimes called even when m->busy doesn't change.

Well, I figured that it would be - maybe mostly theoretical here, but 
still - the case that given three apps A, B, and C with descending 
priorities, that B want to know of a change from A to C so that it could 
grab the sound card now that a lower priority app has it.

> How to proceed? If I was to decide, we'd take my patch for fixing the
> redundant change_cb() calls, and adapt your patch to fix the
> initialization (is_really_busy() should be called in rm_watch(), instead
> of using it in filter_handler() to work around the initialization bug).
> If that sounds good, do you wish to send an updated patch, or do you
> prefer me to do it?

I don't have time/priority to fix the initialization (I believe it 
requires a call to org.freedesktop's GetNameOwner function?), so if you 
do the initialization within a few days time I'm okay with that. 
Otherwise I suggest we push my patch as it is.


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