On Mon, 29.06.09 02:12, Tanu Kaskinen (tanuk at iki.fi) wrote: > Hello everybody! heya! > > If you are interested in controlling PulseAudio programmatically, you > may be delighted about the fact that PulseAudio is going to get a > D-Bus interface. I have finished the first draft of the API [1], and > now would be the perfect time to suggest improvements. Implementation > work should start tomorrow (the server lookup part is already done), > so the sooner the comments come in the less I need to fix things :) > > The API documentation is fairly complete, but assumes quite broad > knowledge of PulseAudio features. I think documenting all the main > PulseAudio concepts in the wiki would be a good project for somebody > (me?), which the API docs could then refer to. Documentation is always very welcome, of course ;-) > 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. Why is org.pulseaudio.ServerLookup1.GetAddress a method? Shouldn't this be a property, too? 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 agree that exposing the server cookie is a bad idea. In fact, having implemented that at all is probably a bad idea. Dunno. 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) 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. 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. 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. Shouldn't org.pulseaudio.Core1.Card have a property to find the sinks/sources of a card? 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". 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. I'd probably go for an uint32_t for states, not uint8_t. Because uint32_t are more "native". Dunno. Shouldn't the "Proplist" properties be named "PropertyList"? I'd leave signal suppression to D-Bus and drop the ListenForSignal() calls. 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. Otherwise this looks pretty nice. Good stuff! Lennart -- Lennart Poettering Red Hat, Inc. lennart [at] poettering [dot] net http://0pointer.net/lennart/ GnuPG 0x1A015CC4
