Re: [Spice-devel] [spice-gtk PATCH v7 2/4] audio: spice-pulse implement async volume-info

2015-04-16 Thread Victor Toso
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

2015-04-16 Thread Marc-André Lureau
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

2015-04-16 Thread Christophe Fergeau
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

2015-04-16 Thread Christophe Fergeau
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

2015-04-16 Thread Daniel P. Berrange
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

2015-04-16 Thread Fabiano Fidencio


- 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()

2015-04-16 Thread Christophe Fergeau
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

2015-04-16 Thread Victor Toso
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

2015-04-16 Thread Victor Toso
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

2015-04-16 Thread Marc-André Lureau
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

2015-04-16 Thread Victor Toso
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()

2015-04-16 Thread Marc-André Lureau
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()

2015-04-16 Thread Christophe Fergeau
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

2015-04-16 Thread Jonathon Jongsma
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