[pulseaudio-discuss] RFC: PulseAudio's D-Bus interface

[Tanu Kaskinen]

ti, 2009-06-30 kello 22:23 +0200, Lennart Poettering kirjoitti:
> On Mon, 29.06.09 02:12, Tanu Kaskinen (tanuk at iki.fi) wrote:
> > I wish feedback especially to the "open questions" listed on the wiki
> > page, and especially from Lennart. Otherwise, if anyone has time, more
> > thorough reviews of the API would be very welcome! It's a wiki, so
> > feel free to fix typos and other obvious mistakes yourself.
> > 
> > [1] http://pulseaudio.org/wiki/DBusInterface
> 
> Here are a few comments, after reading through this:
> 
> I think using org.PulseAudio as iface prefix is more what people would
> expect, instead of org.pulseaudio.

Ok. I would expect the prefix to mimic the form you normally see in urls
(ie. all lowercase), but maybe I'm in a minority.

> Why is org.pulseaudio.ServerLookup1.GetAddress a method? Shouldn't
> this be a property, too?

Yes, maybe it should. The rationale behind the decision to make it a
method was the fact that calling it causes PA to load client.conf from
disk. That felt somehow un-propertey.

> Also, we have X11 root window properties for finding the server, we
> should have that for the D-Bus server too, probably.
> 
> Hmm, I am not really convinced that we need to split up the pa starter
> stuff from normal daemon. I'd rather see a logic like this:
> 
> 1) If $PULSE_DBUS_SERVER is set, connect to it, end of story
> 
> 2) If X11 root window property PULSE_DBUS_SERVER is set, connect to it, end of story 
> 
> 3) If we can connect to /var/lib/pulse/dbus-socket with uuid
>    /var/lib/pulse/dbus-uuid, use that, end of story
> 
> 4) If autospawning is enabled run org.PulseAudio.Server.Activate() or
>    so on the user/session bus, which might cause PA to started up via
>    bus activation and returns the bus adddress, connect
>    to it, end of story
> 
> 5) Similar as #4, but on the system bus.
> 
> A running PA instance would always take up org.PulseAudio.Server on
> the user/session bus when run without --system and on the system bus
> otherwise. It would always return its own address on Activate.
> 
> I don't think we should start "half a daemon" as you seem to suggest,
> i.e. something that would just process the GetAddress request but
> otherwise do nothing. I think X11 properties are a much more suitable
> way to discover remote PA servers.

I'm not convinced yet regarding remote server discovery. "Half a daemon"
reads the configuration from client.conf, so we can instruct users to
just edit one variable in that file and disable the local daemon in
daemon.conf. Now, compare that with X11 properties. I don't know how to
set an X11 property. I'm sure it's just one simple command, so user
instructions wouldn't be any more complex... unless the property isn't
persistent. I would guess X11 properties aren't persistent.

What if the user doesn't have X? Surely X11 properties won't work in
such situation?

> All PA strings are supposed to be UTF-8. Some even ASCII only. Hence
> using binary strings (i.e. arrays of bytes) is always wrong. (only
> exception: properties)

I guess this is related to the question about sample file names?

> Yes, client/stream/device descriptions should be stored only in the
> proplist, we expose them in the structs only for compat reasons.
> 
> Hmm, what is InterfaceRevision good for? You version the ifaces
> anyway, s this could be dropped.

Assume we add a new method somewhere. We bump the version. Now old
clients can't connect anymore unless we maintain the old version in
parallel.

Oh, but in your D-Bus versioning guide (very helpful, btw) you say that
the version should be bumped only when breaking backwards compatibility.
Now the new clients using the new functionality have to check if the new
stuff is available by calling the new method and handling
org.freedesktop.DBus.Error.UnknownMethod if it doesn't exist. But what
if the client doesn't want to call the method in advance (due to the
method side effects) and on the other hand wants to abort early if the
functionality isn't available? I don't see other possibility than doing
introspection and that way check for the method presence, or reading
InterfaceRevision. The latter is probably simpler.

> Not sure if I am happy if we'd expose the sink inputs as playback
> streams here. I think this would be a bit confusing.

Yes it would, at least to those who already familiar with the existing
terminology. But just a bit. I think that for newbies this naming is
more intuitive. But if you think otherwise, that's fine. Do you have
name ideas for the common Stream interface?

> I'd vote for not exposing
> LoadSampleFromFile/AddLazySample/AddLazySamplesFromDirectory for now,
> since the paths passed are server paths. I'd probably even hide the
> fact that there are things like "lazy" samples completely.

Hmm... Telling the server to load the sample from a file is much easier
than to load the file yourself. But yes, this creates an asymmetry
between remote and local clients. I'll drop all three for now.

> Shouldn't org.pulseaudio.Core1.Card have a property to find the
> sinks/sources of a card?

Good idea.

> I think it would be nice if boolean properties would be prefix with
> "Is"/"Has", i.e. as in "IsHardwareDevice" instead of
> "HardwareDevice". And "HasFlatVolume" instead of "FlatVolume". 

Ok.

> We'll probably add flat vols and flat mute for sources, too. I'd thus
> suggest adding this to the Device iface weven if for now it will
> always be FALSE there for sources.

Ok. A related note: flat source volumes imply record stream volumes,
which makes the PlaybackStream interface redundant after the properties
move to the Stream interface (moving the properties was already
suggested by Pierre Bossart, I'll send a reply to that mail after I'm
done with this).

> I'd probably go for an uint32_t for states, not uint8_t. Because
> uint32_t are more "native". Dunno. 

Can do.

> Shouldn't the "Proplist" properties be named "PropertyList"?

Would be better, yes.

> I'd leave signal suppression to D-Bus and drop the ListenForSignal()
> calls. 

I thought that was a bus feature. Does DBusServer really do the
filtering behind the scenes?

> Hmm, no sure if I am happy with the whole Self iface/object.
> 
> I'd rather see this as an additional interface that can be implemented
> by a client object but which would fail with permission errors when
> the client tries modify another client's data. And then there should
> be a function on the core object to find your own client object.

I'll do that.

> Otherwise this looks pretty nice. Good stuff!

Thanks, and thank you for taking the time to review!

-- 
Tanu