On Mon, 2013-03-18 at 14:00 +0100, David Henningsson wrote:
> On 02/20/2013 07:23 PM, Tanu Kaskinen wrote:
> > ---
> > src/modules/dbus/iface-core.c | 173 +++++++++++++++++------------------------
> > 1 file changed, 72 insertions(+), 101 deletions(-)
> >
> > diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c
> > index a9716bc..4621726 100644
> > --- a/src/modules/dbus/iface-core.c
> > +++ b/src/modules/dbus/iface-core.c
> > @@ -108,9 +108,8 @@ struct pa_dbusiface_core {
> > pa_hashmap *modules;
> > pa_hashmap *clients;
> >
> > - pa_sink *fallback_sink;
> > - pa_source *fallback_source;
> > -
> > + pa_hook_slot *default_sink_changed_slot;
> > + pa_hook_slot *default_source_changed_slot;
> > pa_hook_slot *sink_put_slot;
> > pa_hook_slot *sink_unlink_slot;
> > pa_hook_slot *source_put_slot;
> > @@ -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.
> E g, the native protocol (e g pactl stat) uses
> pa_namereg_get_default_sink instead of the direct pointer.
>
> As for the other dbus patches (and "core: Add hooks for default device
> changes"), I looked them briefly through and saw nothing to remark on.
> With the disclaimer that I don't know the dbus stuff very well, but I'd
> just say it's a very welcome fix, if it fixes the occasional crashes on
> device unplug.
OK, if you ack my explanation above, I'll push patches 1-4 (Arun wants
to check patch 5 before I push it). Thanks for the review!
--
Tanu
