[pulseaudio-discuss] [PATCH v2 2/9] dbus: Track the default sink and source with hooks

[David Henningsson]

On 03/18/2013 04:15 PM, Tanu Kaskinen wrote:
> On Mon, 2013-03-18 at 15:44 +0100, David Henningsson wrote:
>> On 03/18/2013 03:39 PM, Tanu Kaskinen wrote:
>>> On Mon, 2013-03-18 at 14:00 +0100, David Henningsson wrote:
>>>> On 02/20/2013 07:23 PM, Tanu Kaskinen wrote:
>>>>> @@ -677,13 +676,12 @@ static void handle_get_fallback_sink(DBusConnection *conn, DBusMessage *msg, voi
>>>>>         pa_assert(msg);
>>>>>         pa_assert(c);
>>>>>
>>>>> -    if (!c->fallback_sink) {
>>>>> -        pa_dbus_send_error(conn, msg, PA_DBUS_ERROR_NO_SUCH_PROPERTY,
>>>>> -                           "There are no sinks, and therefore no fallback sink either.");
>>>>> +    if (!c->core->default_sink) {
>>>>> +        pa_dbus_send_error(conn, msg, PA_DBUS_ERROR_NO_SUCH_PROPERTY, "No fallback sink set.");
>>>>>             return;
>>>>>         }
>>>>
>>>> Is there a reason for using core->default_sink instead of
>>>> pa_namereg_get_default_sink?
>>>
>>> There's this reason: the property change signals should match what
>>> getting the property would return, at least in my opinion. If we'd call
>>> pa_namereg_get_default_sink() here, then the signals would indicate that
>>> no fallback has been set, but getting the fallback would indicate that
>>> there's some fallback set.
>>>
>>> It would definitely be good to have a comment about this here.
>>
>> What about caching the result of pa_namereg_get_default_sink in the
>> beginning, and only send out a dbus signal if the result of the function
>> has actually changed?
>
> I guess you mean this: the D-Bus code has its own default sink variable,
> let's call it dbus.default_sink here. dbus.default_sink is always the
> same as core.default_sink when core.default_sink is non-NULL.
>
> When core.default_sink changes to NULL, the D-Bus code calls
> pa_namereg_get_default_sink() and saves the result to dbus.default_sink.
> If that causes a change to dbus.default_sink, a signal is sent.
>
> When someone queries the property value and core.default_sink is NULL,
> pa_namereg_get_default_sink() is called again, the result is stored in
> dbus.default_sink and the if that causes a change to dbus.default_sink,
> a signal is sent. Then the result is sent as the reply to the original
> query.
>
> This would indeed achieve the desired consistency between the signals
> and the property query results. I'll implement this, unless this
> question causes you to change your mind:
>
> Does it really make sense to return a fallback sink when queried, if
> there's no signal sent when the "ad-hoc default" changes? This applies
> to the native protocol as well. Let's use pavucontrol as an example. It
> shows the current default sink. If core.default_sink is NULL, the
> default that pavucontrol shows is the highest-priority sink at the time
> of the initial pa_context_get_server_info() call. If a higher-priority
> sink appears, pavucontrol won't get a signal about the change in the
> "ad-hoc default" sink, so it will end up displaying incorrect
> information. Would it be better to not show any sink as being the
> default, when the default hasn't been explicitly set?

Ok, fair point. It sound like you need to update (and possibly signal) 
dbus.default_sink when sinks are added and removed too, then? Are there 
other events that can cause the result of pa_namereg_get_default_sink to 
change, so that pa_namereg_get_default_sink() and dbus.default_sink get 
out of sync?


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