Re: [Spice-devel] [PATCH 04/10] Rename display_channel_set_monitors_config_to_primary()

2016-09-09 Thread Pavel Grunt
On Thu, 2016-09-08 at 11:52 -0500, Jonathon Jongsma wrote: > Since this function is a DisplayChannel method, use a name > consistent > with naming conventions. Acked-by: Pavel Grunt > --- >  server/display-channel.c | 2 +- >  server/display-channel.h | 2 +- >  server/red-worker.c  | 2 +- >  3

Re: [Spice-devel] [PATCH 02/10] Add DisplayChannelPrivate struct

2016-09-09 Thread Pavel Grunt
Hi, I guess the "1-element array trick" can be used to avoid leak of "priv" in these patches as well. Pavel On Thu, 2016-09-08 at 11:52 -0500, Jonathon Jongsma wrote: > Move all of the DisplayChannel data memembers into a private struct > to > encapsulate things better. This necessitated a few n

Re: [Spice-devel] [PATCH spice-gtk] Remove trailing semicolon

2016-09-09 Thread Marc-André Lureau
ack - Original Message - > --- > src/channel-main.c | 2 +- > src/usb-device-manager.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/channel-main.c b/src/channel-main.c > index 26c8c6d..990a06a 100644 > --- a/src/channel-main.c > +++ b/src/channel-

Re: [Spice-devel] [PATCH v2 03/10] Generate GTypes for spice-server enums

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 09:09:53AM -0500, Jonathon Jongsma wrote: > On Thu, 2016-09-08 at 08:25 -0400, Frediano Ziglio wrote: > > > > > > > > > Looked at this a bit closer, any reason why this is limited to > > > spice-server.h and not including the other public headers? I don't > > > really unde

Re: [Spice-devel] [qxl-wddm-dod PATCH 1/2] Normalise line ending

2016-09-09 Thread Christophe Fergeau
Acked-by: Christophe Fergeau On Thu, Sep 08, 2016 at 12:22:47PM +0100, Frediano Ziglio wrote: > Some files had different line ending (some lines DOS while some others > UNIX). Use the same line ending using the nearest ending. > > Signed-off-by: Frediano Ziglio > --- > qxldod/QxlDod.cpp |

Re: [Spice-devel] [PATCH v3 00/28] Win10 support patches

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 06:59:06AM -0400, Frediano Ziglio wrote: > > No problem, we will do it this way. > > About future/added files I would suggest to use Unix format, we can always > change > all files to Unix in the future (for instance when we'll update visual studio > basically > changin

Re: [Spice-devel] [qxl-wddm-dod PATCH 2/2] Convert UTF-16LE files to single byte unix format

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 12:22:48PM +0100, Frediano Ziglio wrote: > From: Dmitry Fleytman Is this patch from Dmitry or from you? Since you seem to have tested various tools, and the less binary files, the better, Acked-by: Christophe Fergeau Christophe signature.asc Description: PGP signature

[Spice-devel] [RFC PATCH 2/2] Start writing some documentation on protocol

2016-09-09 Thread Frediano Ziglio
Signed-off-by: Frediano Ziglio --- docs/spice_protocol.txt | 48 1 file changed, 48 insertions(+) diff --git a/docs/spice_protocol.txt b/docs/spice_protocol.txt index b62da25..b0ba568 100644 --- a/docs/spice_protocol.txt +++ b/docs/spice_protocol.

[Spice-devel] [RFC PATCH 1/2] Start adding protocol file documentation

2016-09-09 Thread Frediano Ziglio
The protocol file is not documented and people have to read code to understand the specification. This can lead to unexpected or not optimal results so it's better to have it documented. The m4/spice_manual.m4 came from spice server and is meant to be reused. Signed-off-by: Frediano Ziglio --- M

[Spice-devel] [RFC PATCH 0/2] Protocol file documentation

2016-09-09 Thread Frediano Ziglio
Yesterday we spent a bit trying to understand some extra bytes from the network protocol. We realized that not having a documentation can lead to problem. So I just write some initial patch to add documentation. - Is the format (markdown) a good format? - Should the file be installed somewhere? - I

Re: [Spice-devel] [qxl-wddm-dod PATCH 2/2] Convert UTF-16LE files to single byte unix format

2016-09-09 Thread Frediano Ziglio
> > On Thu, Sep 08, 2016 at 12:22:48PM +0100, Frediano Ziglio wrote: > > From: Dmitry Fleytman > > Is this patch from Dmitry or from you? > Since you seem to have tested various tools, and the less binary files, > the better, > > Acked-by: Christophe Fergeau > > Christophe > Yes, the double

Re: [Spice-devel] [PATCH 05/10] Replace a couple Rings with GList

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 11:52:55AM -0500, Jonathon Jongsma wrote: > Make RedsState::mig_target_clients into a GList to improve encapsulation > and maintainability. Also RedsMigTargetClient::pending_links. With > GList, a type implementation can be ignorant of whether they're > contained within a li

Re: [Spice-devel] [PATCH 09/10] Change RedCharDevice write_queue to GList

2016-09-09 Thread Christophe Fergeau
The use of g_list_last() in this patch makes me think a GQueue would be more appropriate. Christophe On Thu, Sep 08, 2016 at 11:52:59AM -0500, Jonathon Jongsma wrote: > Change a couple more Rings to GList > --- > server/char-device.c | 88 > ++-- >

Re: [Spice-devel] [PATCH 10/10] Change RedCharDevicePrivate::clients to GList

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 11:53:00AM -0500, Jonathon Jongsma wrote: > More Ring cleanup > --- > server/char-device.c | 72 > ++-- > 1 file changed, 30 insertions(+), 42 deletions(-) > > diff --git a/server/char-device.c b/server/char-device.c > index

Re: [Spice-devel] [PATCH 06/10] RedChannelClient: store pipe items in a GList

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 11:52:56AM -0500, Jonathon Jongsma wrote: > Instead of using a Ring (and having a ring item link in every pipe > item), store them in a GList. This also necesitated changing > RedCharDeviceVDIPort->priv->read_bufs to a GList as well. > --- > server/cursor-channel.c

Re: [Spice-devel] [PATCH qxl-wddm-dod v2 16/25] Remove minimum size restrict for custom resolution.

2016-09-09 Thread Frediano Ziglio
> On Wed, Sep 7, 2016 at 4:57 PM, Christophe Fergeau < cferg...@redhat.com > > wrote: > > On Tue, Sep 06, 2016 at 05:29:22PM +0300, Sameeh Jubran wrote: > > > > It is the recommended minimum, however Windows can handle low resolution > > > > just fine. We have tested that, > > > > even the old

Re: [Spice-devel] [PATCH v3 20/28] Add type enum to qxl/vga device

2016-09-09 Thread Frediano Ziglio
> > This patch adds type enum to the qxl/vga device class. > > Based on a patch by Sandy Stutsman > > Signed-off-by: Sameeh Jubran > --- > qxldod/QxlDod.cpp | 2 ++ > qxldod/QxlDod.h | 9 - > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/qxldod/QxlDod.cpp b/qxld

Re: [Spice-devel] [PATCH v3 17/28] Enable HW cursor support

2016-09-09 Thread Frediano Ziglio
> > * Turn on enable pointer > > Based on a patch by Sandy Stutsman > > Signed-off-by: Sameeh Jubran > --- > qxldod/QxlDod.cpp | 10 +- > qxldod/QxlDod.h | 2 +- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp > index 39822

Re: [Spice-devel] [PATCH 07/10] Change Drawable->pipes from Ring to GList

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 11:52:57AM -0500, Jonathon Jongsma wrote: > This greatly improves the readability of the code and keeps things > encapsulated better. > --- > server/dcc.c | 12 +--- > server/dcc.h | 1 - > server/display-channel.c | 39 +

Re: [Spice-devel] [PATCH v3 06/28] Enhance code flow and indentation fix

2016-09-09 Thread Frediano Ziglio
> > This patch fixes code flow in StartDriver function. > > Based on a patch by Sandy Stutsman > > Signed-off-by: Sameeh Jubran The extra indentation is not a great enhancement. The do {} while is quite useless. The style changes without a clear coding style document are not justified. But th

Re: [Spice-devel] [PATCH v3 08/28] Fixed misspelling

2016-09-09 Thread Frediano Ziglio
> > Fixed misspelling: HwDeviceInterface > Fixed misspelling: GetDxgkInterface > > Based on a patch by Sandy Stutsman > > Signed-off-by: Sameeh Jubran > --- > qxldod/QxlDod.cpp | 8 > qxldod/QxlDod.h | 14 -- > 2 files changed, 12 insertions(+), 10 deletions(-) > > di

Re: [Spice-devel] [PATCH v3 14/28] Code Analysis clean up

2016-09-09 Thread Frediano Ziglio
> > Based on a patch by Sandy Stutsman > > Signed-off-by: Sameeh Jubran > --- > qxldod/QxlDod.cpp | 471 > ++ > 1 file changed, 260 insertions(+), 211 deletions(-) > > diff --git a/qxldod/QxlDod.cpp b/qxldod/QxlDod.cpp > index 05c6eb9..a6b7

[Spice-devel] [spice-server 2/3] gstreamer: Use RedChunkIterator

2016-09-09 Thread Christophe Fergeau
--- server/gstreamer-encoder.c | 148 +++-- 1 file changed, 36 insertions(+), 112 deletions(-) diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index d5fa072..c9cd3fb 100644 --- a/server/gstreamer-encoder.c +++ b/server/gstreamer-encoder

[Spice-devel] [spice-server 1/3] encoders: Introduce RedChunkIterator

2016-09-09 Thread Christophe Fergeau
This will be used by the GStreamer/mjpeg encoders to get linear data out of a SpiceChunks (which is an array of (binary data, size)). --- server/Makefile.am | 2 + server/red-chunk-iterator.c| 144 ++ server/red-chunk-iterator.h

[Spice-devel] [spice-server 3/3] mjpeg: Use RedChunkIterator

2016-09-09 Thread Christophe Fergeau
--- server/mjpeg-encoder.c | 54 +++--- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c index 1649516..eff64f8 100644 --- a/server/mjpeg-encoder.c +++ b/server/mjpeg-encoder.c @@ -19,13 +19,

[Spice-devel] [spice-server 0/3] encoders: Wrap SpiceChunks handling in a helper class

2016-09-09 Thread Christophe Fergeau
Hey, The gstreamer encoder was doing a bit too many manual iterations/operations on SpiceChunks to skip chunks, get data, ... This series adds a helper class to move all this code out of the encoder itself and make the code flow more obvious. I don't think this makes a big difference in the numbe

Re: [Spice-devel] [PATCH 06/10] RedChannelClient: store pipe items in a GList

2016-09-09 Thread Christophe Fergeau
On Thu, Sep 08, 2016 at 11:52:56AM -0500, Jonathon Jongsma wrote: > Instead of using a Ring (and having a ring item link in every pipe > item), store them in a GList. This also necesitated changing Forgot to mention 'neces*s*itated' typo. Christophe signature.asc Description: PGP signature

[Spice-devel] [spice-server] Fix 'freezed' typo

2016-09-09 Thread Christophe Fergeau
--- server/migration-protocol.h | 2 +- server/pixmap-cache.c | 16 server/pixmap-cache.h | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/migration-protocol.h b/server/migration-protocol.h index 3f08150..8458187 100644 --- a/server

Re: [Spice-devel] [spice-server 3/3] mjpeg: Use RedChunkIterator

2016-09-09 Thread Frediano Ziglio
> --- > server/mjpeg-encoder.c | 54 > +++--- > 1 file changed, 16 insertions(+), 38 deletions(-) > > diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c > index 1649516..eff64f8 100644 > --- a/server/mjpeg-encoder.c > +++ b/server/mjpeg-encod

Re: [Spice-devel] [spice-server] Fix 'freezed' typo

2016-09-09 Thread Frediano Ziglio
Acked-by: Frediano Ziglio > > --- > server/migration-protocol.h | 2 +- > server/pixmap-cache.c | 16 > server/pixmap-cache.h | 6 +++--- > 3 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/server/migration-protocol.h b/server/migration-protocol

Re: [Spice-devel] [PATCH 01/10] RedsState: clean up spice_server_char_device_add_interface

2016-09-09 Thread Frediano Ziglio
> > On Thu, 2016-09-08 at 13:39 -0400, Frediano Ziglio wrote: > > >  > > >  > > > Previously we were creating a variable named 'dev_state' and then > > > apparently not using it. Well, we *were* actually using it, but in > > > a > > > convoluted sort of way. Creating a new RedCharDevice has a > >

Re: [Spice-devel] [PATCH 05/10] Replace a couple Rings with GList

2016-09-09 Thread Frediano Ziglio
> > On Thu, Sep 08, 2016 at 11:52:55AM -0500, Jonathon Jongsma wrote: > > Make RedsState::mig_target_clients into a GList to improve encapsulation > > and maintainability. Also RedsMigTargetClient::pending_links. With > > GList, a type implementation can be ignorant of whether they're > > containe

Re: [Spice-devel] [PATCH 06/10] RedChannelClient: store pipe items in a GList

2016-09-09 Thread Frediano Ziglio
> Instead of using a Ring (and having a ring item link in every pipe > item), store them in a GList. This also necesitated changing > RedCharDeviceVDIPort->priv->read_bufs to a GList as well. At least use a GQueue, a pipe is a queue. > --- > server/cursor-channel.c | 2 - > server/d

Re: [Spice-devel] [PATCH 07/10] Change Drawable->pipes from Ring to GList

2016-09-09 Thread Frediano Ziglio
> > This greatly improves the readability of the code and keeps things > encapsulated better. Personally I think that the old FOREACH macros were more readable and more maintainable instead of expanding manually list handling. > --- > server/dcc.c | 12 +--- > server/dcc.h

Re: [Spice-devel] [spice-server 3/3] mjpeg: Use RedChunkIterator

2016-09-09 Thread Christophe Fergeau
On Fri, Sep 09, 2016 at 09:54:17AM -0400, Frediano Ziglio wrote: > > static int encode_frame(MJpegEncoder *encoder, const SpiceRect *src, > > const SpiceBitmap *image, int top_down) > > { > > -SpiceChunks *chunks; > > +RedChunkIterator it; > > uint32_t image_

Re: [Spice-devel] [PATCH 10/10] Change RedCharDevicePrivate::clients to GList

2016-09-09 Thread Frediano Ziglio
> > More Ring cleanup Personally I don't approve the rationale. Rings are used by Qemu and Linux kernel, Qemu calls us directly and we deal with Linux too. Not counting all BSD code. So surely are not less tested and maintained then glib code. Rings are not less readable or less type safe then G

Re: [Spice-devel] [spice-server 3/3] mjpeg: Use RedChunkIterator

2016-09-09 Thread Frediano Ziglio
> > On Fri, Sep 09, 2016 at 09:54:17AM -0400, Frediano Ziglio wrote: > > > static int encode_frame(MJpegEncoder *encoder, const SpiceRect *src, > > > const SpiceBitmap *image, int top_down) > > > { > > > -SpiceChunks *chunks; > > > +RedChunkIterator it; > > >

Re: [Spice-devel] [PATCH 02/10] Add DisplayChannelPrivate struct

2016-09-09 Thread Jonathon Jongsma
On Fri, 2016-09-09 at 09:37 +0200, Pavel Grunt wrote: > Hi, > > I guess the "1-element array trick" can be used to avoid leak of > "priv" in these patches as well. > > Pavel Indeed, I forgot to change these. Thanks. > > On Thu, 2016-09-08 at 11:52 -0500, Jonathon Jongsma wrote: > > > > Move

Re: [Spice-devel] [PATCH 10/10] Change RedCharDevicePrivate::clients to GList

2016-09-09 Thread Christophe Fergeau
HHey, On Fri, Sep 09, 2016 at 10:47:12AM -0400, Frediano Ziglio wrote: > > > > More Ring cleanup > > Personally I don't approve the rationale. > > Rings are used by Qemu and Linux kernel, Qemu calls us directly and > we deal with Linux too. Not counting all BSD code. So surely are not > less te

Re: [Spice-devel] [PATCH 10/10] Change RedCharDevicePrivate::clients to GList

2016-09-09 Thread Jonathon Jongsma
On Fri, 2016-09-09 at 17:03 +0200, Christophe Fergeau wrote: > HHey, > > On Fri, Sep 09, 2016 at 10:47:12AM -0400, Frediano Ziglio wrote: > > > > > > > > > > > More Ring cleanup > > > > Personally I don't approve the rationale. > > > > Rings are used by Qemu and Linux kernel, Qemu calls us di

Re: [Spice-devel] [PATCH qxl-wddm-dod v2 00/25] Win10 support patches

2016-09-09 Thread Frediano Ziglio
> > This series contains the latest patches to support Windows 10. > > Visual Studio 2015 with Win10 WDK is required to compile this code, > Current patches may be compiled and will work for Windows 10. > > To ease the process of the reviews, those patches are avilable on: > https://github.com/d

Re: [Spice-devel] [PATCH 06/10] RedChannelClient: store pipe items in a GList

2016-09-09 Thread Jonathon Jongsma
On Fri, 2016-09-09 at 10:27 -0400, Frediano Ziglio wrote: > > > > Instead of using a Ring (and having a ring item link in every pipe > > item), store them in a GList. This also necesitated changing > > RedCharDeviceVDIPort->priv->read_bufs to a GList as well. > > At least use a GQueue, a pipe is

Re: [Spice-devel] [PATCH v2 03/10] Generate GTypes for spice-server enums

2016-09-09 Thread Jonathon Jongsma
On Fri, 2016-09-09 at 11:18 +0200, Christophe Fergeau wrote: > On Thu, Sep 08, 2016 at 09:09:53AM -0500, Jonathon Jongsma wrote: > > > > On Thu, 2016-09-08 at 08:25 -0400, Frediano Ziglio wrote: > > > > > > > > > > > > > > > > > > > Looked at this a bit closer, any reason why this is limited t