[Spice-devel] [PATCH linux vdagent] Add systemd socket activation
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
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
- 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
From: Christophe de DinechinThe 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
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
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
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
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
- 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 Fergeauack > --- > 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
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
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
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
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
- 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
From: Christophe de DinechinThis 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
From: Christophe de DinechinThe 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
From: Christophe de DinechinExperience 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
From: Christophe de DinechinThese 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
From: Christophe de DinechinThe 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
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
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
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
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
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
From: Victor TosoThis 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
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
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
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
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
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
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
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
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
On Wed, Jul 12, 2017 at 10:02 AM Uri Lublinwrote: > 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
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
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
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
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é LureauDate: 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
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 TosoAs 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
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?
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