[Spice-devel] [client v3 2/4] streaming: Don't crash if no frame was received before closing the stream

Signed-off-by: Francois Gouget --- This could potentially happen if we detect a stream right before it ends. But it's mostly useful for the next patch. src/channel-display.c | 39 --- 1 file changed, 20 insertions(+), 19

Re: [Spice-devel] [PATCH win-vdagent v6 2/3] Encapsulating XPDM implementation

On Thu, Aug 11, 2016 at 1:23 PM, Frediano Ziglio wrote: > > > > The Direct3D 9 API operates on either the Windows XP display driver > > model (XPDM) or the Windows Vista display driver model (WDDM), depending > > on the operating system installed. > > > > This patch

Re: [Spice-devel] [PATCH 7/7] Avoids to initialise OpenSSL threading twice

On Thu, 2016-08-11 at 08:42 -0400, Frediano Ziglio wrote: > > > > > > Hi Frediano, > > > > did you notice any issues ? > > > > CRYPTO_get_locking_callback is kinda deprecated/not needed/noop in recent > > openssl: > > https://github.com/openssl/openssl/commit/2e52e7df518d80188c865ea3f7bb3526d1

Re: [Spice-devel] [PATCH 2/7] gstreamer: Reduce #ifdef

The version with #ifdef is more clear/readable for me Pavel On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- >  server/gstreamer-encoder.c | 12 +--- >  1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git

[Spice-devel] [client v3 3/4] streaming: Don't crash if the stream creation fails

Note that this implies closing the stream before receiving any frame. Signed-off-by: Francois Gouget --- src/channel-display.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/channel-display.c b/src/channel-display.c index 22c54f2..709b3d2 100644 ---

Re: [Spice-devel] [vdagent-win v1 1/2] file-transfer: fix typos

> > Hi, > > On Thu, Aug 11, 2016 at 08:56:11AM -0400, Frediano Ziglio wrote: > > > > > > --- > > > vdagent/file_xfer.cpp | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp > > > index 58d3a31..cbf7e5e 100644 >

[Spice-devel] [vdagent-win v2 2/2] vdagent-win: start vdagent with lock info from session

Commit 5907b6cbb5c724f9729da59a644271b4258d122e started to handle Lock/Unlock events from Session at VDAgent. Although that works just fine, it does not cover all the situations as pointed by Andrei at [0] and I quote: > It fails for next test-case: > > * Connect with RV to VM > * Lock VM

Re: [Spice-devel] [vdagent-win v1 1/2] file-transfer: fix typos

Hi, On Thu, Aug 11, 2016 at 08:56:11AM -0400, Frediano Ziglio wrote: > > > > --- > > vdagent/file_xfer.cpp | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp > > index 58d3a31..cbf7e5e 100644 > > ---

Re: [Spice-devel] [PATCH 1/2] Avoids to initialise OpenSSL threading twice

Ack On Thu, 2016-08-11 at 14:22 +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- >  server/reds.c | 7 +++ >  1 file changed, 7 insertions(+) > > diff --git a/server/reds.c b/server/reds.c > index 6f88649..f74c8d3 100644 > --- a/server/reds.c > +++

Re: [Spice-devel] [client v3 1/4] streaming: Check the stream id in display_update_stream_report() too

> > It's safer and more consistent than assuming the caller has done the > check already. > > Signed-off-by: Francois Gouget Acked-by: Frediano Ziglio > --- > src/channel-display.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) >

Re: [Spice-devel] [PATCH 6/7] Make process_commands_generation variable type coherent

Ack On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- >  server/display-channel.c | 6 +++--- >  server/display-channel.h | 2 +- >  2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/server/display-channel.c

Re: [Spice-devel] [PATCH 5/7] gstreamer: Peephole optimisation for SpiceFormatForGStreamer

On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > Reduce structure length using static allocated string inside the > structure. > This will also avoid using .data.rel.ro section and relocations > reducing even more library size. ok > > Signed-off-by: Frediano Ziglio

[Spice-devel] [vdagent-win v2 1/2] vdagent: rework on event_dispatcher

As _stop_event is not mandatory, we are initializing the events array with something that might be NULL. The problem is with the following patch as I'm introducing a new non mandatory event and logic starts to get a bit hard to follow. So this patch makes explicit if we are setting or receiving a

[Spice-devel] [client v3 4/4] streaming: Create the pipeline at the same time as the GStreamer decoder

This lets create_gstreamer_decoder() fail if it cannot create the pipeline it needs, allowing the caller to try fallbacks. This also means the pipeline has the same lifetime as the decoder which makes it possible to remove a check in queue_frame(). Signed-off-by: Francois Gouget

Re: [Spice-devel] [PATCH 1/7] gstreamer: Fix infinite loop in get_period_bit_rate

Ack On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > This was discovered by chance by me and Uri trying some remote connection > with slow network (8Mbit) and high latency > >   $ ping 10.10.48.87 -c 3 >   3 packets transmitted, 3 received, 0% packet loss, time 2002ms >   rtt

[Spice-devel] [client v3 1/4] streaming: Check the stream id in display_update_stream_report() too

It's safer and more consistent than assuming the caller has done the check already. Signed-off-by: Francois Gouget --- src/channel-display.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/channel-display.c b/src/channel-display.c index

[Spice-devel] [client 1/2] streaming: Send a special stream report to signal streaming errors

Servers that recognize this special report then stop streaming (sending regular screen updates instead) while older ones essentially ignore it. Signed-off-by: Francois Gouget --- This patchset is based on Victor Toso's idea [1] of using the stream reports to tell the

Re: [Spice-devel] [PATCH 4/7] gstreamer: Use a dummy format to make sure format is always defined.

On Thu, 2016-08-11 at 13:38 +0200, Pavel Grunt wrote: > Hi Frediano, > > ack and one suggestion below > > On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > > > > This avoid a check for NULL. There is one more check in spice_gst_encoder_encode_frame() > > Also will be used to catch

Re: [Spice-devel] [PATCH 7/7] Avoids to initialise OpenSSL threading twice

> > Hi Frediano, > > did you notice any issues ? > > CRYPTO_get_locking_callback is kinda deprecated/not needed/noop in recent > openssl: > https://github.com/openssl/openssl/commit/2e52e7df518d80188c865ea3f7bb3526d14b0c > 08 > Wonderful, finally some sane improvements about threading and

Re: [Spice-devel] [vdagent-win v1 1/2] file-transfer: fix typos

> > --- > vdagent/file_xfer.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp > index 58d3a31..cbf7e5e 100644 > --- a/vdagent/file_xfer.cpp > +++ b/vdagent/file_xfer.cpp > @@ -86,7 +86,7 @@ void

[Spice-devel] [spice 2/2] streaming: Stop streaming if the client reports a streaming error

Signed-off-by: Francois Gouget --- If there's enough bandwidth it's as if nothing wrong ever happened :-) server/dcc.c | 9 + 1 file changed, 9 insertions(+) diff --git a/server/dcc.c b/server/dcc.c index d387e8b..b4066f5 100644 --- a/server/dcc.c +++

[Spice-devel] [PATCH 1/2] Avoids to initialise OpenSSL threading twice

Signed-off-by: Frediano Ziglio --- server/reds.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/server/reds.c b/server/reds.c index 6f88649..f74c8d3 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2792,6 +2792,13 @@ static void openssl_thread_setup(void) {

[Spice-devel] [PATCH 2/2] OpenSSL from 1.1.0 is thread safe by default

Signed-off-by: Frediano Ziglio --- server/reds.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/server/reds.c b/server/reds.c index f74c8d3..c3780e0 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2771,6 +2771,7 @@ static int ssl_password_cb(char *buf, int size,

[Spice-devel] [PATCH v2] gstreamer: Use a dummy format to make sure format is always defined.

This avoid a check for NULL. Also will be used to catch invalid values when table will be extended. Signed-off-by: Frediano Ziglio --- server/gstreamer-encoder.c | 36 +--- 1 file changed, 21 insertions(+), 15 deletions(-) Changes from v1: -

Re: [Spice-devel] [PATCH win-vdagent v6 2/3] Encapsulating XPDM implementation

> > The Direct3D 9 API operates on either the Windows XP display driver > model (XPDM) or the Windows Vista display driver model (WDDM), depending > on the operating system installed. > > This patch encapsulates the current XPDM interface implementation into > XPDMInterface class which inherits

Re: [Spice-devel] [PATCH 4/7] gstreamer: Use a dummy format to make sure format is always defined.

Hi Frediano, ack and one suggestion below On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > This avoid a check for NULL. > Also will be used to catch invalid values when table will be extended. > > Signed-off-by: Frediano Ziglio > --- >  server/gstreamer-encoder.c

Re: [Spice-devel] [PATCH v2] gstreamer: Use a dummy format to make sure format is always defined.

On Thu, 2016-08-11 at 14:22 +0100, Frediano Ziglio wrote: > This avoid a check for NULL. > Also will be used to catch invalid values when table will be extended. > > Signed-off-by: Frediano Ziglio Acked-by: Pavel Grunt > --- >  server/gstreamer-encoder.c |

Re: [Spice-devel] [PATCH 7/7] Avoids to initialise OpenSSL threading twice

Hi Frediano, did you notice any issues ? CRYPTO_get_locking_callback is kinda deprecated/not needed/noop in recent openssl: https://github.com/openssl/openssl/commit/2e52e7df518d80188c865ea3f7bb3526d14b0c 08 Pavel On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > Signed-off-by:

[Spice-devel] [vdagent-win v1 2/2] vdagent: remove whitespaces

--- vdagent/display_setting.cpp | 10 +- vdagent/display_setting.h | 2 +- vdagent/file_xfer.cpp | 4 ++-- vdagent/vdagent.cpp | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/vdagent/display_setting.cpp b/vdagent/display_setting.cpp index

Re: [Spice-devel] [vdagent-win v2 2/2] vdagent-win: start vdagent with lock info from session

Hi, On Thu, Aug 11, 2016 at 10:27:40AM -0400, Frediano Ziglio wrote: > > > > Commit 5907b6cbb5c724f9729da59a644271b4258d122e started to handle > > Lock/Unlock events from Session at VDAgent. Although that works just > > fine, it does not cover all the situations as pointed by Andrei at [0] > >

[Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

The following patch broke compilation of xf86-video-qxl: commit b41220b1441b8adea6db9a98e9da1b43a8f426bb Author: Christophe Fergeau Date: Thu Mar 5 15:28:22 2015 +0100 Mark unused public API methods/code as deprecated Acked-by: Jonathon Jongsma

Re: [Spice-devel] [vdagent-win v2 1/2] vdagent: rework on event_dispatcher

> > As _stop_event is not mandatory, we are initializing the events array > with something that might be NULL. The problem is with the following > patch as I'm introducing a new non mandatory event and logic starts to > get a bit hard to follow. > > So this patch makes explicit if we are setting

[Spice-devel] [protocol 1/3] macros: Improve the SPICE_GNUC_DEPRECATED* macros

If the user specifically requests access to the deprecated APIs by defining the SPICE_DEPRECATED macro, then turn off the SPICE_GNUC_DEPRECATED* warnings. Also automatically use G_GNUC_DEPRECATED if available. Add SPICE_GNUC_DEPRECATED_FOR(). Signed-off-by: Francois Gouget

[Spice-devel] [spice 2/3] server: Use SPICE_GNUC_DEPRECATED to avoid a dependency on glib.h

spice-server.h cannot include glib.h because it is a public header and is used by projects that do not use GLib. Signed-off-by: Francois Gouget --- server/spice-migration.h | 4 ++-- server/spice-server.h| 13 ++--- 2 files changed, 8 insertions(+), 9

[Spice-devel] [client 3/3] spice-gtk: Use spice-protocol's SPICE_GNUC_DEPRECATED* macros

- spice-protocol's macros have been improved to automatically work with GLib if available so there is no longer any justification for duplicating them in spice-gtk. - This gets rid of spice-gtk's SPICE_DEPRECATED macro which was causing a naming conflict with the corresponding spice-protocol

Re: [Spice-devel] [vdagent-win v2 2/2] vdagent-win: start vdagent with lock info from session

> > Commit 5907b6cbb5c724f9729da59a644271b4258d122e started to handle > Lock/Unlock events from Session at VDAgent. Although that works just > fine, it does not cover all the situations as pointed by Andrei at [0] > and I quote: > > > It fails for next test-case: > > > > * Connect with RV to VM

Re: [Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

> > On Thu, 11 Aug 2016, Daniel P. Berrange wrote: > > > On Thu, Aug 11, 2016 at 04:28:58PM +0200, Francois Gouget wrote: > > > > > > The following patch broke compilation of xf86-video-qxl: > > > > > > commit b41220b1441b8adea6db9a98e9da1b43a8f426bb > > > Author: Christophe Fergeau

Re: [Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

On Thu, Aug 11, 2016 at 04:28:58PM +0200, Francois Gouget wrote: > > The following patch broke compilation of xf86-video-qxl: > > commit b41220b1441b8adea6db9a98e9da1b43a8f426bb > Author: Christophe Fergeau > Date: Thu Mar 5 15:28:22 2015 +0100 > > Mark unused public

Re: [Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

On Thu, 11 Aug 2016, Daniel P. Berrange wrote: > On Thu, Aug 11, 2016 at 04:28:58PM +0200, Francois Gouget wrote: > > > > The following patch broke compilation of xf86-video-qxl: > > > > commit b41220b1441b8adea6db9a98e9da1b43a8f426bb > > Author: Christophe Fergeau > >

[Spice-devel] [PATCH spice-gtk] Fix docs for SpiceFileTransferTask::progress

This property actually represents a fractional value from 0 to 1.0, not a percentage between 0 and 100. --- src/spice-file-transfer-task.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c index

[Spice-devel] [PATCH spice-gtk 2/2] Add ability to get sizes from SpiceFileTransferTask

If a client is handling multiple SpiceFileTransferTasks at one time, it's not currently possible to provide a single overall progress to the user. The only information that the client can get is the percentage progress. This patch adds two new properties: - total-bytes: the size of the file

Re: [Spice-devel] [PATCH 02/11] Move CursorChannelClient to separate file

As a fast look seems that this patch is not updated considering the encapsulation work on CursorChannel. > > --- > server/Makefile.am | 2 + > server/cursor-channel-client.c | 120 > + > server/cursor-channel-client.h | 50

[Spice-devel] [PATCH 2/7] gstreamer: Reduce #ifdef

Signed-off-by: Frediano Ziglio --- server/gstreamer-encoder.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index c8d7d88..cdd9696 100644 --- a/server/gstreamer-encoder.c +++

[Spice-devel] [PATCH 1/7] gstreamer: Fix infinite loop in get_period_bit_rate

This was discovered by chance by me and Uri trying some remote connection with slow network (8Mbit) and high latency $ ping 10.10.48.87 -c 3 3 packets transmitted, 3 received, 0% packet loss, time 2002ms rtt min/avg/max/mdev = 281.069/316.758/374.413/41.153 ms The encoder->history status

[Spice-devel] [PATCH 4/7] gstreamer: Use a dummy format to make sure format is always defined.

This avoid a check for NULL. Also will be used to catch invalid values when table will be extended. Signed-off-by: Frediano Ziglio --- server/gstreamer-encoder.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git

[Spice-devel] [PATCH 6/7] Make process_commands_generation variable type coherent

Signed-off-by: Frediano Ziglio --- server/display-channel.c | 6 +++--- server/display-channel.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/display-channel.c b/server/display-channel.c index 4f52b79..5e0a004 100644 ---

[Spice-devel] [PATCH 5/7] gstreamer: Peephole optimisation for SpiceFormatForGStreamer

Reduce structure length using static allocated string inside the structure. This will also avoid using .data.rel.ro section and relocations reducing even more library size. Signed-off-by: Frediano Ziglio --- server/gstreamer-encoder.c | 2 +- 1 file changed, 1 insertion(+),

[Spice-devel] [PATCH 3/7] gstreamer: Use static compile check

Signed-off-by: Frediano Ziglio --- server/gstreamer-encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index cdd9696..7376d2b 100644 --- a/server/gstreamer-encoder.c +++

[Spice-devel] [PATCH 7/7] Avoids to initialise OpenSSL threading twice

Signed-off-by: Frediano Ziglio --- server/reds.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/server/reds.c b/server/reds.c index 6f88649..f74c8d3 100644 --- a/server/reds.c +++ b/server/reds.c @@ -2792,6 +2792,13 @@ static void openssl_thread_setup(void) {

Re: [Spice-devel] [PATCH 3/7] gstreamer: Use static compile check

On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio Acked-by: Pavel Grunt > --- >  server/gstreamer-encoder.c | 2 +- >  1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/server/gstreamer-encoder.c