[Spice-devel] [announce] alpha glib/gtk client library + app.

[Gerd Hoffmann]

   Hi,

> I had a quick look at this. It seems this is a re-write of a client
> implementation (based on the existing code i guess).

Yes, basically all C++ code.

> I don't necessarily
> disagree with this (as the current code isn't exactly well suited for
> librarification), but this is a pretty big chunk of work that will take
> a while to stabilize.

Indeed.

> Also, unless we completely replace the old client
> implementation we will have to maintain two independent client
> implementations.  I guess if we make the gtk+ one work on windows too
> then we can drop the old one.

Probably a good idea long-term, but I expect it will take a while to get 
the gtk client to the point where it can fully replace the old one.

> If this is the plan we need to ensure that
> the APIs and implementations are not tied to tightly to unix.

I have some X11 MIT-SHM bits in there for performance reasons, that 
needs to be #ifdef'ed and have a alternative (generic cairo/pixman 
based?) implementation for !x11 platforms.  Other that that there should 
be nothing major. glib helps alot here.

> A few random API comments:
>
> "spice_watch", "spice_msg_out" etc is a pretty uncommon capitalization
> for Gtk+/GObject APIs. Generally all (public) types are CamelCase.

spice_msg_out I want keep internal.  Right now it isn't but I want 
figure a way to change that.

Need to look at spice_watch again, not sure how much sense this makes, 
the original idea was to allow different os-specific implementations 
there.  But when we require glib anyway we can just depend on the glib 
main loop unconditionally.

> spice_watch_put =>  spice_watch_free is a more typical Gtk+ naming
> convention (or possibly _destroy). Or _ref/_unref for refcounted types.

Will make that _ref/_unref, they are reference-counted.

> Also, i don't like the audio integration, as it exposes the backend
> implementation (at least by name) in the API. If we have multiple sound
> implementations then each client must have code to integrate with each
> one. So, I'd rather have a public SpiceAudioSink type that has different
> implementations under the hood (which one gets picked might be solely a
> build-time thing, or we could have an env var to choose between them).

Hmm.  The idea was to allow for SpicePulse and SpiceAlsa and probably 
something else on windows and macos.  There isn't much interfacing 
needed, the only thing the client app must do is creating a object 
instance and hand it over a SpiceSesson object.  So having a convenience 
helper function which picks one implementation should be easy to do.

> Things like this is a bit uncommon/lowlevel in a gtk+ api (and, binds
> badly to other languages):
> int spice_session_get_channels(SpiceSession *session, SpiceChannel
> **channels, int max);
> A more typical Gtk+ style call is:
> SpiceChannel ** spice_session_get_channels(SpiceSession *session);
> returning a NULL-terminated array of items. Or possibly a GList *.

Should be easily fixable.

cheers,
   Gerd