Re: [Spice-devel] [vdagent-win PATCH 00/13] Miscellaneous minor patches for the Windows Agent

2018-05-28 Thread Christophe Fergeau
ACK to patches 01 through 07 with the minor comments addressed Christophe On Mon, May 28, 2018 at 09:57:53AM +0100, Frediano Ziglio wrote: > Includes: > - minor cleanups; > - project updates; > - warning removals; > - possible minor buffer overflows; > - code style updates. > > Frediano Ziglio

Re: [Spice-devel] [vdagent-win PATCH 13/13] build: Base cmake support

2018-05-28 Thread Christophe Fergeau
Lacking some rationale in the commit log I think ;) vdagent-win already has 2 build systems (mingw+autotools and VS), there are patches to port other spice components to meson, so why cmake here? Christophe On Mon, May 28, 2018 at 09:58:06AM +0100, Frediano Ziglio wrote: > Signed-off-by:

Re: [Spice-devel] [vdagent-win PATCH 07/13] file_xfer: Remove too C syntax for C++

2018-05-28 Thread Christophe Fergeau
I would be a bit more verbose in the commit log, just state that it's about removing a struct typedef as in C++ simply declaring the struct will create both the 'struct Foo' and 'Foo' types. Christophe On Mon, May 28, 2018 at 09:58:00AM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano

Re: [Spice-devel] [vdagent-win PATCH 06/13] file_xfer: Remove FileXferTask structure alignment

2018-05-28 Thread Christophe Fergeau
On Mon, May 28, 2018 at 09:57:59AM +0100, Frediano Ziglio wrote: > There's no reason beside loosing performances to align > that structure, is not passed as binary data. 'losing', not 'loosing' > > Signed-off-by: Frediano Ziglio > --- > vdagent/file_xfer.h | 4 ++-- > 1

Re: [Spice-devel] [vdagent-win PATCH 05/13] msi: Do not generate deps.txt

2018-05-28 Thread Christophe Fergeau
On Mon, May 28, 2018 at 09:57:58AM +0100, Frediano Ziglio wrote: > There's no reason to tell the package installed on the build system > used. This can be useful to know which version of zlib or libpng are bundled with the agent msi installer when debugging something which is installed on a

Re: [Spice-devel] [vdagent-win PATCH 03/13] Fix minor compiler compatibility

2018-05-28 Thread Christophe Fergeau
On Mon, May 28, 2018 at 09:57:56AM +0100, Frediano Ziglio wrote: > Assure std::min is declared including directly algorithm header. "ensure" ? > Undefine possible min and max macros, some Windows headers define them. > Currently happens using Visual Studio 2015. Huhu, not nice :(( Christophe

Re: [Spice-devel] [PATCH spice-gtk v2 1/3] channel-display-gst: Prevent accumulating output queue

2018-05-28 Thread Christophe Fergeau
On Fri, May 25, 2018 at 01:55:20PM +0100, Frediano Ziglio wrote: > display_queue is queued with decoded frames ready to be displayed. > However current code can insert a timeout before displaying and > removing the queued frames. As the frames are not compressed the > usage of memory by this queue

Re: [Spice-devel] [spice-gtk] widget: avoid gdk_seat_grab/ungrab() API temporarily

2018-05-28 Thread Christophe Fergeau
On Fri, May 25, 2018 at 04:00:43PM +0200, Victor Toso wrote: > Hi, > > Quick update, this will not be needed anymore, yay. > https://gitlab.gnome.org/GNOME/gtk/merge_requests/166 > > Upstream bug with lots of information: > https://gitlab.gnome.org/GNOME/gtk/issues/1073 Ah great to read!

Re: [Spice-devel] [vdagent-win PATCH v2] Avoid to use names with reserved characters.

2018-05-25 Thread Christophe Fergeau
gt;", """ and "|" are reserved for shell usage. > > More information on "Naming Files, Paths, and Namespaces" page at > https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx > > This fixes also https://bugzilla.redhat.com/show_bug.c

Re: [Spice-devel] [spice-server v2] sound: Don't mute recording when client reconnects

2018-05-25 Thread Christophe Fergeau
On Fri, May 25, 2018 at 07:33:56AM -0400, Frediano Ziglio wrote: > > +static gboolean playback_channel_client_initable_init(GInitable *initable, > > + GCancellable > > *cancellable, > > +

Re: [Spice-devel] [PATCH spice-common] fixup! meson build

2018-05-25 Thread Christophe Fergeau
On Thu, May 24, 2018 at 02:15:17PM -0300, Eduardo Lima (Etrunko) wrote: > On 24/05/18 12:53, Jonathon Jongsma wrote: > > On Thu, 2018-05-24 at 12:41 -0300, Eduardo Lima (Etrunko) wrote: > >> On 24/05/18 12:31, Jonathon Jongsma wrote: > >>> On Wed, 2018-05-23 at 15:23 -0300, Eduardo Lima (Etrunko)

[Spice-devel] [spice-server v2] sound: Don't mute recording when client reconnects

2018-05-25 Thread Christophe Fergeau
and then disconnecting/reconnecting the client does not successfully reenable recording. This is a regression introduced by commit d8dc09 'sound: Convert SndChannelClient to RedChannelClient' https://bugzilla.redhat.com/show_bug.cgi?id=1549132 Signed-off-by: Christophe Fergeau <cferg...@redhat.com> -

Re: [Spice-devel] [vdagent-win PATCH] Avoid to use names with reserved characters.

2018-05-25 Thread Christophe Fergeau
Hey, On Thu, May 24, 2018 at 02:16:06PM +0100, Frediano Ziglio wrote: > Some characters are reserved and should not be used in Windows > independently by the file system used. > This avoid to use paths in the filename which could lead to some > nasty hacks (like names like "..\hack.txt"). > The

Re: [Spice-devel] [vdagent-win PATCH] vdagent: Removed unused declaration

2018-05-25 Thread Christophe Fergeau
Sure, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Thu, May 24, 2018 at 02:02:22PM +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio <fzig...@redhat.com> > --- > vdagent/vdagent.cpp | 1 - > 1 file changed, 1 deletion(-) > > diff --git a

Re: [Spice-devel] [PATCH spice-common v2 1/2] lz: Optimise SAME_PIXEL for RGB16

2018-05-25 Thread Christophe Fergeau
Looks good to me, though 16bpp images are probably not used a lot these days ;) Acked-by: Christophe Fergeau <cferg...@redhat.com> Christophe On Thu, May 24, 2018 at 05:15:28PM +0100, Frediano Ziglio wrote: > Do not extract all components and compare one by one, can be easily &

Re: [Spice-devel] [spice-server] sound: Don't mute recording when client reconnects

2018-05-24 Thread Christophe Fergeau
On Thu, May 24, 2018 at 05:06:19AM -0400, Frediano Ziglio wrote: > > > > On Wed, May 23, 2018 at 01:36:35PM -0400, Frediano Ziglio wrote: > > > Testing this, so far is working correctly: > > > > > > Subject: [PATCH spice-server] rcc: Make red_channel_client_is_connected > > > return > > >

Re: [Spice-devel] [spice-server 1/2] worker: Use more local vars in dev_create_primary_surface

2018-05-24 Thread Christophe Fergeau
o account patch 2/2. > The only strong comment is about the fact that this patch does > not remove the upcasts but more reduce them. Ok, changed it to « This commit also adds a new 'channel' local variable to limit the number of upcasts to RedChannel. » > > > > > > &g

Re: [Spice-devel] [spice-server 1/2] worker: Use more local vars in dev_create_primary_surface

2018-05-24 Thread Christophe Fergeau
. Yes, was a bit worried by that, I can keep explicit RED_CHANNEL(display) rather than adding the 'channel' var as this makes it clearer. Christophe > > > > > Signed-off-by: Christophe Fergeau <cferg...@redhat.com> > > --- > > server/red-worker.c | 8 > &g

[Spice-devel] [spice-server 2/2] worker: Remove display_is_connected()

2018-05-24 Thread Christophe Fergeau
It's only called once, and when it's called, we will have dereferenced worker->display_channel a few lines before, so this cannot be NULL. The if (worker->display_channel) check can thus be removed, so display_is_connected() becomes just red_channel_is_connected(). Signed-off-by: Chri

[Spice-devel] [spice-server 1/2] worker: Use more local vars in dev_create_primary_surface

2018-05-24 Thread Christophe Fergeau
There's already a 'display' variable equal to worker->display_channel which is not consistently used. This commit also adds a new 'channel' local variable to remove upcasts to RedChannel. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-worker.c | 8 1 fil

Re: [Spice-devel] [spice-server] sound: Don't mute recording when client reconnects

2018-05-24 Thread Christophe Fergeau
On Wed, May 23, 2018 at 01:36:35PM -0400, Frediano Ziglio wrote: > Testing this, so far is working correctly: > > Subject: [PATCH spice-server] rcc: Make red_channel_client_is_connected return > correct information during initialization > > Use stream->watch as flag to check if connected or not

[Spice-devel] [spice-server] sound: Don't mute recording when client reconnects

2018-05-23 Thread Christophe Fergeau
-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/sound.c | 37 ++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/server/sound.c b/server/sound.c index e3891d2cc..d461b9272 100644 --- a/server/sound.c +++ b/server/sound.c @@ -105,7 +

Re: [Spice-devel] [PATCH spice-streaming-agent v2 3/5] Starts/stops the agent based on VT status

2018-05-22 Thread Christophe Fergeau
On Tue, May 22, 2018 at 07:46:48AM -0400, Frediano Ziglio wrote: > > > > On Mon, May 21, 2018 at 11:45:29AM +0100, Frediano Ziglio wrote: > > > Check if the current display server is active or not and stream only if > > > active. > > > This will allow to support multiple display servers running.

Re: [Spice-devel] [PATCH spice-streaming-agent v2 1/5] Install udev rule

2018-05-22 Thread Christophe Fergeau
On Tue, May 22, 2018 at 07:11:15AM -0400, Frediano Ziglio wrote: > > > > On Mon, May 21, 2018 at 11:45:27AM +0100, Frediano Ziglio wrote: > > > The udev rule is used to do some action when the device is added to the > > > system. Current rule change the permission of the special file to allow to

Re: [Spice-devel] [PATCH spice-streaming-agent v2 3/5] Starts/stops the agent based on VT status

2018-05-22 Thread Christophe Fergeau
On Mon, May 21, 2018 at 11:45:29AM +0100, Frediano Ziglio wrote: > Check if the current display server is active or not and stream only if > active. > This will allow to support multiple display servers running. > When multiple display servers are running only one have the GPU > associated and is

Re: [Spice-devel] [PATCH spice-streaming-agent v2 1/5] Install udev rule

2018-05-22 Thread Christophe Fergeau
On Mon, May 21, 2018 at 11:45:27AM +0100, Frediano Ziglio wrote: > The udev rule is used to do some action when the device is added to the > system. Current rule change the permission of the special file to allow to > open it by any user. I thought this was a temporary 'hack' until we have a

Re: [Spice-devel] [spice-gtk] widget: update comment on fixed gtk+ bug

2018-05-22 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, May 21, 2018 at 02:20:14PM +0200, Victor Toso wrote: > From: Victor Toso <m...@victortoso.com> > > The bug was fixed in GTK+ 3.22. I'm updating the comment plus setting > the !GTK_CHECK_VERSION() to track cha

Re: [Spice-devel] [PATCH spice-common v2] Fix generation of Smartcard channel

2018-05-17 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Wed, May 16, 2018 at 06:00:35PM +0100, Frediano Ziglio wrote: > The Smartcard channel definition has been always broken. > Multiple client messages with the same ID are defined in the channel. > This cause on server demarshall

Re: [Spice-devel] [PATCH spice-common] build: Remove FIXME_SERVER_SMARTCARD hack

2018-05-17 Thread Christophe Fergeau
On Thu, May 10, 2018 at 10:34:11AM -0300, Eduardo Lima (Etrunko) wrote: > Ping. > > The meson port depends on this patch. I tried the testing procedure > described in the link below, but could not make it work. Has anyone ever > made it work? I could use some help here. Last time I tried it was

Re: [Spice-devel] [PATCH spice-common 3/4] Check for messages with duplicate names inside a channel

2018-05-16 Thread Christophe Fergeau
On Wed, May 16, 2018 at 12:24:06PM -0400, Frediano Ziglio wrote: > > > > On Wed, May 16, 2018 at 11:04:17AM -0400, Frediano Ziglio wrote: > > > > > > > > On Mon, May 14, 2018 at 11:18:53PM +0100, Frediano Ziglio wrote: > > > > > Make sure there are not 2 messages with the same name in the > > >

Re: [Spice-devel] [PATCH spice-common 3/4] Check for messages with duplicate names inside a channel

2018-05-16 Thread Christophe Fergeau
On Wed, May 16, 2018 at 11:04:17AM -0400, Frediano Ziglio wrote: > > > > On Mon, May 14, 2018 at 11:18:53PM +0100, Frediano Ziglio wrote: > > > Make sure there are not 2 messages with the same name in the > > > same channel. > > > > > > Signed-off-by: Frediano Ziglio > > >

Re: [Spice-devel] [PATCH spice-common 4/4] Check for messages with duplicate values inside a channel

2018-05-16 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, May 14, 2018 at 11:18:54PM +0100, Frediano Ziglio wrote: > Make sure there are not 2 messages with the same value in the > same channel. > > Signed-off-by: Frediano Ziglio <fzig...@redhat.com> > --- >

Re: [Spice-devel] [PATCH spice-common 3/4] Check for messages with duplicate names inside a channel

2018-05-16 Thread Christophe Fergeau
ages_byname[m.name].name, m.name, > self.name)) I believe this will repeat twice the same name "between xxx and xxx in channel ..". You can only mention the duplicate name once. Looks good otherwise. Acked-by: Christophe Fergeau <cferg...@redhat.com> Christophe signature.asc

Re: [Spice-devel] [PATCH spice-common 2/4] codegen: Remove duplicate client and server code from ChannelType::resolve

2018-05-16 Thread Christophe Fergeau
On Mon, May 14, 2018 at 11:18:52PM +0100, Frediano Ziglio wrote: > Code that handled client and server messages check was the same, just > changed some variable names. > Instead use a class to store same information and reuse the code. > This allows easier extension of the 2 path of code. > >

Re: [Spice-devel] [PATCH spice-common 1/4] Fix generation of Smartcard channel

2018-05-16 Thread Christophe Fergeau
On Mon, May 14, 2018 at 11:18:51PM +0100, Frediano Ziglio wrote: > The Smartcard channel definition has been always broken. > Multiple client messages with the same ID are defined in the channel. > This cause on server demarshaller to only have last message defined, > while on the client

[Spice-devel] [spice-html5] Check if streams array exists in handle_draw_jpeg_onload

2018-05-16 Thread Christophe Fergeau
From: Joel Purra - It seems `SpiceDisplayConn` does not always have the array `this.o.sc.streams` set. - It also seems (stream?) images can be loaded before `streams` is set. - Without `streams`, or the specific stream matching `this.o.id`,

Re: [Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible

2018-05-16 Thread Christophe Fergeau
On Wed, May 16, 2018 at 10:46:35AM +0200, Christophe Fergeau wrote: > On Wed, May 16, 2018 at 10:43:20AM +0200, Christophe Fergeau wrote: > > On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote: > > Yes, indeed, this is similar to what you were doing in earlier

Re: [Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible

2018-05-16 Thread Christophe Fergeau
On Wed, May 16, 2018 at 10:43:20AM +0200, Christophe Fergeau wrote: > On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote: > > > > diff --git a/src/spice-widget.c b/src/spice-widget.c > > > > index 73db593..251b29c 100644 > > > > --- a/src/spice-wi

Re: [Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible

2018-05-16 Thread Christophe Fergeau
On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote: > > > diff --git a/src/spice-widget.c b/src/spice-widget.c > > > index 73db593..251b29c 100644 > > > --- a/src/spice-widget.c > > > +++ b/src/spice-widget.c > > > +static void* prepare_streaming_mode(SpiceChannel *channel, bool > > >

Re: [Spice-devel] [RFC spice-gtk v3 1/1] Gstreamer: Use GstVideoOverlay if possible

2018-05-15 Thread Christophe Fergeau
On Wed, Apr 25, 2018 at 03:43:14PM +0300, Snir Sheriber wrote: > Currently when gstreamer is used to decode a full-screen > stream sent from the server, the decoding frames are being > forced to RBGA format and pushed using appsink to be scaled > and rendered to screen. > > Today most of the

Re: [Spice-devel] [PATCH spice-common 1/3] Add support for building with meson/ninja

2018-05-15 Thread Christophe Fergeau
On Thu, May 10, 2018 at 10:26:59AM -0300, Eduardo Lima (Etrunko) wrote: > >> +option('protocol-version', > >> +type : 'string', > >> +value : '0.12.12', > >> +description : 'SPICE protocol version required (default=0.12.12)') > >> + > > > > Why now there's an option for this ? > > >

Re: [Spice-devel] [PATCH] usbredirhost: host should not be marked as claimed on failure

2018-05-15 Thread Christophe Fergeau
Hey, Sorry for the delay in looking at your patch, On Sat, May 12, 2018 at 12:00:26PM +0800, Qiu Wenbo wrote: > This is a patch trying to fix a bug caused by usbredir. The problem is when > usbredir failed to claim all interface of a USB device it won't set > host->claimed to 0. The patch makes

Re: [Spice-devel] [PATCH spice-server 1/3] event-loop: Avoid possible compiler warning

2018-05-15 Thread Christophe Fergeau
I'm late to the party, but I suspect I would have gone the same way as in spice-gtk https://lists.freedesktop.org/archives/spice-devel/2018-May/043324.html and disabled the warning instead. Christophe On Sun, May 06, 2018 at 12:10:31PM +0100, Frediano Ziglio wrote: > With GCC 8.0.1 (Fedora 28),

Re: [Spice-devel] [PATCH] Revert "usbredir: Disconnect USB device asynchronously"

2018-05-15 Thread Christophe Fergeau
If I remember correctly, this asynchronous disconnection was needed because in some case, during normal usage, the disconnection was blocking for seconds (on Windows with usbdk), and if it's not async, it would block the main loop, and thus the whole GUI (all of this should have been in the commit

Re: [Spice-devel] [spice-jhbuild 0/2] moduleset updates

2018-05-15 Thread Christophe Fergeau
Sure, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, May 14, 2018 at 01:32:52PM +0200, Victor Toso wrote: > From: Victor Toso <m...@victortoso.com> > > Hi, > > * moduleset: Use latest gnome-apps modules > > 3.18 which is currently used is a it old.

Re: [Spice-devel] [spice-gtk] build-sys: Update gettext 0.18.2 -> 0.19.8

2018-05-15 Thread Christophe Fergeau
...) > > > > > > And then got downgraded to 0.18.2 by: > > > > > > commit 20f717dac0962c2c52d0f17ba50556eb5aeffb0f > > > Author: Christophe Fergeau <cferg...@redhat.com> > > > Date: Mon Jul 17 13:21:01 2017 +0200 > > >

[Spice-devel] [spice-space.org] Update asciidoc_reader to latest version

2018-05-14 Thread Christophe Fergeau
It comes from https://github.com/getpelican/pelican-plugins, commit 8d96866a4ec and adds python 3 support. --- plugins/asciidoc_reader/README.rst| 41 ++- plugins/asciidoc_reader/asciidoc_reader.py| 108 +--- plugins/asciidoc_reader/asciidocapi.py| 257

[Spice-devel] [spice-space-pages] Don't use absolute URLs for local resources

2018-05-14 Thread Christophe Fergeau
When referencing content hosted on spice-space.org, no need to use the full URL in our HTML pages (ie use /download/ instead of https://www.spice-space.org/download/). --- download.rst | 32 usbredir.rst | 32 2 files changed, 32

Re: [Spice-devel] [PATCH spice-gtk 1/3] Add support for building with meson

2018-05-04 Thread Christophe Fergeau
On Fri, May 04, 2018 at 12:17:20PM -0300, Eduardo Lima (Etrunko) wrote: > On 03/05/18 12:24, Christophe Fergeau wrote: > > Hey, > > > > So I've experimented with this today, couple of comments/suggested > > improvements/fixes/missing things/... > > It might be

Re: [Spice-devel] [spice-gtk 1/3] build-sys: Disable -Wcast-function-type

2018-05-03 Thread Christophe Fergeau
On Thu, May 03, 2018 at 07:37:44AM -0400, Frediano Ziglio wrote: > > > > With gcc 8.0.1 from Fedora 28, I'm getting warnings related to > > -Wcast-function-type in a few places. Most of these could be solved > > by adding an intermediate wrapper with the right number of args, but the > >

Re: [Spice-devel] [spice-gtk 3/3] win32: Convert locale dir from UTF-8 to local encoding

2018-05-03 Thread Christophe Fergeau
On Thu, May 03, 2018 at 07:28:27AM -0400, Frediano Ziglio wrote: > > > > As indicated in gtk+ win32 gtk_get_localedir [1], bindtextdomain() is not > > UTF-8 aware on win32, so it needs a filename in locale encoding > > > > Stupid questions: what happens if the directory name has like

Re: [Spice-devel] [PATCH spice-gtk 1/3] Add support for building with meson

2018-05-03 Thread Christophe Fergeau
+revision=meson > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 3a0188d..bfa43a3 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -1,5 +1,7 @@ > NULL = > > +EXTRA_DIST = meson.build > + > noinst_PROGRAMS = > TESTS = test-coroutine

[Spice-devel] [spice-gtk 2/3] usb: Remove unused spice_marshaller_add_by_ref_full() user data

2018-05-03 Thread Christophe Fergeau
g_free() only expects a single argument, so it won't make use of the extra 'channel' arg which will be passed as user data. --- src/channel-usbredir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c index f509e49d..0cc56306

[Spice-devel] [spice-gtk 3/3] win32: Convert locale dir from UTF-8 to local encoding

2018-05-03 Thread Christophe Fergeau
As indicated in gtk+ win32 gtk_get_localedir [1], bindtextdomain() is not UTF-8 aware on win32, so it needs a filename in locale encoding [1] https://gitlab.gnome.org/GNOME/gtk/blob/master/gtk/gtkwin32.c#L187 --- src/spice-glib-main.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-)

[Spice-devel] [spice-gtk 1/3] build-sys: Disable -Wcast-function-type

2018-05-03 Thread Christophe Fergeau
With gcc 8.0.1 from Fedora 28, I'm getting warnings related to -Wcast-function-type in a few places. Most of these could be solved by adding an intermediate wrapper with the right number of args, but the g_source_set_callback() warning cannot be solved this way as the callback signature will vary

Re: [Spice-devel] bad id bug in glz decoder

2018-04-26 Thread Christophe Fergeau
On Thu, Apr 26, 2018 at 01:53:41AM -0400, Frediano Ziglio wrote: > > Hi Ziglio, > > > I was wondering if you have had a chance to look at the email below yet. > > > Best regards, > > Zhongqiang Huang > > Yes, weird, usually git CC people in the commit message, but see > >

Re: [Spice-devel] [PATCH spice-streaming-agent 4/4] Add option to disable logging full frames

2018-04-26 Thread Christophe Fergeau
On Thu, Apr 26, 2018 at 07:29:56AM -0400, Frediano Ziglio wrote: > > > > On Mon, Apr 23, 2018 at 04:07:44PM +0100, Frediano Ziglio wrote: > > > In some cases we want to avoid saving huge amount of data on the log. > > > > > > Signed-off-by: Frediano Ziglio > > > --- > > >

Re: [Spice-devel] [PATCH spice-streaming-agent 4/4] Add option to disable logging full frames

2018-04-26 Thread Christophe Fergeau
On Mon, Apr 23, 2018 at 04:07:44PM +0100, Frediano Ziglio wrote: > In some cases we want to avoid saving huge amount of data on the log. > > Signed-off-by: Frediano Ziglio > --- > src/spice-streaming-agent.cpp | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) >

Re: [Spice-devel] [PATCH spice-streaming-agent v2] Eliminate signed/unsigned warning

2018-04-26 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, Apr 23, 2018 at 03:25:39PM +0100, Frediano Ziglio wrote: > From: Christophe de Dinechin <dinec...@redhat.com> > > Currently -Wsign-compare is disabled by default in the default settings > (m4/spice-compile

Re: [Spice-devel] [PATCH spice-streaming-agent 1/4] Add some information to the log

2018-04-26 Thread Christophe Fergeau
On Mon, Apr 23, 2018 at 04:07:41PM +0100, Frediano Ziglio wrote: > Allows to track different frame timing. > > Signed-off-by: Frediano Ziglio > --- > src/spice-streaming-agent.cpp | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git

Re: [Spice-devel] [PATCH spice-streaming-agent] build: Use same options to compile unit tests

2018-04-26 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Mon, Apr 23, 2018 at 04:11:00PM +0100, Frediano Ziglio wrote: > Unit test where not compiling with same options. > In this case warnings are different producing different results. > > Signed-off-by: Frediano Ziglio &

Re: [Spice-devel] Gitlab - 2018!

2018-04-26 Thread Christophe Fergeau
Hey, I don't think I've answered to these various threads, but personally I'm fine with the move. Christophe On Tue, Apr 24, 2018 at 05:15:33PM +0200, Victor Toso wrote: > Hey, > > JFYI, we'll be migrating to gitlab.freedesktop.org late this > week, see: > >

Re: [Spice-devel] [PATCH spice-gtk] tests: Shut up warnings about unitialized struct fields

2018-04-26 Thread Christophe Fergeau
On Tue, Apr 24, 2018 at 04:24:32PM -0300, Eduardo Lima (Etrunko) wrote: > Build complains about lots of unitialized fields in TestCase definition, This has been committed, but I don't think this was happening with git master and the default build flags? This should be mentioned in the commit log

Re: [Spice-devel] how to debug spice-gtk on windows

2018-04-26 Thread Christophe Fergeau
On Thu, Apr 26, 2018 at 10:20:25AM +0300, Sameeh Jubran wrote: > Since the code is compiled using MingW and not VS I would suggest using the > "OutputDebugString" function along with DebugView for viewing the prints. You should also be able to use some mingw version of gdb Christophe

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

2018-04-24 Thread Christophe Fergeau
On Tue, Apr 24, 2018 at 05:01:33PM +0200, Christophe de Dinechin wrote: > But we still have the capability to reject a plugin (in a well > defined, non-crashing way) for other reasons. > [...] > To summarize, the purpose of the compatibility check is to guarantee > well-defined behavior on the

[Spice-devel] [spice-server v3 00/14] qxl: Move more guest resource release to red-parse-qxl

2018-04-24 Thread Christophe Fergeau
ges since v1: - moved renaming patch to the end, and made it much more extensive - added a new patch unifying identical methods - reworked 'qxl: Fix guest resources release in red_put_drawable()' so that it's similar to the cursor changes Christophe Christophe Fergeau (14): qxl: Move red_d

[Spice-devel] [spice-server v3 04/14] qxl: Add red_cursor_cmd_{new, ref, unref} helpers

2018-04-24 Thread Christophe Fergeau
, red_cursor_cmd_free() would currently be enough, but this makes the API consistent with red_drawable_{new,ref,unref}. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/cursor-channel.c | 8 ++-- server/red-parse-qxl.c | 40 +--- serv

[Spice-devel] [spice-server v3 14/14] qxl: Use QXLCommandExt in external red-parse-qxl.h API

2018-04-24 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 146 +++- server/red-parse-qxl.h | 12 ++-- server/red-worker.c | 12 ++-- server/tests/test-qxl-parsing.c | 32 +++-- 4 files change

[Spice-devel] [spice-server v3 02/14] qxl: Make red_{get, put}_drawable static

2018-04-24 Thread Christophe Fergeau
Rather than needing to call red_drawable_new() and then initialize it with red_get_drawable(), we can improve slightly red_drawable new so that red_drawable_{new,ref,unref} is all which is used by code out of red-parse-qxl.c. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- serv

[Spice-devel] [spice-server v3 08/14] qxl: Add red_update_cmd_{new, ref, unref} helpers

2018-04-24 Thread Christophe Fergeau
instead, and get a consistent API. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 38 ++ server/red-parse-qxl.h | 7 --- server/red-worker.c| 14 +++--- 3 files changed, 45 insertions(+), 14 deletions(-)

[Spice-devel] [spice-server v3 03/14] qxl: Fix guest resources release in red_put_drawable()

2018-04-24 Thread Christophe Fergeau
ree the guest QXL resources when RedDrawable::qxl is set. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index a705c0d9d..

[Spice-devel] [spice-server v3 12/14] qxl: Improve 'red' and 'qxl' argument names

2018-04-24 Thread Christophe Fergeau
a 'red' is if you are not familiar with the convention. This commit adds a suffix to each of these 'red'/'qxl' variables so that it's clearer what they are referring to. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c

[Spice-devel] [spice-server v3 06/14] qxl: Add red_message_{new, ref, unref} helpers

2018-04-24 Thread Christophe Fergeau
it instead, and get a consistent API. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 37 ++--- server/red-parse-qxl.h | 7 --- server/red-worker.c| 8 3 files changed, 42 insertions(+), 10 deletions(-)

[Spice-devel] [spice-server v3 09/14] qxl: Add red_surface_cmd_{new, ref, unref} helpers

2018-04-24 Thread Christophe Fergeau
Currently, RedWorker is using stack-allocated variables for RedSurfaceCmd. Surface commands are rare enough that we can dynamically allocated them insntead, and make the API in red-parse-qxl.h consistent with how other QXL commands are handled. Signed-off-by: Christophe Fergeau <cf

[Spice-devel] [spice-server v3 07/14] qxl: Release QXL resources in red_put_update_cmd

2018-04-24 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 11 +++ server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index dfa

[Spice-devel] [spice-server v3 05/14] qxl: Release QXL resource in red_put_message

2018-04-24 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 7 +-- server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index ac612dd5d..24d

[Spice-devel] [spice-server v3 11/14] qxl: Release QXL resources in red_put_surface_cmd

2018-04-24 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/display-channel.c | 3 --- server/red-parse-qxl.c | 12 server/red-parse-qxl.h | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/server/display-channel.c b/server/display-channel.c

[Spice-devel] [spice-server v3 10/14] display-channel: Store full RedSurfaceCmd, not just QXLReleaseInfoExt

2018-04-24 Thread Christophe Fergeau
Now that we have a refcounted RedSurfaceCmd, we can store the command itself in DisplayChannel rather than copying QXLReleaseInfoExt. This will let us move the release of the QXL guest resources in red-parse-qxl in the next commit. Signed-off-by: Christophe Fergeau <cferg...@redhat.

[Spice-devel] [spice-server v3 13/14] qxl: Introduce RedQXLGuestResources

2018-04-24 Thread Christophe Fergeau
This allows to share a bit of code in red-parse-qxl.c, but does not help with reducing the number of args which are passed to the parsing functions. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 112 ++---

[Spice-devel] [spice-server v3 01/14] qxl: Move red_drawable_unref/red_drawable_new

2018-04-24 Thread Christophe Fergeau
/unref'ing QXL commands parsed by red-parse-qxl.h. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 26 ++ server/red-parse-qxl.h | 12 server/red-worker.c| 20 3 files changed, 30 insertions(

Re: [Spice-devel] How to check if the spice-streaming-agent workscorrectly

2018-04-23 Thread Christophe Fergeau
On Mon, Apr 23, 2018 at 02:08:45PM +0200, Lukáš Hrázký wrote: > On Mon, 2018-04-23 at 19:22 +0800, 孙得霖 wrote: > > hi, > > @lhrazky > > host: > > qemu-kvm: 1.5.3 > > spice-server: 0.13.3 > > This is an old version of spice server. You need to build the current > git master of the server

Re: [Spice-devel] [PATCH spice-streaming-agent] Eliminate signed/unsigned warning

2018-04-23 Thread Christophe Fergeau
Hey, I believe my comment from https://lists.freedesktop.org/archives/spice-devel/2018-February/042062.html still apply, by default -Wno-sign-compare is in the CFLAGS/CXXFLAGS, so I'd mention in the log that you need to use non-default CXXFLAGS to hit this. Apart from this, looks good to me.

Re: [Spice-devel] [PATCH spice-streaming-agent 3/3] Add a macro to deal with the boilerplate of writing a streaming agent plugin

2018-04-20 Thread Christophe Fergeau
Looks good to me, Acked-by: Christophe Fergeau <cferg...@redhat.com> On Thu, Apr 19, 2018 at 05:48:35PM +0100, Frediano Ziglio wrote: > From: Christophe de Dinechin <dinec...@redhat.com> > > Signed-off-by: Christophe de Dinechin <dinec...@redhat.com> > --- &g

Re: [Spice-devel] [PATCH spice-streaming-agent 1/3] Ensure that plugins cannot bypass version check

2018-04-20 Thread Christophe Fergeau
Acked-by: Christophe Fergeau <cferg...@redhat.com> On Thu, Apr 19, 2018 at 05:48:33PM +0100, Frediano Ziglio wrote: > From: Christophe de Dinechin <dinec...@redhat.com> > > This change addresses two issues related to plugin version checking: > > 1. It is possible fo

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

2018-04-20 Thread Christophe Fergeau
On Fri, Apr 20, 2018 at 10:10:17AM +0200, Christophe de Dinechin wrote: > There is no easy way to test if a method is there. There is, however, > an easy way to test if a C entry point is there in a shared library. > This is the difference between your scenario and mine. In mine, I can >

Re: [Spice-devel] [PATCH spice-streaming-agent 2/3] Change numbering schema

2018-04-19 Thread Christophe Fergeau
On Wed, Apr 18, 2018 at 12:47:42PM +0100, Frediano Ziglio wrote: > From: Christophe de Dinechin > > A major.minor numbering scheme is not ideal for ABI checks. > In particular, it makes it difficult to react to an incompatibility > that was detected post release. > > [More

Re: [Spice-devel] [PATCH spice-streaming-agent 1/3] Ensure that plugins cannot bypass version check

2018-04-19 Thread Christophe Fergeau
On Wed, Apr 18, 2018 at 12:47:41PM +0100, Frediano Ziglio wrote: > From: Christophe de Dinechin > > This change addresses two issues related to plugin version checking: > > 1. It is possible for plugins to bypass version checking or do it wrong >(as a matter of fact,

Re: [Spice-devel] [PATCH 1/2] Ensure that plugins cannot bypass version check

2018-04-18 Thread Christophe Fergeau
On Tue, Apr 17, 2018 at 06:39:05PM +0200, Christophe de Dinechin wrote: > When does this kind of scenario happen? Imagine we added support for > loading/unloading plugins. Version 13 adds a new interface to unload > plugins, and a new “unloadable plugin” entry point. Therefore, > starting with

Re: [Spice-devel] [PATCH spice-gtk] build: Generate correct version when spice-gtk is a submodule

2018-04-18 Thread Christophe Fergeau
Hey, This script comes from https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=build-aux/git-version-gen;h=6d073fcaddd827a396af4c52f1bf00bdd84a9f66;hb=HEAD where this issue might already be fixed, the line that you changed seems to be replaced by: elif test "`git log -1 --pretty=format:x

[Spice-devel] [spice-server v2 7/9] qxl: Release QXL resource in red_put_message

2018-04-17 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 6 -- server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index b0c47cfeb..a5e

[Spice-devel] [spice-server v2 9/9] qxl: Improve 'red' and 'qxl' argument names

2018-04-17 Thread Christophe Fergeau
a 'red' is if you are not familiar with the convention. This commit adds a suffix to each of these 'red'/'qxl' variables so that it's clearer what they are referring to. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c

[Spice-devel] [spice-server v2 1/9] qxl: Remove red_put_blend()

2018-04-17 Thread Christophe Fergeau
SpiceBlend is a typedef to SpiceCopy, and red_put_blend() and red_put_copy() are identical, so we can add a #define red_put_blend red_put_copy similar to the one we already have for red_get_blend. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 7 +---

[Spice-devel] [spice-server v2 5/9] qxl: Fix guest resources release in red_put_drawable()

2018-04-17 Thread Christophe Fergeau
off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index cc6a8b51d..ccb01d92d 100644 --- a/server/red-parse-qxl.c +++ b/server/

[Spice-devel] [spice-server v2 3/9] qxl: Move red_drawable_unref/red_drawable_new

2018-04-17 Thread Christophe Fergeau
RedDrawable really is a RedDrawCmd which is parsed by red-parse-qxl.h This commit moves them close to the other functions creating/unref'ing QXL commands parsed by red-parse-qxl.h Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.

[Spice-devel] [spice-server v2 2/9] qxl: Remove 'blackness' and 'invers' put/get methods

2018-04-17 Thread Christophe Fergeau
SpiceWhiteness/SpiceBlackness/SpiceInvers are 3 typedef for the same type, no need to have 3 identical red_put_xxx/red_get_xxx methods. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 25 - 1 file changed, 4 insertions(+), 21 del

[Spice-devel] [spice-server v2 8/9] qxl: Release QXL resources in red_put_update_cmd

2018-04-17 Thread Christophe Fergeau
Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- server/red-parse-qxl.c | 11 +++ server/red-parse-qxl.h | 3 ++- server/red-worker.c| 3 +-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c index a5e

[Spice-devel] [spice-server v2 4/9] qxl: Make red_{get, put}_drawable static

2018-04-17 Thread Christophe Fergeau
Rather than needing to call red_drawable_new() and then initialize it with red_get_drawable(), we can improve slightly red_drawable new so that red_drawable_{new,ref,unref} is all which is used by code out of red-parse-qxl.c. Signed-off-by: Christophe Fergeau <cferg...@redhat.com> --- serv

[Spice-devel] [spice-server v2 0/9] qxl: Move more guest resource release to red-parse-qxl

2018-04-17 Thread Christophe Fergeau
. Changes since v1: - moved renaming patch to the end, and made it much more extensive - added a new patch unifying identical methods - reworked 'qxl: Fix guest resources release in red_put_drawable()' so that it's similar to the cursor changes Christophe Christophe Fergeau (9): qxl: Remove

[Spice-devel] [spice-server v2 6/9] qxl: Add red_cursor_cmd_new and red_cursor_cmd_free helpers

2018-04-17 Thread Christophe Fergeau
Currently, the cursor channel is allocating RedCursorCmd instances itself, and then calling into red-parse-qxl.h to initialize it, and doing something similar when releasing the data. This commit moves this common code to red-parse-qxl.[ch] Signed-off-by: Christophe Fergeau <cferg...@redhat.

Re: [Spice-devel] [spice-server 04/10] qxl: Fix guest resources release in red_put_drawable()

2018-04-17 Thread Christophe Fergeau
On Mon, Apr 16, 2018 at 06:51:52PM +0300, Uri Lublin wrote: > On 04/16/2018 01:13 PM, Christophe Fergeau wrote: > > At the moment, we'll unconditionally release the guest QXL resources in > > red_put_drawable() even if red_get_drawable() failed and did not > > initialize drawa

<    3   4   5   6   7   8   9   10   11   12   >