[Spice-devel] [PATCH linux vdagent] Add systemd socket activation

2017-07-13 Thread Jonathon Jongsma
If we are configured to use the systemd init script, also add support
for systemd socket activation. systemd will listen on the socket that is
used to communicate between the session agent and the system daemon.
When the session agent connects, the system daemon will automatically be
started. The socket will be enabled only if the required virtio-port
device exists. The socket is disabled when the device is removed.

This has a couple minor advantages to the previous approach:
  - For VMS that are not running a graphical desktop (and thus no
session agents are running), the system vdagent daemon won't get
started at all even if the spice virtio port is configured. Only the
socket will be enabled. In the previous approach, the system daemon
was started when the virtio device was added regardless of whether
it was needed or not.
  - Solves issues related to switching between systemd targets. With the
previous approach, when a user switches to a different target
("systemctl isolate multi-user.target"), spice-vdagentd.target was
stopped by systemd (since "isolate" by definition stops all targets
except the one specified). This meant that if the user subsequently
switched back to graphical.target, the spice-vdagentd.target would
still be disabled and vdagent functionality would not work. With
this change, the socket will still be listening after switching to a
different target, so as soon as a session agent tries to connect to
the socket, the system daemon will get restarted.

Fixes: rhbz#1340160

Signed-off-by: Jonathon Jongsma 
---
NOTES:
 - this patch piggybacks onto the --with-init-script configure option to
   determine whether to use systemd socket activation. I'm not sure whether
   that's the correct choice or not, but it seems to work ok. If the init
   script is specified as "systemd" or "systemd+redhat" and we have a new
   enough libsystemd, we attempt to use socket activation.
 - Even if the agent is configured with systemd socket activation, it should
   still fall back to opening its own socket if it fails to get a socket from
   systemd (for example, if it is running under a different init system).
 - In theory, we could use socket activation with systemd < 209, but then we'd
   have to link against the separate libsystemd-daemon library. Since this is a
   new feature, I decided to only support more recent versions of systemd. But
   support for older versions could be added pretty easily.


 Makefile.am  |  5 +++-
 configure.ac |  8 +++
 data/70-spice-vdagentd.rules |  2 +-
 data/spice-vdagentd.service  |  8 +--
 data/spice-vdagentd.socket   | 10 
 data/spice-vdagentd.target   |  2 --
 src/udscs.c  | 45 
 src/udscs.h  | 11 +
 src/vdagentd/vdagentd.c  | 55 +++-
 9 files changed, 109 insertions(+), 37 deletions(-)
 create mode 100644 data/spice-vdagentd.socket
 delete mode 100644 data/spice-vdagentd.target

diff --git a/Makefile.am b/Makefile.am
index 7755f09..f70d514 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,7 @@ src_spice_vdagent_SOURCES =   \
 
 src_spice_vdagentd_CFLAGS =\
$(DBUS_CFLAGS)  \
+   $(LIBSYSTEMD_DAEMON_CFLAGS) \
$(LIBSYSTEMD_LOGIN_CFLAGS)  \
$(PCIACCESS_CFLAGS) \
$(SPICE_CFLAGS) \
@@ -52,6 +53,7 @@ src_spice_vdagentd_CFLAGS =   \
 
 src_spice_vdagentd_LDADD = \
$(DBUS_LIBS)\
+   $(LIBSYSTEMD_DAEMON_LIBS)   \
$(LIBSYSTEMD_LOGIN_LIBS)\
$(PCIACCESS_LIBS)   \
$(SPICE_LIBS)   \
@@ -102,7 +104,7 @@ if INIT_SCRIPT_SYSTEMD
 systemdunitdir = $(SYSTEMDSYSTEMUNITDIR)
 systemdunit_DATA = \
$(top_srcdir)/data/spice-vdagentd.service \
-   $(top_srcdir)/data/spice-vdagentd.target
+   $(top_srcdir)/data/spice-vdagentd.socket
 
 udevrulesdir = /lib/udev/rules.d
 udevrules_DATA = $(top_srcdir)/data/70-spice-vdagentd.rules
@@ -124,6 +126,7 @@ EXTRA_DIST =\
data/spice-vdagent.desktop  \
data/spice-vdagentd \
data/spice-vdagentd.service \
+   data/spice-vdagentd.socket  \
data/spice-vdagentd.target  \
data/tmpfiles.d/spice-vdagentd.conf \
data/xorg.conf.RHEL-5   \
diff --git a/configure.ac b/configure.ac
index f207240..fbc20a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,6 +65,14 @@ AC_MSG_RESULT($with_init_script)
 if test "x$init_systemd" = "xyes"; then
   SYSTEMDSYSTEMUNITDIR=`${PKG_CONFIG} systemd 

Re: [Spice-devel] [PATCH spice-gtk 2/2] display: use streams_ namespace for stream related functions

2017-07-13 Thread Jonathon Jongsma
On Fri, 2017-06-30 at 12:51 +0200, Victor Toso wrote:
> From: Victor Toso 
> 
> Changing the name from clear_streams() to streams_finalize() to
> better
> match streams_check_init().

Since I was not really in favor of the new factored-out
streams_check_init() function, the consistency issue maybe isn't as
relevant, but I'm not opposed to a better name here. I'm not sure that
I like _finalize(), since "finalize" has a pretty specific meaning in
GObject, and this function can be called from more places than the
object's finalize stage. I wouldn't be opposed to simply swapping the
order of the words though. e.g. streams_clear(). Or maybe
streams_reset(). 

In OO terms though, it is basically a member function of
SpiceDisplayChannel, and I tend to prefer using the full naming
convention, even for static functions (e.g.
spice_display_channel_streams_clear()?). But there's plenty of
precedent for shortening static functions in the code as well.

Jonathon


> 
> This function is called on:
> - display_handle_stream_destroy_all()
> - spice_display_channel_reset()
> - spice_display_channel_finalize()
> 
> Signed-off-by: Victor Toso 
> ---
>  src/channel-display.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 9ae2851..a68a1ca 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -104,7 +104,7 @@ static void spice_display_channel_up(SpiceChannel
> *channel);
>  static void channel_set_handlers(SpiceChannelClass *klass);
>  
>  static void clear_surfaces(SpiceChannel *channel, gboolean
> keep_primary);
> -static void clear_streams(SpiceChannel *channel);
> +static void streams_finalize(SpiceChannel *channel);
>  static void streams_check_init(SpiceChannel *channel, guint
> stream_id);
>  static display_surface *find_surface(SpiceDisplayChannelPrivate *c,
> guint32 surface_id);
>  static void spice_display_channel_reset(SpiceChannel *channel,
> gboolean migrating);
> @@ -172,7 +172,7 @@ static void
> spice_display_channel_finalize(GObject *object)
>  g_clear_pointer(>monitors, g_array_unref);
>  clear_surfaces(SPICE_CHANNEL(object), FALSE);
>  g_hash_table_unref(c->surfaces);
> -clear_streams(SPICE_CHANNEL(object));
> +streams_finalize(SPICE_CHANNEL(object));
>  g_clear_pointer(>palettes, cache_free);
>  
>  if (G_OBJECT_CLASS(spice_display_channel_parent_class)-
> >finalize)
> @@ -253,7 +253,7 @@ static void
> spice_display_set_property(GObject  *object,
>  static void spice_display_channel_reset(SpiceChannel *channel,
> gboolean migrating)
>  {
>  /* palettes, images, and glz_window are cleared in the session
> */
> -clear_streams(channel);
> +streams_finalize(channel);
>  clear_surfaces(channel, TRUE);
>  
>  SPICE_CHANNEL_CLASS(spice_display_channel_parent_class)-
> >channel_reset(channel, migrating);
> @@ -1572,7 +1572,7 @@ static void
> destroy_display_stream(display_stream *st, int id)
>  g_free(st);
>  }
>  
> -static void clear_streams(SpiceChannel *channel)
> +static void streams_finalize(SpiceChannel *channel)
>  {
>  SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)-
> >priv;
>  int i;
> @@ -1626,7 +1626,7 @@ static void
> display_handle_stream_destroy(SpiceChannel *channel, SpiceMsgIn *in)
>  /* coroutine context */
>  static void display_handle_stream_destroy_all(SpiceChannel *channel,
> SpiceMsgIn *in)
>  {
> -clear_streams(channel);
> +streams_finalize(channel);
>  }
>  
>  /* coroutine context */
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Marc-André Lureau


- Original Message -
> On Thu, Jul 13, 2017 at 12:13:59PM -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> > > On Thu, Jul 13, 2017 at 08:58:42AM -0400, Marc-André Lureau wrote:
> > > > Hi
> > > > 
> > > > - Original Message -
> > > > > On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> > > > > > >
> > > > > > > This is the latest gettext release, which is not even 1 year old,
> > > > > > > is
> > > > > > > this the only one which is going to work? Or is it possible to
> > > > > > > relax
> > > > > > > this version chekc?
> > > > > > >
> > > > > > 
> > > > > > It's quite a mess to downgrade gettext or find the minimum required
> > > > > > version. RHEL7 has 0.19.8.1 already. I suppose it should work with
> > > > > > earlier
> > > > > > 0.19 releases, perhaps even older..
> > > > > 
> > > > > RHEL7.x is using 0.18, and has been doing that for a few point
> > > > > releases.
> > > > > RHEL7.4 will apparently have gettext 0.19, but that's not released
> > > > > yet..
> > > > > Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
> > > > > stable/oldstable are both on 0.19.
> > > > > However, I was able to build spice-gtk git master on an up to date
> > > > > RHEL
> > > > > 7.3 after changing the required gettext version to 0.18, so I'd just
> > > > > lower the requirement.
> > > > > 
> > > > 
> > > > ACK feel free to push the change if you tested it.
> > > 
> > > Building with AM_GNU_GETTEXT_VERSION([0.18]) works, except that this
> > > triggers some autotools warnings about the use of a deprecated
> > > AM_PROG_MKDIR_P macro, which then aborts as we are using
> > > AM_INIT_AUTOMATKE([-Werror]). Did not have time to dig more into it yet.
> > > 
> > 
> > How do you reproduce it? I have no such error on f26, and my rhel7 is
> > already 0.19.8.1
> 
> git clean -dfx before running autogen.sh.
> I suspect your rhel7 is a 7.4 development version, not a released
> version.
> 

Ok thanks, make it 0.18.2 and it passes. Apparently that's what rhel/centos 
have, so it should work there too.

> > 
> > > I'd just revert this patch as what is meant as a build system cleanup
> > > prevents building from git on platforms which are fine otherwise.
> > 
> > They can still build from releases if they don't modify build-sys. If
> > they do, it's fair to also change gettext version, or revert this
> > patch.
> > 
> > The question is do we want/need to target those old distros for
> > developpers?
> 
> old? RHEL/CentOS 7.3 are the latest releases at the moment. You suggest
> complicating build on these distros, but for not much gain.
> 
> Christophe
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] Fix occasional black screen at startup

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

The problem occurs when we call spice_playback_channel_set_delay before
the channel had received any data setting c->last_time, but after
session initialization had set mm_time in the session. The result was
that the (good) value in session->mm_time would be overwritten with 0.

Signed-off-by: Christophe de Dinechin 
---
 src/channel-playback.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/channel-playback.c b/src/channel-playback.c
index ca14b96..f5acf23 100644
--- a/src/channel-playback.c
+++ b/src/channel-playback.c
@@ -471,7 +471,8 @@ void spice_playback_channel_set_delay(SpicePlaybackChannel 
*channel, guint32 del
 
 session = spice_channel_get_session(SPICE_CHANNEL(channel));
 if (session) {
-spice_session_set_mm_time(session, c->last_time - delay_ms);
+if (c->last_time != 0)
+spice_session_set_mm_time(session, c->last_time - delay_ms);
 } else {
 CHANNEL_DEBUG(channel, "channel detached from session, mm time 
skipped");
 }
-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 1/2] display: factor out initialization of stream array

2017-07-13 Thread Jonathon Jongsma
On Fri, 2017-06-30 at 12:51 +0200, Victor Toso wrote:
> From: Victor Toso 
> 
> Including some comment about current implementation of stream-id
> value
> in the Spice.
> 
> Signed-off-by: Victor Toso 
> ---
>  src/channel-display.c | 42 +++
> ---
>  1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 06c503c..9ae2851 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -105,6 +105,7 @@ static void
> channel_set_handlers(SpiceChannelClass *klass);
>  
>  static void clear_surfaces(SpiceChannel *channel, gboolean
> keep_primary);
>  static void clear_streams(SpiceChannel *channel);
> +static void streams_check_init(SpiceChannel *channel, guint
> stream_id);
>  static display_surface *find_surface(SpiceDisplayChannelPrivate *c,
> guint32 surface_id);
>  static void spice_display_channel_reset(SpiceChannel *channel,
> gboolean migrating);
>  static void spice_display_channel_reset_capabilities(SpiceChannel
> *channel);
> @@ -1233,17 +1234,7 @@ static void
> display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
>  
>  CHANNEL_DEBUG(channel, "%s: id %u", __FUNCTION__, op->id);
>  
> -if (op->id >= c->nstreams) {
> -int n = c->nstreams;
> -if (!c->nstreams) {
> -c->nstreams = 1;
> -}
> -while (op->id >= c->nstreams) {
> -c->nstreams *= 2;
> -}
> -c->streams = realloc(c->streams, c->nstreams * sizeof(c-
> >streams[0]));
> -memset(c->streams + n, 0, (c->nstreams - n) * sizeof(c-
> >streams[0]));
> -}
> +streams_check_init(channel, op->id);
>  g_return_if_fail(c->streams[op->id] == NULL);
>  
>  c->streams[op->id] = display_stream_create(channel, op-
> >surface_id,
> @@ -1593,6 +1584,35 @@ static void clear_streams(SpiceChannel
> *channel)
>  c->nstreams = 0;
>  }
>  
> +/* Always called on SPICE_MSG_DISPLAY_STREAM_CREATE to verify if the
> c->streams
> + * array is big enough to handle the new stream. This obviously
> takes in
> + * consideration that the stream_id could grow overtime from 0 to
> MAX_STREAMS
> + * value but on server side, the id is picked in descending order,
> starting with
> + * (MAX_STREAMS - 1). As the id itself is not enforced by the
> protocol, we
> + * should keep the current check to avoid regressions or other
> unknown issues.

This comment is not totally clear to me. What is the "current check"
that you're referring to here?


> + */
> +static void streams_check_init(SpiceChannel *channel, guint
> stream_id)
> +{
> +SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)-
> >priv;
> +gint old, new;
> +
> +if (stream_id < c->nstreams) {
> +/* No need to increase c->streams */
> +return;
> +}
> +
> +old = c->nstreams;
> +new = (c->nstreams == 0) ? 1 : c->nstreams;
> +
> +while (stream_id > new) {
> +new *= 2;
> +}
> +
> +c->nstreams = new;
> +c->streams = realloc(c->streams, new * sizeof(c->streams[0]));
> +memset(c->streams + old, 0, (new - old) * sizeof(c-
> >streams[0]));
> +}
> +
>  /* coroutine context */
>  static void display_handle_stream_destroy(SpiceChannel *channel,
> SpiceMsgIn *in)
>  {



To be honest, I don't know if I see the value in factoring this out to
a separate function. It's only called from a single place, and in some
ways it makes the behavior of the code a bit more opaque since the
caller is relying on side-effects of the called function. For example,
consider the following simplified and contrived example. I think this
code is pretty clear:

void func(SpiceChannel *channel, uint8_t n)
{
  if (!channel->priv->member) {
channel->priv->member = g_new0(uint8_t, NUM_ELEMENTS);
  }
  channel->priv->member[0] = n;
}



This code is less clear:

void func(SpiceChannel *channel, uint8_t n)
{
  init(channel);
  channel->priv->member[0] = n;
  /* how do we know that the priv->member array has been allocated?
   * we have to go look at the implementation of init() */
}

void init(SpiceChannel *channel)
{
  if (!channel->priv->member) {
channel->priv->member = g_new0(uint8_t, NUM_ELEMENTS);
  }
}


If the same init() code was used multiple places, it would make more
sense to factor it out to a separate function, but I'm not sure it's
very beneficial when it's just called from one location.

Jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix build for GStreamer < 1.5.1

2017-07-13 Thread Marc-André Lureau
Hi

- Original Message -
> GST_DEBUG_GRAPH_SHOW_FULL_PARAMS is available since that
> ---
>  src/channel-display-gst.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 87e472c..fcb9e25 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -297,7 +297,9 @@ static gboolean handle_pipeline_message(GstBus *bus,
> GstMessage *msg, gpointer v
>
> gst_opts[decoder->base.codec_type].name);
>  GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(decoder->pipeline),
>GST_DEBUG_GRAPH_SHOW_ALL
> +#if GST_CHECK_VERSION(1,5,1)
>  | GST_DEBUG_GRAPH_SHOW_FULL_PARAMS
> +#endif
>  | GST_DEBUG_GRAPH_SHOW_STATES,
>  filename);
>  g_free(filename);
> --
> 2.13.0

ack
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] gst: Fix build for GStreamer < 1.5.1

2017-07-13 Thread Pavel Grunt
GST_DEBUG_GRAPH_SHOW_FULL_PARAMS is available since that
---
 src/channel-display-gst.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 87e472c..fcb9e25 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -297,7 +297,9 @@ static gboolean handle_pipeline_message(GstBus *bus, 
GstMessage *msg, gpointer v
   
gst_opts[decoder->base.codec_type].name);
 GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(decoder->pipeline),
   GST_DEBUG_GRAPH_SHOW_ALL
+#if GST_CHECK_VERSION(1,5,1)
 | GST_DEBUG_GRAPH_SHOW_FULL_PARAMS
+#endif
 | GST_DEBUG_GRAPH_SHOW_STATES,
 filename);
 g_free(filename);
-- 
2.13.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Christophe Fergeau
On Thu, Jul 13, 2017 at 12:13:59PM -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > On Thu, Jul 13, 2017 at 08:58:42AM -0400, Marc-André Lureau wrote:
> > > Hi
> > > 
> > > - Original Message -
> > > > On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> > > > > >
> > > > > > This is the latest gettext release, which is not even 1 year old, is
> > > > > > this the only one which is going to work? Or is it possible to relax
> > > > > > this version chekc?
> > > > > >
> > > > > 
> > > > > It's quite a mess to downgrade gettext or find the minimum required
> > > > > version. RHEL7 has 0.19.8.1 already. I suppose it should work with
> > > > > earlier
> > > > > 0.19 releases, perhaps even older..
> > > > 
> > > > RHEL7.x is using 0.18, and has been doing that for a few point releases.
> > > > RHEL7.4 will apparently have gettext 0.19, but that's not released yet..
> > > > Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
> > > > stable/oldstable are both on 0.19.
> > > > However, I was able to build spice-gtk git master on an up to date RHEL
> > > > 7.3 after changing the required gettext version to 0.18, so I'd just
> > > > lower the requirement.
> > > > 
> > > 
> > > ACK feel free to push the change if you tested it.
> > 
> > Building with AM_GNU_GETTEXT_VERSION([0.18]) works, except that this
> > triggers some autotools warnings about the use of a deprecated
> > AM_PROG_MKDIR_P macro, which then aborts as we are using
> > AM_INIT_AUTOMATKE([-Werror]). Did not have time to dig more into it yet.
> > 
> 
> How do you reproduce it? I have no such error on f26, and my rhel7 is already 
> 0.19.8.1

git clean -dfx before running autogen.sh.
I suspect your rhel7 is a 7.4 development version, not a released
version.

> 
> > I'd just revert this patch as what is meant as a build system cleanup
> > prevents building from git on platforms which are fine otherwise.
> 
> They can still build from releases if they don't modify build-sys. If
> they do, it's fair to also change gettext version, or revert this
> patch.
> 
> The question is do we want/need to target those old distros for developpers?

old? RHEL/CentOS 7.3 are the latest releases at the moment. You suggest
complicating build on these distros, but for not much gain.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk] build-sys: Update keycodemapdb submodule

2017-07-13 Thread Marc-André Lureau


- Original Message -
> This brings in 2 bug fixes, one of these is important on RHEL7 to
> workaround an argparser bug with the python2 version it ships.
> 
> Signed-off-by: Christophe Fergeau 

ack

> ---
>  src/keycodemapdb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/keycodemapdb b/src/keycodemapdb
> index 1a6aaf85..7bf5710b 16
> --- a/src/keycodemapdb
> +++ b/src/keycodemapdb
> @@ -1 +1 @@
> -Subproject commit 1a6aaf853ff7202b869dc7868f800ed7da9538d0
> +Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
> --
> 2.13.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Marc-André Lureau
Hi

- Original Message -
> On Thu, Jul 13, 2017 at 08:58:42AM -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> > > On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> > > > >
> > > > > This is the latest gettext release, which is not even 1 year old, is
> > > > > this the only one which is going to work? Or is it possible to relax
> > > > > this version chekc?
> > > > >
> > > > 
> > > > It's quite a mess to downgrade gettext or find the minimum required
> > > > version. RHEL7 has 0.19.8.1 already. I suppose it should work with
> > > > earlier
> > > > 0.19 releases, perhaps even older..
> > > 
> > > RHEL7.x is using 0.18, and has been doing that for a few point releases.
> > > RHEL7.4 will apparently have gettext 0.19, but that's not released yet..
> > > Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
> > > stable/oldstable are both on 0.19.
> > > However, I was able to build spice-gtk git master on an up to date RHEL
> > > 7.3 after changing the required gettext version to 0.18, so I'd just
> > > lower the requirement.
> > > 
> > 
> > ACK feel free to push the change if you tested it.
> 
> Building with AM_GNU_GETTEXT_VERSION([0.18]) works, except that this
> triggers some autotools warnings about the use of a deprecated
> AM_PROG_MKDIR_P macro, which then aborts as we are using
> AM_INIT_AUTOMATKE([-Werror]). Did not have time to dig more into it yet.
> 

How do you reproduce it? I have no such error on f26, and my rhel7 is already 
0.19.8.1

> I'd just revert this patch as what is meant as a build system cleanup
> prevents building from git on platforms which are fine otherwise.

They can still build from releases if they don't modify build-sys. If they do, 
it's fair to also change gettext version, or revert this patch.

The question is do we want/need to target those old distros for developpers?
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk] build-sys: Update keycodemapdb submodule

2017-07-13 Thread Christophe Fergeau
This brings in 2 bug fixes, one of these is important on RHEL7 to
workaround an argparser bug with the python2 version it ships.

Signed-off-by: Christophe Fergeau 
---
 src/keycodemapdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/keycodemapdb b/src/keycodemapdb
index 1a6aaf85..7bf5710b 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 1a6aaf853ff7202b869dc7868f800ed7da9538d0
+Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
-- 
2.13.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Christophe Fergeau
On Thu, Jul 13, 2017 at 08:58:42AM -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> > > >
> > > > This is the latest gettext release, which is not even 1 year old, is
> > > > this the only one which is going to work? Or is it possible to relax
> > > > this version chekc?
> > > >
> > > 
> > > It's quite a mess to downgrade gettext or find the minimum required
> > > version. RHEL7 has 0.19.8.1 already. I suppose it should work with earlier
> > > 0.19 releases, perhaps even older..
> > 
> > RHEL7.x is using 0.18, and has been doing that for a few point releases.
> > RHEL7.4 will apparently have gettext 0.19, but that's not released yet..
> > Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
> > stable/oldstable are both on 0.19.
> > However, I was able to build spice-gtk git master on an up to date RHEL
> > 7.3 after changing the required gettext version to 0.18, so I'd just
> > lower the requirement.
> > 
> 
> ACK feel free to push the change if you tested it.

Building with AM_GNU_GETTEXT_VERSION([0.18]) works, except that this
triggers some autotools warnings about the use of a deprecated
AM_PROG_MKDIR_P macro, which then aborts as we are using
AM_INIT_AUTOMATKE([-Werror]). Did not have time to dig more into it yet.

I'd just revert this patch as what is meant as a build system cleanup
prevents building from git on platforms which are fine otherwise.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 0/3] spice-gtk: Drop frames if they pile up because client is overloaded

2017-07-13 Thread Marc-André Lureau
Hi

- Original Message -
> From: Christophe de Dinechin 
> 
> This patch series is a crude attempt at getting rid of the frames backlog
> when the spice client is overloaded. Another parameter I have been
> playing wiht is the max-bytes parameter in the GStreamer
> pipeline, which is currently set to 0 (unlimited), which is probably
> a lot more than necessary.
> 
> I am still experimenting to figure out the correct values, and I am
> considering less agressive heuristics than dropping *all* the
> frames. I am also not entirely convinced that returning TRUE when we
> clear the frames is the right thing to do. It looks like falling
> through is another valid option. This is particularly true if the
> frame is an I-frame.
> 
> I share this patch as is as an illustration of a scenario
> where it is nice to be able to tweak parameters dynamically for
> experimentation purpose. In my 'recorder' branch, the tweaks are
> dynamic and defined using an environment variable.
> 

For now it's a simple #define, let's not hide this behind a SPICE_TWEAK macro.

If the parameter is useful for end-users, it can be exposed as an option and 
object properties, so it can be changed during run-time via UI.

> Christophe de Dinechin (3):
>   Add SPICE_TWEAK_DEFINE and SPICE_TWEAK macros
>   Drop frames if the backlog is above some limit
>   Add configurable value for gst_max_bytes
> 
>  src/channel-display-gst.c | 18 +-
>  src/spice-util.h  |  3 +++
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> --
> 2.11.0 (Apple Git-81)
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/3] Drop frames if the backlog is above some limit

2017-07-13 Thread Marc-André Lureau


- Original Message -
> From: Christophe de Dinechin 
> 
> Experience has shown that if the machine running the guest is overloaded,
> it may pile up a lot of backlog in the frames queue. This patch clears
> the queue if it exceeds 100 entries. This value is arbitrary. It
> corresponds to a few seconds on a highly overloaded machine.
> 
> Signed-off-by: Christophe de Dinechin 
> ---
>  src/channel-display-gst.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 3cf451b..17c2847 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -498,6 +498,9 @@ static void spice_gst_decoder_destroy(VideoDecoder
> *video_decoder)
>   * 9) display_frame() then frees the SpiceGstFrame, which frees the
>   SpiceFrame
>   *and decompressed frame with it.
>   */
> +
> +SPICE_TWEAK_DEFINE(gst_queue_max_length, 100, "Max length of GStreamer
> queue");
> +
>  static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>SpiceFrame *frame, int
>latency)
>  {
> @@ -541,6 +544,16 @@ static gboolean
> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>  }
>  #endif
>  
> +if (SPICE_TWEAK(gst_queue_max_length) &&
> +decoder->decoding_queue->length > SPICE_TWEAK(gst_queue_max_length))
> {
> +   SpiceGstFrame *gstframe;
> +   g_mutex_lock(>queues_mutex);
> +while ((gstframe = g_queue_pop_head(decoder->decoding_queue)))
> +free_gst_frame(gstframe);
> +g_mutex_unlock(>queues_mutex);
> +return TRUE;
> +}
> +

I am surprised that GStreamer queues are not smart enough to deal with this 
kind of situation. Oh wait, why is the queue in spice-gtk side? Perhaps 
Francois can comment on this?

>  /* ref() the frame data for the buffer */
>  frame->ref_data(frame->data_opaque);
>  GstBuffer *buffer =
>  gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_PHYSICALLY_CONTIGUOUS,
> --
> 2.11.0 (Apple Git-81)
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk 0/3] spice-gtk: Drop frames if they pile up because client is overloaded

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

This patch series is a crude attempt at getting rid of the frames backlog
when the spice client is overloaded. Another parameter I have been
playing wiht is the max-bytes parameter in the GStreamer
pipeline, which is currently set to 0 (unlimited), which is probably
a lot more than necessary.

I am still experimenting to figure out the correct values, and I am
considering less agressive heuristics than dropping *all* the
frames. I am also not entirely convinced that returning TRUE when we
clear the frames is the right thing to do. It looks like falling
through is another valid option. This is particularly true if the
frame is an I-frame.

I share this patch as is as an illustration of a scenario
where it is nice to be able to tweak parameters dynamically for
experimentation purpose. In my 'recorder' branch, the tweaks are
dynamic and defined using an environment variable.

Christophe de Dinechin (3):
  Add SPICE_TWEAK_DEFINE and SPICE_TWEAK macros
  Drop frames if the backlog is above some limit
  Add configurable value for gst_max_bytes

 src/channel-display-gst.c | 18 +-
 src/spice-util.h  |  3 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk 3/3] Add configurable value for gst_max_bytes

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

The GStreamer configuration currently sets max-bytes=0 in the pipeline.
The rationale is that we need to be able to keep frames in case later frames
require them for decoding. However, having experienced scenarios where
spice would accumulate gigabytes of late frames. So it would be useful
to be able to configure that value at runtime.

Signed-off-by: Christophe de Dinechin 
---
 src/channel-display-gst.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 17c2847..b60c7df 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -332,6 +332,8 @@ static void app_source_setup(GstElement *pipeline 
G_GNUC_UNUSED,
 }
 #endif
 
+SPICE_TWEAK_DEFINE(gst_max_bytes, 0, "Max number of bytes queued, 
0=unlimited");
+
 static gboolean create_pipeline(SpiceGstDecoder *decoder)
 {
 GstAppSinkCallbacks appsink_cbs = { NULL };
@@ -388,9 +390,10 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
  * - Set max-bytes=0 on appsrc so it does not drop frames that may be
  *   needed by those that follow.
  */
-desc = g_strdup_printf("appsrc name=src is-live=true format=time 
max-bytes=0 block=true "
+desc = g_strdup_printf("appsrc name=src is-live=true format=time 
max-bytes=%lu block=true "
"caps=%s ! %s ! videoconvert ! appsink name=sink "
"caps=video/x-raw,format=BGRx sync=false 
drop=false",
+   SPICE_TWEAK(gst_max_bytes),
gst_opts[decoder->base.codec_type].dec_caps,
gst_opts[decoder->base.codec_type].dec_name);
 SPICE_DEBUG("GStreamer pipeline: %s", desc);
-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk 2/3] Drop frames if the backlog is above some limit

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

Experience has shown that if the machine running the guest is overloaded,
it may pile up a lot of backlog in the frames queue. This patch clears
the queue if it exceeds 100 entries. This value is arbitrary. It
corresponds to a few seconds on a highly overloaded machine.

Signed-off-by: Christophe de Dinechin 
---
 src/channel-display-gst.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 3cf451b..17c2847 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -498,6 +498,9 @@ static void spice_gst_decoder_destroy(VideoDecoder 
*video_decoder)
  * 9) display_frame() then frees the SpiceGstFrame, which frees the SpiceFrame
  *and decompressed frame with it.
  */
+
+SPICE_TWEAK_DEFINE(gst_queue_max_length, 100, "Max length of GStreamer queue");
+
 static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
   SpiceFrame *frame, int latency)
 {
@@ -541,6 +544,16 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder 
*video_decoder,
 }
 #endif
 
+if (SPICE_TWEAK(gst_queue_max_length) &&
+decoder->decoding_queue->length > SPICE_TWEAK(gst_queue_max_length)) {
+   SpiceGstFrame *gstframe;
+   g_mutex_lock(>queues_mutex);
+while ((gstframe = g_queue_pop_head(decoder->decoding_queue)))
+free_gst_frame(gstframe);
+g_mutex_unlock(>queues_mutex);
+return TRUE;
+}
+
 /* ref() the frame data for the buffer */
 frame->ref_data(frame->data_opaque);
 GstBuffer *buffer = 
gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_PHYSICALLY_CONTIGUOUS,
-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk 1/3] Add SPICE_TWEAK_DEFINE and SPICE_TWEAK macros

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

These two macros are intended to identify places in the code where there
are ad-hoc values that can be tweaked to optimize behavior. They may be
connected later to a mechanism that allows such "tweaks" to be made
without recompiling, e.g. by passing environment variables.

Signed-off-by: Christophe de Dinechin 
---
 src/spice-util.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/spice-util.h b/src/spice-util.h
index 1d80f1c..9aa346b 100644
--- a/src/spice-util.h
+++ b/src/spice-util.h
@@ -38,6 +38,9 @@ gchar* spice_uuid_to_string(const guint8 uuid[16]);
 g_debug(G_STRLOC " " fmt, ## __VA_ARGS__);  \
 } while (0)
 
+#define SPICE_TWEAK_DEFINE(Name, Value, Description)gint64 Name = Value
+#define SPICE_TWEAK(Name)   (Name)
+
 #define SPICE_RESERVED_PADDING (10 * sizeof(void*))
 
 G_END_DECLS
-- 
2.11.0 (Apple Git-81)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] spice-gtk: Use time comparisons that still work after wraparound

2017-07-13 Thread Christophe de Dinechin
From: Christophe de Dinechin 

The mm timer is a millisecond timer that wraps around after ~49 days.
All comparisons that look like a
---
 src/channel-display-gst.c   | 4 ++--
 src/channel-display-mjpeg.c | 4 ++--
 src/channel-display.c   | 2 +-
 src/channel-playback.c  | 2 +-
 src/spice-channel-priv.h| 5 +
 src/spice-session.c | 4 ++--
 6 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 3cf451b..495036f 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -166,7 +166,7 @@ static void schedule_frame(SpiceGstDecoder *decoder)
 break;
 }
 
-if (now < gstframe->frame->mm_time) {
+if (spice_mm_strictly_before(now, gstframe->frame->mm_time)) {
 decoder->timer_id = g_timeout_add(gstframe->frame->mm_time - now,
   display_frame, decoder);
 } else if (g_queue_get_length(decoder->display_queue) == 1) {
@@ -509,7 +509,7 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder 
*video_decoder,
 return TRUE;
 }
 
-if (frame->mm_time < decoder->last_mm_time) {
+if (spice_mm_strictly_before(frame->mm_time, decoder->last_mm_time)) {
 SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):"
 " resetting stream",
 frame->mm_time, decoder->last_mm_time);
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index ee33b01..c91fa53 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -201,7 +201,7 @@ static void mjpeg_decoder_schedule(MJpegDecoder *decoder)
 decoder->cur_frame = NULL;
 do {
 if (frame) {
-if (time <= frame->mm_time) {
+if (spice_mm_before(time, frame->mm_time)) {
 guint32 d = frame->mm_time - time;
 decoder->cur_frame = frame;
 decoder->timer_id = g_timeout_add(d, 
mjpeg_decoder_decode_frame, decoder);
@@ -251,7 +251,7 @@ static gboolean mjpeg_decoder_queue_frame(VideoDecoder 
*video_decoder,
 
 last_frame = g_queue_peek_tail(decoder->msgq);
 if (last_frame) {
-if (frame->mm_time < last_frame->mm_time) {
+if (spice_mm_strictly_before(frame->mm_time, last_frame->mm_time)) {
 /* This should really not happen */
 SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):"
 " resetting stream",
diff --git a/src/channel-display.c b/src/channel-display.c
index 3c98d79..fe7506e 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1366,7 +1366,7 @@ static void 
display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
 }
 
 if (st->report_num_frames >= st->report_max_window ||
-now - st->report_start_time >= st->report_timeout ||
+spice_mm_after(now - st->report_start_time, st->report_timeout) ||
 st->report_drops_seq_len >= STREAM_REPORT_DROP_SEQ_LEN_LIMIT) {
 SpiceMsgcDisplayStreamReport report;
 SpiceSession *session = 
spice_channel_get_session(SPICE_CHANNEL(channel));
diff --git a/src/channel-playback.c b/src/channel-playback.c
index ca14b96..451dfaf 100644
--- a/src/channel-playback.c
+++ b/src/channel-playback.c
@@ -313,7 +313,7 @@ static void playback_handle_data(SpiceChannel *channel, 
SpiceMsgIn *in)
   packet->time, packet->data, packet->data_size);
 #endif
 
-if (c->last_time > packet->time)
+if (spice_mm_strictly_after(c->last_time, packet->time))
 g_warn_if_reached();
 
 c->last_time = packet->time;
diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h
index 7288920..b896aaa 100644
--- a/src/spice-channel-priv.h
+++ b/src/spice-channel-priv.h
@@ -43,6 +43,11 @@ G_BEGIN_DECLS
 #define CHANNEL_DEBUG(channel, fmt, ...) \
 SPICE_DEBUG("%s: " fmt, SPICE_CHANNEL(channel)->priv->name, ## __VA_ARGS__)
 
+#define spice_mm_after(now, time)   ((int32_t) ((now)-(time)) >= 0)
+#define spice_mm_before(now, time)  ((int32_t) ((now)-(time)) <= 0)
+#define spice_mm_strictly_after(now, time)  ((int32_t) ((now)-(time)) > 0)
+#define spice_mm_strictly_before(now, time) ((int32_t) ((now)-(time)) < 0)
+
 struct _SpiceMsgOut {
 int   refcount;
 SpiceChannel  *channel;
diff --git a/src/spice-session.c b/src/spice-session.c
index 6f8cf5e..62b041e 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -2336,8 +2336,8 @@ void spice_session_set_mm_time(SpiceSession *session, 
guint32 time)
 s->mm_time = time;
 s->mm_time_at_clock = g_get_monotonic_time();
 SPICE_DEBUG("set mm time: %u", spice_session_get_mm_time(session));
-if (time > old_time + MM_TIME_DIFF_RESET_THRESH ||
-time < old_time) {
+if (spice_mm_strictly_after(time, old_time + MM_TIME_DIFF_RESET_THRESH) ||
+spice_mm_strictly_before(time, 

Re: [Spice-devel] [spice-gtk v1] display-gst: Improve h264 elements filtering

2017-07-13 Thread Victor Toso
Hi,

On Thu, Jul 13, 2017 at 02:50:52PM +0200, Pavel Grunt wrote:
> On Thu, 2017-07-13 at 14:33 +0200, Victor Toso wrote:
> > From: Victor Toso 
> >
> > This patch fixes the avdec_h264 element not being present on
> > gstvideo_has_codec() which get all decoder elements from GstRegistry
> > and filter them on our GstCaps in order to get the ones for given
> > codec.
> > 
> > The issue is around the filtering. The current GstCaps for h264 is not
> > consider a subset of avdec_h264's capabilites and that will fiter this
> > element out of the list.
> > 
> > The proposed solution for that is to set `subsetonly` parameter from
> > gst_element_factory_list_filter() to false.
>
> that is incorrect for the same reason as my patch (some required
> element plugin may be missing)

This is correct for the following question:
- Do we have video (or image) decoders for given capability (such as
  video/x-h264) ?

This patch should solve this question correctly.

But we are not verifying if the _pipeline_ works, for that we should
stick with previous implementation. It is not a problem to add the
parser I just think it'll add unnecessary complexity and could lead to
new errors (such as mentioned before, not having a decoder element while
having a parser element for that capability).

> >
> > While at it, the cap "stream-format=byte-stream" is less useful now
> > because it isn't needed to play the stream - see 6fe88871240c53b8
> >
> > In my system, our debug shows:
> > .. gstvideo_debug_available_decoders: From 228 video decoder elements,
> > - 4 can handle caps   image/jpeg: jpegdec, nvdec, avdec_mjpeg, vaapijpegdec
>
> like here a parser for avdec_mjpeg
>
> > - 3 can handle caps  video/x-vp8: vaapidecodebin, vp8dec, avdec_vp8
> > - 4 can handle caps video/x-h264: vaapidecodebin, avdec_h264, nvdec,
> > vaapih264dec
> > - 3 can handle caps  video/x-vp9: vaapidecodebin, vp9dec, avdec_vp9
> > 
> > Signed-off-by: Victor Toso 
> > ---
> >  src/channel-display-gst.c  | 2 +-
> >  src/channel-display-priv.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 20d236a..1bd7df1 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -647,7 +647,7 @@ gboolean gstvideo_has_codec(int codec_type)
> >  }
> >  
> >  caps = gst_caps_from_string(gst_opts[codec_type].dec_caps);
> > -codec_decoders = gst_element_factory_list_filter(all_decoders, caps,
> > GST_PAD_SINK, TRUE);
> > +codec_decoders = gst_element_factory_list_filter(all_decoders, caps,
> > GST_PAD_SINK, FALSE);
> >  gst_caps_unref(caps);
> >  
> >  if (codec_decoders == NULL) {
> > diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> > index 04cb4d1..9bfd4ac 100644
> > --- a/src/channel-display-priv.h
> > +++ b/src/channel-display-priv.h
> > @@ -181,7 +181,7 @@ static const struct {
> >   * (hardcoded in spice-server), let's add it here to avoid the warning.
> >   */
> >  { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
> > -  "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
> > +  "h264parse ! avdec_h264", "video/x-h264" },
> >  
> >  /* SPICE_VIDEO_CODEC_TYPE_VP9 */
> >  { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Marc-André Lureau
Hi

- Original Message -
> On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> > >
> > > This is the latest gettext release, which is not even 1 year old, is
> > > this the only one which is going to work? Or is it possible to relax
> > > this version chekc?
> > >
> > 
> > It's quite a mess to downgrade gettext or find the minimum required
> > version. RHEL7 has 0.19.8.1 already. I suppose it should work with earlier
> > 0.19 releases, perhaps even older..
> 
> RHEL7.x is using 0.18, and has been doing that for a few point releases.
> RHEL7.4 will apparently have gettext 0.19, but that's not released yet..
> Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
> stable/oldstable are both on 0.19.
> However, I was able to build spice-gtk git master on an up to date RHEL
> 7.3 after changing the required gettext version to 0.18, so I'd just
> lower the requirement.
> 

ACK feel free to push the change if you tested it.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk 2/2] build-sys: modernize gettext

2017-07-13 Thread Christophe Fergeau
On Wed, Jul 12, 2017 at 03:20:26PM +, Marc-André Lureau wrote:
> >
> > This is the latest gettext release, which is not even 1 year old, is
> > this the only one which is going to work? Or is it possible to relax
> > this version chekc?
> >
> 
> It's quite a mess to downgrade gettext or find the minimum required
> version. RHEL7 has 0.19.8.1 already. I suppose it should work with earlier
> 0.19 releases, perhaps even older..

RHEL7.x is using 0.18, and has been doing that for a few point releases.
RHEL7.4 will apparently have gettext 0.19, but that's not released yet..
Ubuntu 16.04 LTS has 0.19, Ubuntu 14.04 had 0.18. Debian
stable/oldstable are both on 0.19.
However, I was able to build spice-gtk git master on an up to date RHEL
7.3 after changing the required gettext version to 0.18, so I'd just
lower the requirement.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v1] display-gst: Improve h264 elements filtering

2017-07-13 Thread Pavel Grunt
On Thu, 2017-07-13 at 14:33 +0200, Victor Toso wrote:
> From: Victor Toso 
> 
> This patch fixes the avdec_h264 element not being present on
> gstvideo_has_codec() which get all decoder elements from GstRegistry
> and filter them on our GstCaps in order to get the ones for given
> codec.
> 
> The issue is around the filtering. The current GstCaps for h264 is not
> consider a subset of avdec_h264's capabilites and that will fiter this
> element out of the list.
> 
> The proposed solution for that is to set `subsetonly` parameter from
> gst_element_factory_list_filter() to false.

that is incorrect for the same reason as my patch (some required element plugin
may be missing)

> 
> While at it, the cap "stream-format=byte-stream" is less useful now
> because it isn't needed to play the stream - see 6fe88871240c53b8
> 
> In my system, our debug shows:
> .. gstvideo_debug_available_decoders: From 228 video decoder elements,
> - 4 can handle caps   image/jpeg: jpegdec, nvdec, avdec_mjpeg, vaapijpegdec

like here a parser for avdec_mjpeg

> - 3 can handle caps  video/x-vp8: vaapidecodebin, vp8dec, avdec_vp8
> - 4 can handle caps video/x-h264: vaapidecodebin, avdec_h264, nvdec,
> vaapih264dec
> - 3 can handle caps  video/x-vp9: vaapidecodebin, vp9dec, avdec_vp9
> 
> Signed-off-by: Victor Toso 
> ---
>  src/channel-display-gst.c  | 2 +-
>  src/channel-display-priv.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 20d236a..1bd7df1 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -647,7 +647,7 @@ gboolean gstvideo_has_codec(int codec_type)
>  }
>  
>  caps = gst_caps_from_string(gst_opts[codec_type].dec_caps);
> -codec_decoders = gst_element_factory_list_filter(all_decoders, caps,
> GST_PAD_SINK, TRUE);
> +codec_decoders = gst_element_factory_list_filter(all_decoders, caps,
> GST_PAD_SINK, FALSE);
>  gst_caps_unref(caps);
>  
>  if (codec_decoders == NULL) {
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index 04cb4d1..9bfd4ac 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -181,7 +181,7 @@ static const struct {
>   * (hardcoded in spice-server), let's add it here to avoid the warning.
>   */
>  { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
> -  "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
> +  "h264parse ! avdec_h264", "video/x-h264" },
>  
>  /* SPICE_VIDEO_CODEC_TYPE_VP9 */
>  { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Victor Toso
On Thu, Jul 13, 2017 at 01:31:34PM +0200, Pavel Grunt wrote:
> On Thu, 2017-07-13 at 13:22 +0200, Victor Toso wrote:
> > Hi,
> > 
> > On Thu, Jul 13, 2017 at 01:18:14PM +0200, Pavel Grunt wrote:
> > > On Thu, 2017-07-13 at 13:11 +0200, Victor Toso wrote:
> > > > On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> > > > > GStreamer's avdec_h264 needs h264parse to be able to process H264 
> > > > > video
> > > > > streams. However the check for elements through GstRegistry forgot to
> > > > > include the parsers, thus making spice-gtk to not set the relevant cap
> > > > > to inform the server about H264 decoding capability.
> > > > 
> > > > Wouldn't that make it possible to find a parser but not a decoder and
> > > > evaluate the _has_codec() to true?
> > > 
> > > yes, it would
> > > 
> > > so it must be a check for parsers and then searching for decoders 
> > > accepting
> > > the
> > > parser(s)'s output
> > 
> > Not really, avdec_h264 should show when we list the decoders.
> 
> it should not, the caps we set are not a subset of avdec_h264 caps.

A bit confusing but you were right!
https://paste.fedoraproject.org/paste/hx0F3Tbx5W1fDGRelvJCUw

Cheers,

> Documentation of GstCaps may not be clear, but take a look at tests. Basically
> "video/x-raw" is the superset for "video/x-raw, format=(string)YUY2".
> 
> Pavel



> 
> 
> > 
> > > 
> > > Pavel
> > > > > ---
> > > > >  src/channel-display-gst.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > > > > index 54edd6b..d883d9f 100644
> > > > > --- a/src/channel-display-gst.c
> > > > > +++ b/src/channel-display-gst.c
> > > > > @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
> > > > >  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> > > > >  
> > > > >  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> > > > > +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> > > > > GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> > > > > GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
> > > > >  all_decoders = gst_element_factory_list_get_elements(type,
> > > > > GST_RANK_NONE);
> > > > > -- 
> > > > > 2.13.0
> > > > > 
> > > > > ___
> > > > > Spice-devel mailing list
> > > > > Spice-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk v1] display-gst: Improve h264 elements filtering

2017-07-13 Thread Victor Toso
From: Victor Toso 

This patch fixes the avdec_h264 element not being present on
gstvideo_has_codec() which get all decoder elements from GstRegistry
and filter them on our GstCaps in order to get the ones for given
codec.

The issue is around the filtering. The current GstCaps for h264 is not
consider a subset of avdec_h264's capabilites and that will fiter this
element out of the list.

The proposed solution for that is to set `subsetonly` parameter from
gst_element_factory_list_filter() to false.

While at it, the cap "stream-format=byte-stream" is less useful now
because it isn't needed to play the stream - see 6fe88871240c53b8

In my system, our debug shows:
.. gstvideo_debug_available_decoders: From 228 video decoder elements,
- 4 can handle caps   image/jpeg: jpegdec, nvdec, avdec_mjpeg, vaapijpegdec
- 3 can handle caps  video/x-vp8: vaapidecodebin, vp8dec, avdec_vp8
- 4 can handle caps video/x-h264: vaapidecodebin, avdec_h264, nvdec, 
vaapih264dec
- 3 can handle caps  video/x-vp9: vaapidecodebin, vp9dec, avdec_vp9

Signed-off-by: Victor Toso 
---
 src/channel-display-gst.c  | 2 +-
 src/channel-display-priv.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 20d236a..1bd7df1 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -647,7 +647,7 @@ gboolean gstvideo_has_codec(int codec_type)
 }
 
 caps = gst_caps_from_string(gst_opts[codec_type].dec_caps);
-codec_decoders = gst_element_factory_list_filter(all_decoders, caps, 
GST_PAD_SINK, TRUE);
+codec_decoders = gst_element_factory_list_filter(all_decoders, caps, 
GST_PAD_SINK, FALSE);
 gst_caps_unref(caps);
 
 if (codec_decoders == NULL) {
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 04cb4d1..9bfd4ac 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -181,7 +181,7 @@ static const struct {
  * (hardcoded in spice-server), let's add it here to avoid the warning.
  */
 { SPICE_DISPLAY_CAP_CODEC_H264, "h264",
-  "h264parse ! avdec_h264", "video/x-h264,stream-format=byte-stream" },
+  "h264parse ! avdec_h264", "video/x-h264" },
 
 /* SPICE_VIDEO_CODEC_TYPE_VP9 */
 { SPICE_DISPLAY_CAP_CODEC_VP9, "vp9",
-- 
2.13.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Pavel Grunt
On Thu, 2017-07-13 at 13:22 +0200, Victor Toso wrote:
> Hi,
> 
> On Thu, Jul 13, 2017 at 01:18:14PM +0200, Pavel Grunt wrote:
> > On Thu, 2017-07-13 at 13:11 +0200, Victor Toso wrote:
> > > On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> > > > GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> > > > streams. However the check for elements through GstRegistry forgot to
> > > > include the parsers, thus making spice-gtk to not set the relevant cap
> > > > to inform the server about H264 decoding capability.
> > > 
> > > Wouldn't that make it possible to find a parser but not a decoder and
> > > evaluate the _has_codec() to true?
> > 
> > yes, it would
> > 
> > so it must be a check for parsers and then searching for decoders accepting
> > the
> > parser(s)'s output
> 
> Not really, avdec_h264 should show when we list the decoders.

it should not, the caps we set are not a subset of avdec_h264 caps.
Documentation of GstCaps may not be clear, but take a look at tests. Basically
"video/x-raw" is the superset for "video/x-raw, format=(string)YUY2".

Pavel


> 
> > 
> > Pavel
> > > > ---
> > > >  src/channel-display-gst.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > > > index 54edd6b..d883d9f 100644
> > > > --- a/src/channel-display-gst.c
> > > > +++ b/src/channel-display-gst.c
> > > > @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
> > > >  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> > > >  
> > > >  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> > > > +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> > > > GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> > > > GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
> > > >  all_decoders = gst_element_factory_list_get_elements(type,
> > > > GST_RANK_NONE);
> > > > -- 
> > > > 2.13.0
> > > > 
> > > > ___
> > > > Spice-devel mailing list
> > > > Spice-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Victor Toso
Hi,

On Thu, Jul 13, 2017 at 01:18:14PM +0200, Pavel Grunt wrote:
> On Thu, 2017-07-13 at 13:11 +0200, Victor Toso wrote:
> > On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> > > GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> > > streams. However the check for elements through GstRegistry forgot to
> > > include the parsers, thus making spice-gtk to not set the relevant cap
> > > to inform the server about H264 decoding capability.
> > 
> > Wouldn't that make it possible to find a parser but not a decoder and
> > evaluate the _has_codec() to true?
> yes, it would
> 
> so it must be a check for parsers and then searching for decoders accepting 
> the
> parser(s)'s output

Not really, avdec_h264 should show when we list the decoders.

> 
> Pavel
> > > ---
> > >  src/channel-display-gst.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > > index 54edd6b..d883d9f 100644
> > > --- a/src/channel-display-gst.c
> > > +++ b/src/channel-display-gst.c
> > > @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
> > >  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> > >  
> > >  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> > > +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> > > GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> > > GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
> > >  all_decoders = gst_element_factory_list_get_elements(type,
> > > GST_RANK_NONE);
> > > -- 
> > > 2.13.0
> > > 
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Pavel Grunt
On Thu, 2017-07-13 at 13:11 +0200, Victor Toso wrote:
> On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> > GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> > streams. However the check for elements through GstRegistry forgot to
> > include the parsers, thus making spice-gtk to not set the relevant cap
> > to inform the server about H264 decoding capability.
> 
> Wouldn't that make it possible to find a parser but not a decoder and
> evaluate the _has_codec() to true?
yes, it would

so it must be a check for parsers and then searching for decoders accepting the
parser(s)'s output

Pavel
> > ---
> >  src/channel-display-gst.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 54edd6b..d883d9f 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
> >  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> >  
> >  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> > +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> > GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> > GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
> >  all_decoders = gst_element_factory_list_get_elements(type,
> > GST_RANK_NONE);
> > -- 
> > 2.13.0
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Victor Toso
Hi,

On Thu, Jul 13, 2017 at 01:11:00PM +0200, Victor Toso wrote:
> On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> > GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> > streams. However the check for elements through GstRegistry forgot to
> > include the parsers, thus making spice-gtk to not set the relevant cap
> > to inform the server about H264 decoding capability.
> 
> Wouldn't that make it possible to find a parser but not a decoder and
> evaluate the _has_codec() to true?

In my system:

gstvideo_debug_available_decoders: From 241 video decoder elements (...)
  3 can handle caps   image/jpeg: jpegdec, nvdec, vaapijpegdec
  3 can handle caps  video/x-vp8: vaapidecodebin, vp8dec, avdec_vp8
  3 can handle caps video/x-h264,stream-format=byte-stream: vaapidecodebin, 
h264parse, vaapih264dec
  3 can handle caps  video/x-vp9: vaapidecodebin, vp9dec, avdec_vp9

For h264, that's h264parse that shows not avdec_h264. I would take this
as a workaround... we could add that in the commit log and have it for
now...

>
> > ---
> >  src/channel-display-gst.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > index 54edd6b..d883d9f 100644
> > --- a/src/channel-display-gst.c
> > +++ b/src/channel-display-gst.c
> > @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
> >  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
> >  
> >  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> > +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> > GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> > GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
> >  all_decoders = gst_element_factory_list_get_elements(type, 
> > GST_RANK_NONE);
> > -- 
> > 2.13.0
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel



> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Victor Toso
On Thu, Jul 13, 2017 at 01:05:31PM +0200, Pavel Grunt wrote:
> GStreamer's avdec_h264 needs h264parse to be able to process H264 video
> streams. However the check for elements through GstRegistry forgot to
> include the parsers, thus making spice-gtk to not set the relevant cap
> to inform the server about H264 decoding capability.

Wouldn't that make it possible to find a parser but not a decoder and
evaluate the _has_codec() to true?

> ---
>  src/channel-display-gst.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 54edd6b..d883d9f 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
>  g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
>  
>  type = GST_ELEMENT_FACTORY_TYPE_DECODER |
> +   GST_ELEMENT_FACTORY_TYPE_PARSER |
> GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
> GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
>  all_decoders = gst_element_factory_list_get_elements(type, 
> GST_RANK_NONE);
> -- 
> 2.13.0
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [PATCH spice-gtk] gst: Fix detection of h264 stream decoder

2017-07-13 Thread Pavel Grunt
GStreamer's avdec_h264 needs h264parse to be able to process H264 video
streams. However the check for elements through GstRegistry forgot to
include the parsers, thus making spice-gtk to not set the relevant cap
to inform the server about H264 decoding capability.
---
 src/channel-display-gst.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 54edd6b..d883d9f 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -652,6 +652,7 @@ gboolean gstvideo_has_codec(int codec_type)
 g_return_val_if_fail(VALID_VIDEO_CODEC_TYPE(codec_type), FALSE);
 
 type = GST_ELEMENT_FACTORY_TYPE_DECODER |
+   GST_ELEMENT_FACTORY_TYPE_PARSER |
GST_ELEMENT_FACTORY_TYPE_MEDIA_VIDEO |
GST_ELEMENT_FACTORY_TYPE_MEDIA_IMAGE;
 all_decoders = gst_element_factory_list_get_elements(type, GST_RANK_NONE);
-- 
2.13.0

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [spice-gtk v2 2/2] file-xfer: Inform client of errors on init of xfer

2017-07-13 Thread Victor Toso
Pushed upstream,

Shortlog: file-xfer: Inform client of errors on init of xfer
Commit  : c021bb051fac1b66199ea75ce10139f16b081f42
Victor Toso on Thu, 13 Jul 2017 12:23:43 +0200

On Tue, Jul 11, 2017 at 12:42:29PM +0200, Victor Toso wrote:
> From: Victor Toso 
> 
> With SpiceFileTransferTask we suggest that Spice clients watch
> (signal) SpiceMainChannel::new-file-transfer so they can follow the
> transfer's progress till the moment it is finished.
> 
> The signal SpiceFileTransferTask::finished informs if an error happens
> to the application.
> 
> This patch solves the problem of SpiceFileTransferTask::finished not
> being emitted when the agent is not connected nor when file-transfer
> is disabled in the host.
> 
> We should first emit SpiceMainChannel::new-file-transfer followed up
> by SpiceFileTransferTask::finished, which is done by the function
> spice_file_transfer_task_completed().
> 
> As channel-main chains up the "finish" signal by removing the
> SpiceFileTransferTask from its GHashTable, we have to change from
> GHashTableIter to a simple GList of keys.
> 
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1373830
> Signed-off-by: Victor Toso 
> ---
>  src/channel-main.c | 50 ++
>  1 file changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 68ec62e..17f9122 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -3103,9 +3103,9 @@ void spice_main_file_copy_async(SpiceMainChannel 
> *channel,
>  gpointer user_data)
>  {
>  SpiceMainChannelPrivate *c;
> -GHashTableIter iter;
> -gpointer key, value;
>  FileTransferOperation *xfer_op;
> +GError *error = NULL;
> +GList *it, *keys;
>  
>  g_return_if_fail(channel != NULL);
>  g_return_if_fail(SPICE_IS_MAIN_CHANNEL(channel));
> @@ -3113,25 +3113,13 @@ void spice_main_file_copy_async(SpiceMainChannel 
> *channel,
>  
>  c = channel->priv;
>  if (!c->agent_connected) {
> -g_task_report_new_error(channel,
> -callback,
> -user_data,
> -spice_main_file_copy_async,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_FAILED,
> -_("The agent is not connected"));
> -return;
> -}
> -
> -if (test_agent_cap(channel, VD_AGENT_CAP_FILE_XFER_DISABLED)) {
> -g_task_report_new_error(channel,
> -callback,
> -user_data,
> -spice_main_file_copy_async,
> -SPICE_CLIENT_ERROR,
> -SPICE_CLIENT_ERROR_FAILED,
> -_("The file transfer is disabled"));
> -return;
> +error = g_error_new(SPICE_CLIENT_ERROR,
> +SPICE_CLIENT_ERROR_FAILED,
> +_("The agent is not connected"));
> +} else if (test_agent_cap(channel, VD_AGENT_CAP_FILE_XFER_DISABLED)) {
> +error = g_error_new(SPICE_CLIENT_ERROR,
> +SPICE_CLIENT_ERROR_FAILED,
> +_("The file transfer is disabled"));
>  }
>  
>  xfer_op = g_new0(FileTransferOperation, 1);
> @@ -3144,23 +3132,29 @@ void spice_main_file_copy_async(SpiceMainChannel 
> *channel,
> flags,
> cancellable);
>  xfer_op->stats.num_files = g_hash_table_size(xfer_op->xfer_task);
> -g_hash_table_iter_init(, xfer_op->xfer_task);
> -while (g_hash_table_iter_next(, , )) {
> +keys = g_hash_table_get_keys(xfer_op->xfer_task);
> +for (it = keys; it != NULL; it = it->next) {
>  guint32 task_id;
> -SpiceFileTransferTask *xfer_task = value;
> +SpiceFileTransferTask *xfer_task = 
> g_hash_table_lookup(xfer_op->xfer_task, it->data);
>  
>  task_id = spice_file_transfer_task_get_id(xfer_task);
>  
>  SPICE_DEBUG("Insert a xfer task:%u to task list", task_id);
>  
> -g_hash_table_insert(c->file_xfer_tasks, key, xfer_op);
> +g_hash_table_insert(c->file_xfer_tasks, it->data, xfer_op);
>  g_signal_connect(xfer_task, "finished", 
> G_CALLBACK(file_transfer_operation_task_finished), NULL);
>  g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0, 
> xfer_task);
>  
> -spice_file_transfer_task_init_task_async(xfer_task,
> - 
> file_xfer_init_task_async_cb,
> - xfer_op);
> +if (error == NULL) {
> +spice_file_transfer_task_init_task_async(xfer_task,
> + 

Re: [Spice-devel] [PATCH spice-gtk] display-gst: fix format error

2017-07-13 Thread Victor Toso
Hi,

On Thu, Jul 13, 2017 at 12:05:40PM +0200, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> ../../src/channel-display-gst.c: In function 'handle_pipeline_message':
> ../../src/channel-display-gst.c:289:75: error: format '%ld' expects argument 
> of type 'long int', but argument 2 has type 'gint64 {aka long long int}' 
> [-Werror=format=]
>  gchar *filename = 
> g_strdup_printf("spice-gtk-gst-pipeline-debug-%ld-%s",
>  ~~^
>  %I64d
>
> get_stream_id_by_stream(decoder->base.stream->channel,
>
> ~~
>
> decoder->base.stream),
>
> ~

oops :)

> 
> Signed-off-by: Marc-André Lureau 
Acked-by: Victor Toso 

> ---
>  src/channel-display-gst.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 3cf451b..20d236a 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -286,7 +286,7 @@ static gboolean handle_pipeline_message(GstBus *bus, 
> GstMessage *msg, gpointer v
>  break;
>  }
>  case GST_MESSAGE_STREAM_START: {
> -gchar *filename = 
> g_strdup_printf("spice-gtk-gst-pipeline-debug-%ld-%s",
> +gchar *filename = g_strdup_printf("spice-gtk-gst-pipeline-debug-%" 
> G_GINT64_FORMAT "-%s",
>
> get_stream_id_by_stream(decoder->base.stream->channel,
>
> decoder->base.stream),
>
> gst_opts[decoder->base.codec_type].name);
> -- 
> 2.13.1.395.gf7b71de06
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v2] win-usb-dev: fix device arrival event logic

2017-07-13 Thread Marc-André Lureau
On Wed, Jul 12, 2017 at 10:02 AM Uri Lublin  wrote:

> Hi Yuri,
>
> I'd mention in the Subject/Log that a device is now identified
> by its bus.addr instead of vid:pid.
>
> On 07/05/2017 08:21 AM, Yuri Benditovich wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1425961
> > If attached new device when one device with the same vid
> > and pid already present, the notification is ignored and
> > attached device is not redirected (if auto share set) and
> > not displayed in USB devices widget.
> >
> > This commit reverts older commit f9631cd6f8, which was
> > intended to solve problem when bus:addr is sometimes changed when
> > using WinUSB. The same commit creates the limitation for automatic
> > redirection of second device with the same pid:vid.
> > Now the preferred backend for Spice-GTK on Windows is UsbDk.
> > In case users of newer WinUSB will still need backward compatible
> > behavior, consider backend-aware comparison procedure.
> >
> > Signed-off-by: Yuri Benditovich 
>
> Acked-by: Uri Lublin 
>
>
pushed with commit comment addition:

commit de41d93285461264d7d1c14a1e649433d3911da6
Author: Yuri Benditovich 
Date:   Wed Jul 5 08:21:46 2017 +0300

win-usb-dev: fix device arrival event logic

Thanks,
>  Uri.
>
> > ---
> >   src/win-usb-dev.c | 12 ++--
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> > index ec3dd91..e2d77b3 100644
> > --- a/src/win-usb-dev.c
> > +++ b/src/win-usb-dev.c
> > @@ -380,20 +380,20 @@ static gboolean get_usb_dev_info(libusb_device
> *dev, GUdevDeviceInfo *udevinfo)
> >   return TRUE;
> >   }
> >
> > -/* Only vid:pid are compared */
> > +/* Only bus:addr are compared */
> >   static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
> >   {
> >   GUdevDeviceInfo *ai, *bi;
> > -gboolean same_vid;
> > -gboolean same_pid;
> > +gboolean same_bus;
> > +gboolean same_addr;
> >
> >   ai = G_UDEV_DEVICE(a)->priv->udevinfo;
> >   bi = G_UDEV_DEVICE(b)->priv->udevinfo;
> >
> > -same_vid  = (ai->vid == bi->vid);
> > -same_pid  = (ai->pid == bi->pid);
> > +same_bus = (ai->bus == bi->bus);
> > +same_addr = (ai->addr == bi->addr);
> >
> > -return (same_pid && same_vid) ? 0 : -1;
> > +return (same_bus && same_addr) ? 0 : -1;
> >   }
> >
> >   static void notify_dev_state_change(GUdevClient *self,
> >
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v3] Make error messages translatable

2017-07-13 Thread Victor Toso
Hi,

In my comments below, I assume that if we are using a message for debug
purposes, we don't need to translate it. That is, if we GError* will not
be exported to the application.

Let me know if my assumption is wrong.

On Tue, Jul 11, 2017 at 03:43:41PM +0200, Pavel Grunt wrote:
> Signed-off-by: Victor Toso 
> Signed-off-by: Pavel Grunt 
> ---
>  po/POTFILES.in | 13 +
>  src/channel-main.c | 15 ++-
>  src/channel-port.c |  4 +++-
>  src/channel-usbredir.c |  8 
>  src/giopipe.c  |  7 +--
>  src/smartcard-manager.c|  5 +++--
>  src/spice-channel.c|  2 +-
>  src/spice-client-glib-usb-acl-helper.c |  2 +-
>  src/spice-file-transfer-task.c |  6 --
>  src/spice-pulse.c  |  8 +---
>  src/spice-session.c|  2 +-
>  src/spice-uri.c| 17 ++---
>  src/spice-widget-egl.c | 23 ---
>  src/usb-acl-helper.c   |  9 +
>  src/usb-device-manager.c   |  6 +++---
>  src/vmcstream.c|  3 ++-
>  src/win-usb-dev.c  |  8 
>  src/wocky-http-proxy.c | 11 ++-
>  18 files changed, 92 insertions(+), 57 deletions(-)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index d1033f9..7a744af 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -1,9 +1,22 @@
>  src/channel-main.c
> +src/channel-port.c
>  src/channel-usbredir.c
>  src/desktop-integration.c
> +src/giopipe.c
> +src/smartcard-manager.c
>  src/spice-channel.c
> +src/spice-client-glib-usb-acl-helper.c
> +src/spice-file-transfer-task.c
>  src/spice-option.c
> +src/spice-pulse.c
> +src/spice-session.c
> +src/spice-uri.c
> +src/spice-widget-egl.c
> +src/usb-acl-helper.c
>  src/usb-device-manager.c
>  src/usb-device-widget.c
>  src/usbutil.c
> +src/vmcstream.c
> +src/win-usb-dev.c
> +src/wocky-http-proxy.c
>  tools/spice-cmdline.c
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 4edd575..c035ce1 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -17,6 +17,7 @@
>  */
>  #include "config.h"
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1900,7 +1901,7 @@ static void 
> main_agent_handle_xfer_status(SpiceMainChannel *channel,
>  default:
>  g_warn_if_reached();
>  error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"unhandled status type: %u", msg->result);
> +_("unhandled status type: %u"), msg->result);
>  break;
>  }
>  
> @@ -2906,12 +2907,16 @@ failed:
>  
>  static void file_transfer_operation_free(FileTransferOperation *xfer_op)
>  {
> +const char *error_summary_fmt;
>  g_return_if_fail(xfer_op != NULL);
>  
> +error_summary_fmt = ngettext(_("Transferring %u file: %u succeed, %u 
> cancelled, %u failed"),
> + _("Transferring %u files: %u succeed, %u 
> cancelled, %u failed"),
> + xfer_op->stats.num_files);
>  if (xfer_op->stats.failed != 0) {
>  GError *error = g_error_new(SPICE_CLIENT_ERROR,
>  SPICE_CLIENT_ERROR_FAILED,
> -"Transferring %u files: %u succeed, %u 
> cancelled, %u failed",
> +error_summary_fmt,
>  xfer_op->stats.num_files,
>  xfer_op->stats.succeed,
>  xfer_op->stats.cancelled,
> @@ -2921,7 +2926,7 @@ static void 
> file_transfer_operation_free(FileTransferOperation *xfer_op)
>  } else if (xfer_op->stats.cancelled != 0 && xfer_op->stats.succeed == 0) 
> {
>  GError *error = g_error_new(G_IO_ERROR,
>  G_IO_ERROR_CANCELLED,
> -"Transferring %u files: %u succeed, %u 
> cancelled, %u failed",
> +error_summary_fmt,
>  xfer_op->stats.num_files,
>  xfer_op->stats.succeed,
>  xfer_op->stats.cancelled,
> @@ -2962,7 +2967,7 @@ static void 
> spice_main_channel_reset_all_xfer_operations(SpiceMainChannel *chann
>  }
>  
>  error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> -"Agent connection closed");
> +_("Agent connection closed"));
>  spice_file_transfer_task_completed(xfer_task, error);
>  }
>  g_list_free(keys);
> @@ -3119,7 +3124,7 @@ void spice_main_file_copy_async(SpiceMainChannel 
> *channel,
>  

[Spice-devel] [PATCH spice-gtk] display-gst: fix format error

2017-07-13 Thread marcandre . lureau
From: Marc-André Lureau 

../../src/channel-display-gst.c: In function 'handle_pipeline_message':
../../src/channel-display-gst.c:289:75: error: format '%ld' expects argument of 
type 'long int', but argument 2 has type 'gint64 {aka long long int}' 
[-Werror=format=]
 gchar *filename = 
g_strdup_printf("spice-gtk-gst-pipeline-debug-%ld-%s",
 ~~^
 %I64d
   
get_stream_id_by_stream(decoder->base.stream->channel,
   
~~
   
decoder->base.stream),
   
~

Signed-off-by: Marc-André Lureau 
---
 src/channel-display-gst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 3cf451b..20d236a 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -286,7 +286,7 @@ static gboolean handle_pipeline_message(GstBus *bus, 
GstMessage *msg, gpointer v
 break;
 }
 case GST_MESSAGE_STREAM_START: {
-gchar *filename = 
g_strdup_printf("spice-gtk-gst-pipeline-debug-%ld-%s",
+gchar *filename = g_strdup_printf("spice-gtk-gst-pipeline-debug-%" 
G_GINT64_FORMAT "-%s",
   
get_stream_id_by_stream(decoder->base.stream->channel,
   
decoder->base.stream),
   
gst_opts[decoder->base.codec_type].name);
-- 
2.13.1.395.gf7b71de06

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] spice-protocol 0.12.13 missing signature

2017-07-13 Thread Christophe Fergeau
On Wed, Jul 12, 2017 at 03:20:53PM -0400, Leo Famulari wrote:
> Hello,
> 
> I noticed the release of spice-protocol 0.12.13 doesn't seem to have an
> associated signature file, which is different from the last few years of
> releases.
> 
> Is this a change in policy for spice-protocol?

I believe I'm the only one adding signature files to the releases, and I
did not do that one, I would not call that "a change in policy". I've
uploaded one now after checking the tarball content matches the 0.12.13
git tag.
It's signed with my GPG key, 4096R/29AC6C82

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] Fwd: [Spice-commits] 5 commits - autogen.sh configure.ac Makefile.am NEWS po/fr.po po/it.po po/Makevars README src/win-usb-dev.c

2017-07-13 Thread Marc-André Lureau
Hi

I pushed my pending build-sys/gettext series, instead of a single patch.
The v1 was already reviewed, and I was going to send a v2 for final
comments. Since it's build-sys changes already reviewed, let's avoid the
churn of revert, and you can have a look at the commits for additional
fixup. Sorry for the mess.

-- Forwarded message -
From: Marc-André Lureau 
Date: Thu, Jul 13, 2017 at 11:57 AM
Subject: [Spice-commits] 5 commits - autogen.sh configure.ac Makefile.am
NEWS po/fr.po po/it.po po/Makevars README src/win-usb-dev.c
To: 


 Makefile.am   |   46 
 NEWS  |   28 ++
 README|4
 autogen.sh|1
 configure.ac  |8
 po/Makevars   |   95 +
 po/fr.po  |  522
++
 po/it.po  |  262 +++
 src/win-usb-dev.c |   12 -
 9 files changed, 667 insertions(+), 311 deletions(-)

New commits:
commit de41d93285461264d7d1c14a1e649433d3911da6
Author: Yuri Benditovich 
Date:   Wed Jul 5 08:21:46 2017 +0300

win-usb-dev: fix device arrival event logic

https://bugzilla.redhat.com/show_bug.cgi?id=1425961
If attached new device when one device with the same vid
and pid already present, the notification is ignored and
attached device is not redirected (if auto share set) and
not displayed in USB devices widget.

This commit reverts older commit f9631cd6f8, which was
intended to solve problem when bus:addr is sometimes changed when
using WinUSB. The same commit creates the limitation for automatic
redirection of second device with the same pid:vid.
Now the preferred backend for Spice-GTK on Windows is UsbDk.
In case users of newer WinUSB will still need backward compatible
behavior, consider backend-aware comparison procedure.

A device is now identified again by its bus.addr instead of vid:pid.

Signed-off-by: Yuri Benditovich 
Message-Id: <
1499232106-16448-1-git-send-email-yuri.benditov...@daynix.com>
Acked-by: Uri Lublin 
[ Marc-André - Added commit comment from Uri's review. ]
Signed-off-by: Marc-André Lureau 

diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
index ec3dd91..e2d77b3 100644
--- a/src/win-usb-dev.c
+++ b/src/win-usb-dev.c
@@ -380,20 +380,20 @@ static gboolean get_usb_dev_info(libusb_device *dev,
GUdevDeviceInfo *udevinfo)
 return TRUE;
 }

-/* Only vid:pid are compared */
+/* Only bus:addr are compared */
 static gint gudev_devices_differ(gconstpointer a, gconstpointer b)
 {
 GUdevDeviceInfo *ai, *bi;
-gboolean same_vid;
-gboolean same_pid;
+gboolean same_bus;
+gboolean same_addr;

 ai = G_UDEV_DEVICE(a)->priv->udevinfo;
 bi = G_UDEV_DEVICE(b)->priv->udevinfo;

-same_vid  = (ai->vid == bi->vid);
-same_pid  = (ai->pid == bi->pid);
+same_bus = (ai->bus == bi->bus);
+same_addr = (ai->addr == bi->addr);

-return (same_pid && same_vid) ? 0 : -1;
+return (same_bus && same_addr) ? 0 : -1;
 }

 static void notify_dev_state_change(GUdevClient *self,
commit 9017f58c7f0147a773559b4ebac3a811f024ad31
Author: Marc-André Lureau 
Date:   Thu Jun 15 17:44:38 2017 +0400

Release notes for v0.34

Signed-off-by: Marc-André Lureau 

diff --git a/NEWS b/NEWS
index 989f012..e691583 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,31 @@
+v0.34
+=
+
+- NOTE: this is the last release with the spice-controller library
+- add VP9 codec support
+- API: add spice_display_change_preferred_video_codec_type()
+- API: add new SpiceCursorChannel:cursor property, deprecate "cursor-set"
signal
+- API: spice_audio_new() is no longer in public header (it was deprecated
+  for a long while)
+- fix clipboard crash and other regressions from 0.33
+- report invalid or stopped streams to the server
+- use playbin instead of decodebin with gstreamer > 1.9
+- support GST_DEBUG_BIN_TO_DOT_FILE debug
+- deprecate a few esoteric options from --spice group:
+  --spice-color-depth, --spice-cache-size, --spice-glz-window-size used
+  mainly for development. They may be available with spicy in the future.
+- win32: handle failures when starting win-usb manager
+- win32: removed windows usb-clerk support, replaced by UsbDk
+- win32: fix alt-tab & grab issues
+- spicy learned to tweak codec preference, cancel transfer, and resize
+  precisely for debugging purposes
+- use keycodemapdb submodule, drop perl(Text::CSV) dependency
+- file-xfer: fix bad filename encoding
+- file-xfer: handle new error kind
+- build-sys fixes for macos
+- replace some deprecated gtk code
+- memory leak fixes, new tests
+
 v0.33
 =

commit 17593eedd3e1d0d717bbaf204c834342c2e2e20a
Author: Marc-André Lureau 
Date:   Wed Jul 12 

Re: [Spice-devel] [PATCH spice-gtk v2] Make error messages translatable

2017-07-13 Thread Victor Toso
Hi,

On Tue, Jul 11, 2017 at 01:13:44PM +0200, Victor Toso wrote:
> Hi,
>
> On Tue, Jul 11, 2017 at 09:21:53AM +0200, Pavel Grunt wrote:
> > ---
> > v2:
> >  - Use ngettext for error messages in plural
> >  - rebased on top of Victor's patch
>
> Looks good
> Acked-by: Victor Toso 

As discussed on IRC, feel free to squash both patches and resend when
you have the new changes done. I'll rebase and push the first patch.

Cheers,


> > ---
> >  po/POTFILES.in | 4 
> >  src/channel-main.c | 9 +++--
> >  src/channel-port.c | 4 +++-
> >  src/channel-usbredir.c | 2 +-
> >  src/smartcard-manager.c| 5 +++--
> >  src/spice-channel.c| 2 +-
> >  src/spice-file-transfer-task.c | 6 --
> >  src/spice-pulse.c  | 4 +++-
> >  8 files changed, 26 insertions(+), 10 deletions(-)
> > 
> > diff --git a/po/POTFILES.in b/po/POTFILES.in
> > index d1033f9..5f438b5 100644
> > --- a/po/POTFILES.in
> > +++ b/po/POTFILES.in
> > @@ -1,8 +1,12 @@
> >  src/channel-main.c
> > +src/channel-port.c
> >  src/channel-usbredir.c
> >  src/desktop-integration.c
> > +src/smartcard-manager.c
> >  src/spice-channel.c
> > +src/spice-file-transfer-task.c
> >  src/spice-option.c
> > +src/spice-pulse.c
> >  src/usb-device-manager.c
> >  src/usb-device-widget.c
> >  src/usbutil.c
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 68ec62e..c035ce1 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -17,6 +17,7 @@
> >  */
> >  #include "config.h"
> >  
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -2906,12 +2907,16 @@ failed:
> >  
> >  static void file_transfer_operation_free(FileTransferOperation *xfer_op)
> >  {
> > +const char *error_summary_fmt;
> >  g_return_if_fail(xfer_op != NULL);
> >  
> > +error_summary_fmt = ngettext(_("Transferring %u file: %u succeed, %u 
> > cancelled, %u failed"),
> > + _("Transferring %u files: %u succeed, %u 
> > cancelled, %u failed"),
> > + xfer_op->stats.num_files);
> >  if (xfer_op->stats.failed != 0) {
> >  GError *error = g_error_new(SPICE_CLIENT_ERROR,
> >  SPICE_CLIENT_ERROR_FAILED,
> > -_("Transferring %u files: %u succeed, 
> > %u cancelled, %u failed"),
> > +error_summary_fmt,
> >  xfer_op->stats.num_files,
> >  xfer_op->stats.succeed,
> >  xfer_op->stats.cancelled,
> > @@ -2921,7 +2926,7 @@ static void 
> > file_transfer_operation_free(FileTransferOperation *xfer_op)
> >  } else if (xfer_op->stats.cancelled != 0 && xfer_op->stats.succeed == 
> > 0) {
> >  GError *error = g_error_new(G_IO_ERROR,
> >  G_IO_ERROR_CANCELLED,
> > -_("Transferring %u files: %u succeed, 
> > %u cancelled, %u failed"),
> > +error_summary_fmt,
> >  xfer_op->stats.num_files,
> >  xfer_op->stats.succeed,
> >  xfer_op->stats.cancelled,
> > diff --git a/src/channel-port.c b/src/channel-port.c
> > index d922e4b..9fb7f05 100644
> > --- a/src/channel-port.c
> > +++ b/src/channel-port.c
> > @@ -17,6 +17,8 @@
> >  */
> >  #include "config.h"
> >  
> > +#include 
> > +
> >  #include "spice-client.h"
> >  #include "spice-common.h"
> >  #include "spice-channel-priv.h"
> > @@ -294,7 +296,7 @@ void spice_port_write_async(SpicePortChannel *self,
> >  g_task_report_new_error(self, callback,
> >  user_data, spice_port_write_async,
> >  SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
> > -"The port is not opened");
> > +_("The port is not opened"));
> >  return;
> >  }
> >  
> > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > index fef62ce..f8a5234 100644
> > --- a/src/channel-usbredir.c
> > +++ b/src/channel-usbredir.c
> > @@ -348,7 +348,7 @@ static void spice_usbredir_channel_open_acl_cb(
> >  spice_usb_acl_helper_open_acl_finish(acl_helper, acl_res, );
> >  if (!err && priv->state == STATE_DISCONNECTING) {
> >  err = g_error_new_literal(G_IO_ERROR, G_IO_ERROR_CANCELLED,
> > -  "USB redirection channel connect 
> > cancelled");
> > +  _("USB redirection channel connect 
> > cancelled"));
> >  }
> >  if (!err) {
> >  spice_usbredir_channel_open_device(channel, );
> > diff --git a/src/smartcard-manager.c b/src/smartcard-manager.c
> > index 708f976..05e113d 100644
> > --- a/src/smartcard-manager.c
> > +++ b/src/smartcard-manager.c
> > @@ -18,6 +18,7 @@
> >  #include "config.h"
> >  
> >  

[Spice-devel] spice-protocol 0.12.13 missing signature

2017-07-13 Thread Leo Famulari
Hello,

I noticed the release of spice-protocol 0.12.13 doesn't seem to have an
associated signature file, which is different from the last few years of
releases.

Is this a change in policy for spice-protocol?

Leo


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] Consult:how to install Spice in Openstack?

2017-07-13 Thread 李比
Hello guys!Sorry to interrupt you.I want to remote access to VM through
spice protocol instead of RDP protocol .I have read the offical website
about how to install SPICE throuth virt-manager or libvirt or QEMU.But the
content doesn`t refer to how to integrate SPICE with Openstack.Do you know
how to install Spice protocol in Openstack?

Much Appreciate!Looking forward to your reply.








   Sincerely,

  Bi,Li
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel