Re: [Spice-devel] [spice-gtk PATCH v7 2/4] audio: spice-pulse implement async volume-info
Hi, thanks for the review On Wed, Apr 15, 2015 at 07:52:46PM +0200, Marc-André Lureau wrote: On Wed, Apr 15, 2015 at 6:39 PM, Victor Toso victort...@redhat.com wrote: In case of volume-sync between client and guest, we request volume-info from the availables streams and if the stream is not available we rely on ext-stream-restore. By using ext-stream-restore we can get the last stream data of the application that is stored by PulseAudio. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1012868 --- gtk/spice-pulse.c | 449 -- 1 file changed, 440 insertions(+), 9 deletions(-) diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c index c583032..9b58ec0 100644 --- a/gtk/spice-pulse.c +++ b/gtk/spice-pulse.c @@ -25,18 +25,34 @@ #include pulse/glib-mainloop.h #include pulse/pulseaudio.h +#include pulse/ext-stream-restore.h #define SPICE_PULSE_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_PULSE, SpicePulsePrivate)) +struct async_task { +SpicePulse *pulse; +SpiceMainChannel *main_channel; +GSimpleAsyncResult *res; +GAsyncReadyCallbackcallback; +gpointer user_data; +gboolean is_playback; +pa_operation *pa_op; +gulong cancel_id; +GCancellable *cancellable; +}; + struct stream { -pa_sample_spec spec; -pa_stream *stream; -int state; -pa_operation*uncork_op; -pa_operation*cork_op; -gbooleanstarted; -guint num_underflow; +pa_sample_spec spec; +pa_stream *stream; +intstate; +pa_operation *uncork_op; +pa_operation *cork_op; +gboolean started; +guint num_underflow; +gboolean info_updated; +gchar *name; +pa_ext_stream_restore_info info; }; struct _SpicePulsePrivate { @@ -50,6 +66,8 @@ struct _SpicePulsePrivate { struct stream record; guint last_delay; guint target_delay; +gbooleanrestore_stream_info; +GList *results; }; G_DEFINE_TYPE(SpicePulse, spice_pulse, SPICE_TYPE_AUDIO) @@ -77,6 +95,18 @@ static const char *context_state_names[] = { static void stream_stop(SpicePulse *pulse, struct stream *s); static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel); static void channel_weak_notified(gpointer data, GObject *where_the_object_was); +static void spice_pulse_get_playback_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_playback_volume_info_finish(SpiceAudio *audio, GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void spice_pulse_get_record_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_record_volume_info_finish(SpiceAudio *audio,GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void stream_restore_read_cb(pa_context *context, +const pa_ext_stream_restore_info *info, int eol, void *userdata); +static void spice_pulse_complete_async_task(struct async_task *task, const gchar *err_msg); +static void spice_pulse_complete_all_async_tasks(SpicePulse *pulse, const gchar *err_msg); static void spice_pulse_finalize(GObject *obj) { @@ -118,6 +148,12 @@ static void spice_pulse_dispose(GObject *obj) pa_operation_unref(p-record.cork_op); p-record.cork_op = NULL; +if (p-results != NULL) +spice_pulse_complete_all_async_tasks(pulse, PulseAudio is being dispose); + +g_free(p-playback.name); +g_free(p-record.name); + if (p-pchannel) g_object_weak_unref(G_OBJECT(p-pchannel), channel_weak_notified, pulse); p-pchannel = NULL; @@ -140,6 +176,10 @@ static void spice_pulse_class_init(SpicePulseClass *klass) SpiceAudioClass *audio_class = SPICE_AUDIO_CLASS(klass); audio_class-connect_channel = connect_channel; +audio_class-get_playback_volume_info_async = spice_pulse_get_playback_volume_info_async; +audio_class-get_playback_volume_info_finish = spice_pulse_get_playback_volume_info_finish; +
Re: [Spice-devel] [spice-gtk] Add spec files
Hi, - Original Message - They can be used to build spice-gtk / mingw-spice-gtk RPM packages. These spec files are based off the .spec used in Fedora Why is it needed? I dislike having to maintain .spec files in several places. Furthermore, I don't get why we should have .spec file upstream and not of other distro. --- configure.ac | 2 + data/Makefile.am | 8 ++ data/mingw-spice-gtk.spec.in | 279 +++ data/spice-gtk.spec.in | 256 +++ 4 files changed, 545 insertions(+) create mode 100644 data/mingw-spice-gtk.spec.in create mode 100644 data/spice-gtk.spec.in diff --git a/configure.ac b/configure.ac index cf5a039..c6c82ea 100644 --- a/configure.ac +++ b/configure.ac @@ -745,6 +745,8 @@ spice-client-gtk-3.0.pc spice-controller.pc data/Makefile data/spicy.nsis +data/spice-gtk.spec +data/mingw-spice-gtk.spec po/Makefile.in gtk/Makefile gtk/spice-version.h diff --git a/data/Makefile.am b/data/Makefile.am index a289c23..82f4ba1 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -1,13 +1,21 @@ NULL= EXTRA_DIST = \ + mingw-spice-gtk.spec\ spicy.nsis \ + spice-gtk.spec \ spice-protocol.vapi \ gtkrc \ $(desktop_in_files) \ org.spice-space.lowlevelusbaccess.policy \ $(NULL) +DISTCLEAN_FILES =\ + mingw-spice-gtk.spec\ + spice-gtk.spec \ + $(NULL) + + vapidir = $(VAPIDIR) vapi_DATA = spice-protocol.vapi diff --git a/data/mingw-spice-gtk.spec.in b/data/mingw-spice-gtk.spec.in new file mode 100644 index 000..c27e91e --- /dev/null +++ b/data/mingw-spice-gtk.spec.in @@ -0,0 +1,279 @@ +%{?mingw_package_header} + +Name: mingw-spice-gtk +Version:@VERSION@ +Release:1%{?dist} +Summary:A GTK+ widget for SPICE clients + +License:LGPLv2+ +URL:http://spice-space.org/page/Spice-Gtk +Source0: http://www.spice-space.org/download/gtk/spice-gtk-%{version}%{?_version_suffix}.tar.bz2 + +BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) +BuildArch: noarch + +BuildRequires: mingw32-filesystem = 95 +BuildRequires: mingw64-filesystem = 95 +BuildRequires: mingw32-gcc +BuildRequires: mingw64-gcc +BuildRequires: mingw32-binutils +BuildRequires: mingw64-binutils +BuildRequires: glib2-devel + +BuildRequires: mingw32-gtk2 = 2.14 +BuildRequires: mingw64-gtk2 = 2.14 +BuildRequires: mingw32-gtk3 = 2.91.3 +BuildRequires: mingw64-gtk3 = 2.91.3 +BuildRequires: mingw32-pixman +BuildRequires: mingw64-pixman +BuildRequires: mingw32-openssl +BuildRequires: mingw64-openssl +BuildRequires: mingw32-libjpeg-turbo +BuildRequires: mingw64-libjpeg-turbo +BuildRequires: mingw32-celt051 +BuildRequires: mingw64-celt051 +BuildRequires: mingw32-zlib +BuildRequires: mingw64-zlib +BuildRequires: mingw32-gstreamer1 +BuildRequires: mingw64-gstreamer1 +BuildRequires: mingw32-gstreamer1-plugins-base +BuildRequires: mingw64-gstreamer1-plugins-base +BuildRequires: mingw32-usbredir +BuildRequires: mingw64-usbredir +BuildRequires: mingw32-libusbx +BuildRequires: mingw64-libusbx +BuildRequires: mingw32-opus +BuildRequires: mingw64-opus +# Hack because of bz #613466 +BuildRequires: intltool +BuildRequires: libtool + +%description +Client libraries for SPICE desktop servers. + + +# Mingw32 +%package -n mingw32-spice-gtk +Summary: %{summary} +Requires: mingw32-spice-glib = %{version}-%{release} +Requires: mingw32-gtk2 +Requires: pkgconfig + +%description -n mingw32-spice-gtk +Gtk+2 client libraries for SPICE desktop servers. + +%package -n mingw32-spice-gtk3 +Summary: %{summary} +Requires: mingw32-spice-glib = %{version}-%{release} +Requires: mingw32-gtk3 +Requires: pkgconfig + +%description -n mingw32-spice-gtk3 +Gtk+3 client libraries for SPICE desktop servers. + +%package -n mingw32-spice-glib +Summary: GLib-based library to connect to SPICE servers +Requires: pkgconfig +Requires: mingw32-glib2 +Requires: mingw32-spice-protocol + +%description -n mingw32-spice-glib +A SPICE client library using GLib2. + +%package -n mingw32-spice-gtk-static +Summary: %{summary} +Requires: mingw32-spice-gtk = %{version}-%{release} + +%description -n mingw32-spice-gtk-static +Gtk+ client static libraries for SPICE desktop servers. + +# Mingw64 +%package -n mingw64-spice-gtk +Summary: %{summary} +Requires: mingw64-spice-glib = %{version}-%{release} +Requires: mingw64-gtk2 +Requires: pkgconfig + +%description -n mingw64-spice-gtk +Gtk+2 client libraries for SPICE desktop servers. + +%package -n mingw64-spice-gtk3
[Spice-devel] [spice-gtk] Add spec files
They can be used to build spice-gtk / mingw-spice-gtk RPM packages. These spec files are based off the .spec used in Fedora --- configure.ac | 2 + data/Makefile.am | 8 ++ data/mingw-spice-gtk.spec.in | 279 +++ data/spice-gtk.spec.in | 256 +++ 4 files changed, 545 insertions(+) create mode 100644 data/mingw-spice-gtk.spec.in create mode 100644 data/spice-gtk.spec.in diff --git a/configure.ac b/configure.ac index cf5a039..c6c82ea 100644 --- a/configure.ac +++ b/configure.ac @@ -745,6 +745,8 @@ spice-client-gtk-3.0.pc spice-controller.pc data/Makefile data/spicy.nsis +data/spice-gtk.spec +data/mingw-spice-gtk.spec po/Makefile.in gtk/Makefile gtk/spice-version.h diff --git a/data/Makefile.am b/data/Makefile.am index a289c23..82f4ba1 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -1,13 +1,21 @@ NULL= EXTRA_DIST = \ + mingw-spice-gtk.spec\ spicy.nsis \ + spice-gtk.spec \ spice-protocol.vapi \ gtkrc \ $(desktop_in_files) \ org.spice-space.lowlevelusbaccess.policy \ $(NULL) +DISTCLEAN_FILES = \ + mingw-spice-gtk.spec\ + spice-gtk.spec \ + $(NULL) + + vapidir = $(VAPIDIR) vapi_DATA = spice-protocol.vapi diff --git a/data/mingw-spice-gtk.spec.in b/data/mingw-spice-gtk.spec.in new file mode 100644 index 000..c27e91e --- /dev/null +++ b/data/mingw-spice-gtk.spec.in @@ -0,0 +1,279 @@ +%{?mingw_package_header} + +Name: mingw-spice-gtk +Version:@VERSION@ +Release:1%{?dist} +Summary:A GTK+ widget for SPICE clients + +License:LGPLv2+ +URL:http://spice-space.org/page/Spice-Gtk +Source0: http://www.spice-space.org/download/gtk/spice-gtk-%{version}%{?_version_suffix}.tar.bz2 + +BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) +BuildArch: noarch + +BuildRequires: mingw32-filesystem = 95 +BuildRequires: mingw64-filesystem = 95 +BuildRequires: mingw32-gcc +BuildRequires: mingw64-gcc +BuildRequires: mingw32-binutils +BuildRequires: mingw64-binutils +BuildRequires: glib2-devel + +BuildRequires: mingw32-gtk2 = 2.14 +BuildRequires: mingw64-gtk2 = 2.14 +BuildRequires: mingw32-gtk3 = 2.91.3 +BuildRequires: mingw64-gtk3 = 2.91.3 +BuildRequires: mingw32-pixman +BuildRequires: mingw64-pixman +BuildRequires: mingw32-openssl +BuildRequires: mingw64-openssl +BuildRequires: mingw32-libjpeg-turbo +BuildRequires: mingw64-libjpeg-turbo +BuildRequires: mingw32-celt051 +BuildRequires: mingw64-celt051 +BuildRequires: mingw32-zlib +BuildRequires: mingw64-zlib +BuildRequires: mingw32-gstreamer1 +BuildRequires: mingw64-gstreamer1 +BuildRequires: mingw32-gstreamer1-plugins-base +BuildRequires: mingw64-gstreamer1-plugins-base +BuildRequires: mingw32-usbredir +BuildRequires: mingw64-usbredir +BuildRequires: mingw32-libusbx +BuildRequires: mingw64-libusbx +BuildRequires: mingw32-opus +BuildRequires: mingw64-opus +# Hack because of bz #613466 +BuildRequires: intltool +BuildRequires: libtool + +%description +Client libraries for SPICE desktop servers. + + +# Mingw32 +%package -n mingw32-spice-gtk +Summary: %{summary} +Requires: mingw32-spice-glib = %{version}-%{release} +Requires: mingw32-gtk2 +Requires: pkgconfig + +%description -n mingw32-spice-gtk +Gtk+2 client libraries for SPICE desktop servers. + +%package -n mingw32-spice-gtk3 +Summary: %{summary} +Requires: mingw32-spice-glib = %{version}-%{release} +Requires: mingw32-gtk3 +Requires: pkgconfig + +%description -n mingw32-spice-gtk3 +Gtk+3 client libraries for SPICE desktop servers. + +%package -n mingw32-spice-glib +Summary: GLib-based library to connect to SPICE servers +Requires: pkgconfig +Requires: mingw32-glib2 +Requires: mingw32-spice-protocol + +%description -n mingw32-spice-glib +A SPICE client library using GLib2. + +%package -n mingw32-spice-gtk-static +Summary: %{summary} +Requires: mingw32-spice-gtk = %{version}-%{release} + +%description -n mingw32-spice-gtk-static +Gtk+ client static libraries for SPICE desktop servers. + +# Mingw64 +%package -n mingw64-spice-gtk +Summary: %{summary} +Requires: mingw64-spice-glib = %{version}-%{release} +Requires: mingw64-gtk2 +Requires: pkgconfig + +%description -n mingw64-spice-gtk +Gtk+2 client libraries for SPICE desktop servers. + +%package -n mingw64-spice-gtk3 +Summary: %{summary} +Requires: mingw64-spice-glib = %{version}-%{release} +Requires: mingw64-gtk3 +Requires: pkgconfig + +%description -n mingw64-spice-gtk3 +Gtk+3 client libraries for SPICE desktop servers. + +%package -n mingw64-spice-glib +Summary: GLib-based library to connect to SPICE servers +Requires: pkgconfig
Re: [Spice-devel] [spice-gtk] Add spec files
On Thu, Apr 16, 2015 at 06:11:40AM -0400, Marc-André Lureau wrote: Hi, - Original Message - They can be used to build spice-gtk / mingw-spice-gtk RPM packages. These spec files are based off the .spec used in Fedora Why is it needed? I dislike having to maintain .spec files in several places. Furthermore, I don't get why we should have .spec file upstream and not of other distro. I followed Fidencio's suggestion, it would have been nice to voice your disagreement before I spent a few minutes doing the work ;) Christophe pgpEu5wHh4r4G.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Add spec files
On Thu, Apr 16, 2015 at 12:30:27PM +0200, Marc-André Lureau wrote: I don't think there are valid reason to put fedora-specific spec file in upstream. You can easily build a rpm with fedpkg. nack We've done this for years with all libvirt related packages and it has proved pretty userful for both end users and maintainers alike. For example, when upstream makes a change that would impact the RPM packaging, it is usually quicker easier for the person making that change to do the RPM spec file change upstream at the same time, than for the downstream maintainer to waste time trying to figure out what the change was. By having an upstream RPM spec that is always buildable, it makes it easier for users to test out new releases that are not yet in the official distro repos, because again they don't have to figure out how to fix the RPM spec to work with latest code changes. Having a single canonical RPM spec that works across all current versions of RHEL Fedora has also reduced the workload on the Fedora maintainers too. The included RPM spec also serves as a helpful guide for other distros figuring out how to package libvirt in non-RPM formats So yes, it might seem odd to include a downstream specific RPM spec in upstream, but in practice it has proved pretty useful over the 9+ years libvirt related apps have been around Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Add spec files
- Original Message - From: Marc-André Lureau marcandre.lur...@gmail.com To: Christophe Fergeau cferg...@redhat.com Cc: spice-de...@freedesktop.org Sent: Thursday, April 16, 2015 12:30:27 PM Subject: Re: [Spice-devel] [spice-gtk] Add spec files I don't think there are valid reason to put fedora-specific spec file in upstream. You can easily build a rpm with fedpkg. nack I disagree. We have it for virt-viewer and another spice projects. I'd prefer to have it here. It doesn't harm ... On Thu, Apr 16, 2015 at 12:17 PM, Christophe Fergeau cferg...@redhat.com wrote: On Thu, Apr 16, 2015 at 06:11:40AM -0400, Marc-André Lureau wrote: Hi, - Original Message - They can be used to build spice-gtk / mingw-spice-gtk RPM packages. These spec files are based off the .spec used in Fedora Why is it needed? I dislike having to maintain .spec files in several places. Furthermore, I don't get why we should have .spec file upstream and not of other distro. I followed Fidencio's suggestion, it would have been nice to voice your disagreement before I spent a few minutes doing the work ;) Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel Best Regards, -- Fabiano Fidêncio ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk] Fix GSocketAddress leak in proxy_lookup_ready()
g_proxy_address_new() returns a new GProxyAddress, so we must unref it when no longer needed. This fixes: ==6481== 234 (48 direct, 186 indirect) bytes in 1 blocks are definitely lost in loss record 10,062 of 10, ==6481==at 0x31FF230A58: g_type_create_instance (gtype.c:1849) ==6481==by 0x31FF21501A: g_object_new_internal (gobject.c:1774) ==6481==by 0x31FF216EB4: g_object_new_valist (gobject.c:2033) ==6481==by 0x31FF217220: g_object_new (gobject.c:1617) ==6481==by 0x3D4386F33A: g_proxy_address_new (gproxyaddress.c:325) ==6481==by 0x5717440: proxy_lookup_ready (spice-session.c:2011) ==6481==by 0x3D43885082: g_task_return_now (gtask.c:1088) ==6481==by 0x3D438850B8: complete_in_idle_cb (gtask.c:1102) ==6481==by 0x31FEE4A0B9: g_main_dispatch (gmain.c:3122) ==6481==by 0x31FEE4A0B9: g_main_context_dispatch (gmain.c:3737) ==6481==by 0x31FEE4A44F: g_main_context_iterate.isra.29 (gmain.c:3808) ==6481==by 0x31FEE4A771: g_main_loop_run (gmain.c:4002) ==6481==by 0x363AC06CC4: gtk_main (in /usr/lib64/libgtk-3.so.0.1600.1) --- gtk/spice-session.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gtk/spice-session.c b/gtk/spice-session.c index 1a68d7d..020a70e 100644 --- a/gtk/spice-session.c +++ b/gtk/spice-session.c @@ -2020,6 +2020,7 @@ static void proxy_lookup_ready(GObject *source_object, GAsyncResult *result, open_host_connectable_connect(open_host, G_SOCKET_CONNECTABLE(address)); g_resolver_free_addresses(addresses); +g_object_unref(address); } /* main context */ -- 2.3.5 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk PATCH v8 0/4] volume-info: spice-pulse
spice-pulse: - track later pa_operation related to ext-stream-restore that only starts after context is in READY state; - avoid triggering the above operation more then once; - cancel pa_operations when complete_all_async_tasks is called. this is useful in dispose() as we may have pa_operations running; note: I'm removing the __func__ from SPICE_DEBUG in other patches as well. Victor Toso (4): audio: spice-audio with get mute and volume audio: spice-pulse implement async volume-info audio: spice-gstaudio implements async volume-info agent: sync guest audio with client values gtk/channel-main.c | 141 ++ gtk/spice-audio.h| 28 ++- gtk/spice-gstaudio.c | 191 ++- gtk/spice-pulse.c| 476 ++- gtk/spice-session-priv.h | 2 +- 5 files changed, 825 insertions(+), 13 deletions(-) -- 2.1.0 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk PATCH v8 2/4] audio: spice-pulse implement async volume-info
In case of volume-sync between client and guest, we request volume-info from the availables streams and if the stream is not available we rely on ext-stream-restore. By using ext-stream-restore we can get the last stream data of the application that is stored by PulseAudio. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1012868 --- gtk/spice-pulse.c | 476 -- 1 file changed, 467 insertions(+), 9 deletions(-) diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c index c583032..c2be1b3 100644 --- a/gtk/spice-pulse.c +++ b/gtk/spice-pulse.c @@ -25,18 +25,34 @@ #include pulse/glib-mainloop.h #include pulse/pulseaudio.h +#include pulse/ext-stream-restore.h #define SPICE_PULSE_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_PULSE, SpicePulsePrivate)) +struct async_task { +SpicePulse *pulse; +SpiceMainChannel *main_channel; +GSimpleAsyncResult *res; +GAsyncReadyCallbackcallback; +gpointer user_data; +gboolean is_playback; +pa_operation *pa_op; +gulong cancel_id; +GCancellable *cancellable; +}; + struct stream { -pa_sample_spec spec; -pa_stream *stream; -int state; -pa_operation*uncork_op; -pa_operation*cork_op; -gbooleanstarted; -guint num_underflow; +pa_sample_spec spec; +pa_stream *stream; +intstate; +pa_operation *uncork_op; +pa_operation *cork_op; +gboolean started; +guint num_underflow; +gboolean info_updated; +gchar *name; +pa_ext_stream_restore_info info; }; struct _SpicePulsePrivate { @@ -50,6 +66,8 @@ struct _SpicePulsePrivate { struct stream record; guint last_delay; guint target_delay; +struct async_task *pending_restore_task; +GList *results; }; G_DEFINE_TYPE(SpicePulse, spice_pulse, SPICE_TYPE_AUDIO) @@ -77,6 +95,18 @@ static const char *context_state_names[] = { static void stream_stop(SpicePulse *pulse, struct stream *s); static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel); static void channel_weak_notified(gpointer data, GObject *where_the_object_was); +static void spice_pulse_get_playback_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_playback_volume_info_finish(SpiceAudio *audio, GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void spice_pulse_get_record_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_record_volume_info_finish(SpiceAudio *audio,GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void stream_restore_read_cb(pa_context *context, +const pa_ext_stream_restore_info *info, int eol, void *userdata); +static void spice_pulse_complete_async_task(struct async_task *task, const gchar *err_msg); +static void spice_pulse_complete_all_async_tasks(SpicePulse *pulse, const gchar *err_msg); static void spice_pulse_finalize(GObject *obj) { @@ -118,6 +148,12 @@ static void spice_pulse_dispose(GObject *obj) pa_operation_unref(p-record.cork_op); p-record.cork_op = NULL; +if (p-results != NULL) +spice_pulse_complete_all_async_tasks(pulse, PulseAudio is being dispose); + +g_free(p-playback.name); +g_free(p-record.name); + if (p-pchannel) g_object_weak_unref(G_OBJECT(p-pchannel), channel_weak_notified, pulse); p-pchannel = NULL; @@ -140,6 +176,10 @@ static void spice_pulse_class_init(SpicePulseClass *klass) SpiceAudioClass *audio_class = SPICE_AUDIO_CLASS(klass); audio_class-connect_channel = connect_channel; +audio_class-get_playback_volume_info_async = spice_pulse_get_playback_volume_info_async; +audio_class-get_playback_volume_info_finish = spice_pulse_get_playback_volume_info_finish; +audio_class-get_record_volume_info_async = spice_pulse_get_record_volume_info_async; +audio_class-get_record_volume_info_finish = spice_pulse_get_record_volume_info_finish; gobject_class-finalize = spice_pulse_finalize; gobject_class-dispose = spice_pulse_dispose; @@ -812,18 +852,40 @@ static void context_state_callback(pa_context *c, void *userdata) if
Re: [Spice-devel] [spice-gtk PATCH v8 2/4] audio: spice-pulse implement async volume-info
Hi A few comments below. On Thu, Apr 16, 2015 at 4:42 PM, Victor Toso victort...@redhat.com wrote: In case of volume-sync between client and guest, we request volume-info from the availables streams and if the stream is not available we rely on ext-stream-restore. By using ext-stream-restore we can get the last stream data of the application that is stored by PulseAudio. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1012868 --- gtk/spice-pulse.c | 476 -- 1 file changed, 467 insertions(+), 9 deletions(-) diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c index c583032..c2be1b3 100644 --- a/gtk/spice-pulse.c +++ b/gtk/spice-pulse.c @@ -25,18 +25,34 @@ #include pulse/glib-mainloop.h #include pulse/pulseaudio.h +#include pulse/ext-stream-restore.h #define SPICE_PULSE_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_PULSE, SpicePulsePrivate)) +struct async_task { +SpicePulse *pulse; +SpiceMainChannel *main_channel; +GSimpleAsyncResult *res; +GAsyncReadyCallbackcallback; +gpointer user_data; +gboolean is_playback; +pa_operation *pa_op; +gulong cancel_id; +GCancellable *cancellable; +}; + struct stream { -pa_sample_spec spec; -pa_stream *stream; -int state; -pa_operation*uncork_op; -pa_operation*cork_op; -gbooleanstarted; -guint num_underflow; +pa_sample_spec spec; +pa_stream *stream; +intstate; +pa_operation *uncork_op; +pa_operation *cork_op; +gboolean started; +guint num_underflow; +gboolean info_updated; +gchar *name; +pa_ext_stream_restore_info info; }; struct _SpicePulsePrivate { @@ -50,6 +66,8 @@ struct _SpicePulsePrivate { struct stream record; guint last_delay; guint target_delay; +struct async_task *pending_restore_task; +GList *results; }; G_DEFINE_TYPE(SpicePulse, spice_pulse, SPICE_TYPE_AUDIO) @@ -77,6 +95,18 @@ static const char *context_state_names[] = { static void stream_stop(SpicePulse *pulse, struct stream *s); static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel); static void channel_weak_notified(gpointer data, GObject *where_the_object_was); +static void spice_pulse_get_playback_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_playback_volume_info_finish(SpiceAudio *audio, GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void spice_pulse_get_record_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_record_volume_info_finish(SpiceAudio *audio,GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void stream_restore_read_cb(pa_context *context, +const pa_ext_stream_restore_info *info, int eol, void *userdata); +static void spice_pulse_complete_async_task(struct async_task *task, const gchar *err_msg); +static void spice_pulse_complete_all_async_tasks(SpicePulse *pulse, const gchar *err_msg); static void spice_pulse_finalize(GObject *obj) { @@ -118,6 +148,12 @@ static void spice_pulse_dispose(GObject *obj) pa_operation_unref(p-record.cork_op); p-record.cork_op = NULL; +if (p-results != NULL) +spice_pulse_complete_all_async_tasks(pulse, PulseAudio is being dispose); + +g_free(p-playback.name); +g_free(p-record.name); + if (p-pchannel) g_object_weak_unref(G_OBJECT(p-pchannel), channel_weak_notified, pulse); p-pchannel = NULL; @@ -140,6 +176,10 @@ static void spice_pulse_class_init(SpicePulseClass *klass) SpiceAudioClass *audio_class = SPICE_AUDIO_CLASS(klass); audio_class-connect_channel = connect_channel; +audio_class-get_playback_volume_info_async = spice_pulse_get_playback_volume_info_async; +audio_class-get_playback_volume_info_finish = spice_pulse_get_playback_volume_info_finish; +audio_class-get_record_volume_info_async = spice_pulse_get_record_volume_info_async; +audio_class-get_record_volume_info_finish = spice_pulse_get_record_volume_info_finish; gobject_class-finalize
[Spice-devel] [spice-gtk PATCH v8 2/4] audio: spice-pulse implement async volume-info
In case of volume-sync between client and guest, we request volume-info from the availables streams and if the stream is not available we rely on ext-stream-restore. By using ext-stream-restore we can get the last stream data of the application that is stored by PulseAudio. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1012868 --- gtk/spice-pulse.c | 476 -- 1 file changed, 467 insertions(+), 9 deletions(-) diff --git a/gtk/spice-pulse.c b/gtk/spice-pulse.c index c583032..c2be1b3 100644 --- a/gtk/spice-pulse.c +++ b/gtk/spice-pulse.c @@ -25,18 +25,34 @@ #include pulse/glib-mainloop.h #include pulse/pulseaudio.h +#include pulse/ext-stream-restore.h #define SPICE_PULSE_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_PULSE, SpicePulsePrivate)) +struct async_task { +SpicePulse *pulse; +SpiceMainChannel *main_channel; +GSimpleAsyncResult *res; +GAsyncReadyCallbackcallback; +gpointer user_data; +gboolean is_playback; +pa_operation *pa_op; +gulong cancel_id; +GCancellable *cancellable; +}; + struct stream { -pa_sample_spec spec; -pa_stream *stream; -int state; -pa_operation*uncork_op; -pa_operation*cork_op; -gbooleanstarted; -guint num_underflow; +pa_sample_spec spec; +pa_stream *stream; +intstate; +pa_operation *uncork_op; +pa_operation *cork_op; +gboolean started; +guint num_underflow; +gboolean info_updated; +gchar *name; +pa_ext_stream_restore_info info; }; struct _SpicePulsePrivate { @@ -50,6 +66,8 @@ struct _SpicePulsePrivate { struct stream record; guint last_delay; guint target_delay; +struct async_task *pending_restore_task; +GList *results; }; G_DEFINE_TYPE(SpicePulse, spice_pulse, SPICE_TYPE_AUDIO) @@ -77,6 +95,18 @@ static const char *context_state_names[] = { static void stream_stop(SpicePulse *pulse, struct stream *s); static gboolean connect_channel(SpiceAudio *audio, SpiceChannel *channel); static void channel_weak_notified(gpointer data, GObject *where_the_object_was); +static void spice_pulse_get_playback_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_playback_volume_info_finish(SpiceAudio *audio, GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void spice_pulse_get_record_volume_info_async(SpiceAudio *audio, GCancellable *cancellable, +SpiceMainChannel *main_channel, GAsyncReadyCallback callback, gpointer user_data); +static gboolean spice_pulse_get_record_volume_info_finish(SpiceAudio *audio,GAsyncResult *res, +gboolean *mute, guint8 *nchannels, guint16 **volume, GError **error); +static void stream_restore_read_cb(pa_context *context, +const pa_ext_stream_restore_info *info, int eol, void *userdata); +static void spice_pulse_complete_async_task(struct async_task *task, const gchar *err_msg); +static void spice_pulse_complete_all_async_tasks(SpicePulse *pulse, const gchar *err_msg); static void spice_pulse_finalize(GObject *obj) { @@ -118,6 +148,12 @@ static void spice_pulse_dispose(GObject *obj) pa_operation_unref(p-record.cork_op); p-record.cork_op = NULL; +if (p-results != NULL) +spice_pulse_complete_all_async_tasks(pulse, PulseAudio is being dispose); + +g_free(p-playback.name); +g_free(p-record.name); + if (p-pchannel) g_object_weak_unref(G_OBJECT(p-pchannel), channel_weak_notified, pulse); p-pchannel = NULL; @@ -140,6 +176,10 @@ static void spice_pulse_class_init(SpicePulseClass *klass) SpiceAudioClass *audio_class = SPICE_AUDIO_CLASS(klass); audio_class-connect_channel = connect_channel; +audio_class-get_playback_volume_info_async = spice_pulse_get_playback_volume_info_async; +audio_class-get_playback_volume_info_finish = spice_pulse_get_playback_volume_info_finish; +audio_class-get_record_volume_info_async = spice_pulse_get_record_volume_info_async; +audio_class-get_record_volume_info_finish = spice_pulse_get_record_volume_info_finish; gobject_class-finalize = spice_pulse_finalize; gobject_class-dispose = spice_pulse_dispose; @@ -812,18 +852,40 @@ static void context_state_callback(pa_context *c, void *userdata) if
Re: [Spice-devel] [spice-gtk] Fix GSocketAddress leak in proxy_lookup_ready()
this patch was already acked in February. On Thu, Apr 16, 2015 at 3:36 PM, Christophe Fergeau cferg...@redhat.com wrote: g_proxy_address_new() returns a new GProxyAddress, so we must unref it when no longer needed. This fixes: ==6481== 234 (48 direct, 186 indirect) bytes in 1 blocks are definitely lost in loss record 10,062 of 10, ==6481==at 0x31FF230A58: g_type_create_instance (gtype.c:1849) ==6481==by 0x31FF21501A: g_object_new_internal (gobject.c:1774) ==6481==by 0x31FF216EB4: g_object_new_valist (gobject.c:2033) ==6481==by 0x31FF217220: g_object_new (gobject.c:1617) ==6481==by 0x3D4386F33A: g_proxy_address_new (gproxyaddress.c:325) ==6481==by 0x5717440: proxy_lookup_ready (spice-session.c:2011) ==6481==by 0x3D43885082: g_task_return_now (gtask.c:1088) ==6481==by 0x3D438850B8: complete_in_idle_cb (gtask.c:1102) ==6481==by 0x31FEE4A0B9: g_main_dispatch (gmain.c:3122) ==6481==by 0x31FEE4A0B9: g_main_context_dispatch (gmain.c:3737) ==6481==by 0x31FEE4A44F: g_main_context_iterate.isra.29 (gmain.c:3808) ==6481==by 0x31FEE4A771: g_main_loop_run (gmain.c:4002) ==6481==by 0x363AC06CC4: gtk_main (in /usr/lib64/libgtk-3.so.0.1600.1) --- gtk/spice-session.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gtk/spice-session.c b/gtk/spice-session.c index 1a68d7d..020a70e 100644 --- a/gtk/spice-session.c +++ b/gtk/spice-session.c @@ -2020,6 +2020,7 @@ static void proxy_lookup_ready(GObject *source_object, GAsyncResult *result, open_host_connectable_connect(open_host, G_SOCKET_CONNECTABLE(address)); g_resolver_free_addresses(addresses); +g_object_unref(address); } /* main context */ -- 2.3.5 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk] Fix GSocketAddress leak in proxy_lookup_ready()
On Thu, Apr 16, 2015 at 03:56:39PM +0200, Marc-André Lureau wrote: this patch was already acked in February. I had vague memories of already fixing that, but it's not present in my local git branch, sorry for the resend. I'll push it now this time. Christophe pgpnka2ic6hFP.pgp Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/2] RFC: handle startup race for monitors config
On Thu, 2015-04-09 at 14:06 -0500, Jonathon Jongsma wrote: On Thu, 2015-04-02 at 11:15 -0500, Jonathon Jongsma wrote: On Thu, 2015-04-02 at 16:42 +0200, Marc-André Lureau wrote: On Thu, Apr 2, 2015 at 4:36 PM, Jonathon Jongsma jjong...@redhat.com wrote: That would be a pretty drastic change in behavior. So, no, I have not considered that. It would also open up a large can of worms. For example, what would happen if you re-configured the displays from within the guest control panel. What would you do then? I would eventually follow guest configuration, but I wouldn't send back a new configuration (no auto-resize). So, I did a quick test where I used an un-modified spice-server and the following diff to virt-viewer: === diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c index 131a500..5f47d09 100644 --- a/src/virt-viewer-session.c +++ b/src/virt-viewer-session.c @@ -412,6 +412,13 @@ virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession* self, if (!klass-apply_monitor_geometry) return; +/* don't send monitor resize if we're in auto-conf fullscreen mode...*/ +if (virt_viewer_app_get_fullscreen(self-priv-app)) { +g_debug(%s: not sending new monitors config because app is in fullscreen mode, G_STRFUNC); +return; +} +g_debug(%s: sending, G_STRFUNC); + /* find highest monitor ID so we can create the sparse array */ for (l = self-priv-displays; l; l = l-next) { VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l-data); This is basically what you suggest. While we're in auto-conf mode, it never sends down monitor config changes, but it does respond to display updates from the server. When the user exits fullscreen mode, it resumes auto-resize and sends new monitor config messages to the server. It improved the situation, but didn't fix it. Before applying this patch, when I started virt-viewer in --full-screen mode, the extra display was left enabled about 25-50% of the time. After this patch, it happens only about 10% of the time. So here's the current state of this investigation. Just to recap, this is (as far as I can tell) the source of the problem: [guest has 2 displays enabled] * main channel connects * client sends monitor config (only 1 display enabled) to server * server sends new monitor config to guest * display channel connects * display channel sends out monitor config message with old state (2 displays) * client receives monitor config and enables 2 display windows * guest finishes display configuration * display channel sends out monitor config message with new state (1 display) * second display on client becomes unready (waiting for display 2...) * Some future re-allocation / etc causes the client to send down new monitor configuration, which re-enables the unready second display (see rhbz#868970 for why this happens and why it's very difficult to solve) There are two basic approaches that can be taken here. The first is the one that I originally proposed: handle it on the server side. The rationale for this approach is pretty straightforward: the server *knows* that it is sending out an invalid monitor configuration at startup (since it knows that it has already received a new monitor configuration from the client), and we should not send out wrong information knowingly. The change that I originally proposed is not particularly elegant, I'll admit. It does use a timeout to delay sending out our initial monitor configuration to give the guest a chance to reconfigure itself before we send the configuration. Obviously, if the guest takes longer to reconfigure displays than the timeout, we'll still see this issue. And a timeout inherently adds more chances for race conditions, etc. But in my testing (with host and client on different machines on a local network), I've never yet seen a failure with this patch applied. The second approach is to change the client so that it can handle the server sending us an old configuration followed immediately by a new configuration. However, this is not trivial, and everything I've tried so far to handle it has opened up new regressions in other areas that are hard to predict and require very extensive testing to even find. It's clear that the client sends out too many unnecessary monitor reconfiguration messages. But preventing the client from sending out these messages is risky; it's not always immediately clear which ones are unnecessary. One of Marc-Andre's proposed patches doing that introduced 2 new regressions (mentioned in other places in this thread). It also did not