[Spice-devel] [PATCH spice-server 1/3] ci: Add glib-networking package

This is required by the new test-listen test to connect using TLS. Signed-off-by: Frediano Ziglio --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 94325604..535d220d 100644 --- a/.gitlab-ci.yml +++

[Spice-devel] [PATCH spice-server 2/3] test-listen: Increase failure timeout

The timeout is too short when the test run under Valgrind Signed-off-by: Frediano Ziglio --- server/tests/test-listen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/test-listen.c b/server/tests/test-listen.c index c63ee17e..d355b471 100644

[Spice-devel] [PATCH spice-server 3/3] valgrind: Ignore some library leaks

Ignore getaddrinfo and libproxy leaks. Signed-off-by: Frediano Ziglio --- server/tests/valgrind/glib.supp | 19 +++ 1 file changed, 19 insertions(+) diff --git a/server/tests/valgrind/glib.supp b/server/tests/valgrind/glib.supp index ccfab67d..c4625bba

Re: [Spice-devel] [PATCH spice-server] video-stream: Document the usage of RedUpgradeItem structure

Acked-by: Jonathon Jongsma On Fri, 2018-03-09 at 18:00 +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > server/video-stream.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/server/video-stream.h

Re: [Spice-devel] [PATCH spice-server] stream-channel: Implements monitors_config

On Tue, 2018-03-13 at 10:15 -0400, Frediano Ziglio wrote: > > > > On Tue, 2018-03-13 at 06:21 +, Frediano Ziglio wrote: > > > Although not necessary for a single monitor DisplayChannel implementation > > > this make the DiisplayChannels more coherent from the client > > > point of view. > >

Re: [Spice-devel] [PATCH spice-streaming-agent 2/2] Do not redefine "msg" field

On Fri, 2018-03-09 at 02:41 -0500, Frediano Ziglio wrote: > > > > msg.msg was redefining msg.StreamMsgNotifyError::msg. > > This cause some confusion. > > > > Signed-off-by: Frediano Ziglio > > --- > > src/spice-streaming-agent.cpp | 4 ++-- > > 1 file changed, 2

Re: [Spice-devel] [PATCH spice-streaming-agent 2/2] Do not redefine "msg" field

On Fri, 2018-03-09 at 07:35 +, Frediano Ziglio wrote: > msg.msg was redefining msg.StreamMsgNotifyError::msg. > This cause some confusion. > > Signed-off-by: Frediano Ziglio > --- > src/spice-streaming-agent.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-)

Re: [Spice-devel] [PATCH spice-streaming-agent 1/2] Add name to an unnamed structure

On Fri, 2018-03-09 at 07:35 +, Frediano Ziglio wrote: > Unnamed structure combined with inheritance is considered a > bit hard to read, add a name to make more readable. > > Signed-off-by: Frediano Ziglio > --- > src/spice-streaming-agent.cpp | 2 +- > 1 file changed, 1

Re: [Spice-devel] [PATCH spice-server] stream-channel: Implements monitors_config

> > On Tue, 2018-03-13 at 06:21 +, Frediano Ziglio wrote: > > Although not necessary for a single monitor DisplayChannel implementation > > this make the DiisplayChannels more coherent from the client > > point of view. > > Typo: "DiisplayChannels" > > I've tested this, as expected, with

Re: [Spice-devel] [PATCH spice-server] stream-channel: Implements monitors_config

On Tue, 2018-03-13 at 06:21 +, Frediano Ziglio wrote: > Although not necessary for a single monitor DisplayChannel implementation > this make the DiisplayChannels more coherent from the client > point of view. Typo: "DiisplayChannels" I've tested this, as expected, with the heads attribute

Re: [Spice-devel] [PATCH spice-server 1/2] Change ENABLE_EXTRA_CHECKS statements to #ifdef

On 13/03/18 04:21, Frediano Ziglio wrote: >> >> This patch makes it clear that this is a configure switch and not a >> variable defined somewhere else in the code. >> > > The code is intended that way to make the compiler always parse > these parts. Note that that define is always defined so your

Re: [Spice-devel] [PATCH spice-server] test-listen: Fix some use after free

Hi, On Tue, Mar 13, 2018 at 01:14:35PM +, Frediano Ziglio wrote: > Do not dereference thread_data after has been freed. > > Signed-off-by: Frediano Ziglio Sure, Acked-by: Victor Toso > --- > server/tests/test-listen.c | 4 ++-- > 1 file

Re: [Spice-devel] [spice-gtk v1 01/11] channel-display: remove id parameter from helper function

Hi, On Tue, Mar 13, 2018 at 09:19:00AM -0400, Frediano Ziglio wrote: > > > > From: Victor Toso > > > > Instead of passing the id parameter for destroy_display_stream() which > > is only used for debug, let's store the id when creating the stream at > >

Re: [Spice-devel] [spice-gtk v1 02/11] channel-display: rename display_stream_destroy()

> > From: Victor Toso > > This patch also renames destroy_display_stream() to I would remove the "also", I think just a leftover. > display_stream_destroy() to keep compatibility with > display_stream_create() > > Signed-off-by: Victor Toso > ---

Re: [Spice-devel] [spice-gtk v1 01/11] channel-display: remove id parameter from helper function

> > From: Victor Toso > > Instead of passing the id parameter for destroy_display_stream() which > is only used for debug, let's store the id when creating the stream at > display_stream_create(). > > With the removal of Id parameter, we are using a gpointer for >

[Spice-devel] [PATCH spice-server] test-listen: Fix some use after free

Do not dereference thread_data after has been freed. Signed-off-by: Frediano Ziglio --- server/tests/test-listen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/tests/test-listen.c b/server/tests/test-listen.c index 531bd1bd..c63ee17e 100644

[Spice-devel] [spice-gtk v1 10/11] channel-display-gst: summarize number of frames dropped

From: Victor Toso For example, this has produced 9 lines of debug below instead of 31. GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4 frames GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 5 frames GSpice-DEBUG:

[Spice-devel] [spice-gtk v1 00/11] misc improvements in channel-display

From: Victor Toso Hi, The first four patches are related to using hash table to track stream's structure, discussed with Lukas at [0]. The others are trying to: - reduce too much code in one place; - reduce the verbose logs we have with streaming; [0]

[Spice-devel] [spice-gtk v1 11/11] channel-display-mjpeg: remove verbose logs

From: Victor Toso Those don't add any useful information. Signed-off-by: Victor Toso --- src/channel-display-mjpeg.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c index

[Spice-devel] [spice-gtk v1 07/11] channel-display: add display_stream_stats_save() helper

From: Victor Toso To factor out code from display_handle_stream_data() that are specific to statistics in display_stream and not on handling stream_data itself. Signed-off-by: Victor Toso --- src/channel-display.c | 64

[Spice-devel] [spice-gtk v1 09/11] channel-display: don't debug latency for each frame

From: Victor Toso Becomes quite hard to find meaning on something that is printed every time. Only print latency value if it is a new min/max or if average latency is 10% bigger/lower then usual. Not aiming to perfect statistics in latency here, just to avoid too verbose

[Spice-devel] [spice-gtk v1 06/11] channel-display: add spice_frame_new() helper

From: Victor Toso As it makes easier to track what is allocated in its own function and ultimately what needs to be freed later on. Signed-off-by: Victor Toso --- src/channel-display.c | 30 ++ 1 file changed, 22

[Spice-devel] [spice-gtk v1 05/11] channel-display: add spice_frame_free() helper

From: Victor Toso The SpiceFrame is created in channel-display.c but it is currently freed at each decoders' end. A helper function can reduce some code and makes it easier to check if the function is called, what time was called, etc. In channel-display-mjpeg.c this means

[Spice-devel] [spice-gtk v1 01/11] channel-display: remove id parameter from helper function

From: Victor Toso Instead of passing the id parameter for destroy_display_stream() which is only used for debug, let's store the id when creating the stream at display_stream_create(). With the removal of Id parameter, we are using a gpointer for display_stream struct to

[Spice-devel] [spice-gtk v1 03/11] channel-display: remove unneeded function

From: Victor Toso The get_stream_id_by_stream() was introduced in 141c2d82 to debug GStreamer's pipeline. But with previous patch, we are moving the ID to the display_stream structure and can be accessible directly. Signed-off-by: Victor Toso ---

[Spice-devel] [spice-gtk v1 02/11] channel-display: rename display_stream_destroy()

From: Victor Toso This patch also renames destroy_display_stream() to display_stream_destroy() to keep compatibility with display_stream_create() Signed-off-by: Victor Toso --- src/channel-display.c | 8 1 file changed, 4 insertions(+), 4

[Spice-devel] [spice-gtk v1 08/11] channel-display: add display_stream_stats_debug() helper

From: Victor Toso To factor out code from display_stream_destroy() that are specific to statistics in display_stream and not handling stream's destruction. Signed-off-by: Victor Toso --- src/channel-display.c | 89

[Spice-devel] [spice-gtk v1 04/11] channel-display: use GHashTable to keep stream's structure

From: Victor Toso The major issue with the current approach is that it relies on the ID from SpiceMsgDisplayStreamCreate to create the smallest 2^n sized array that could fit the stream's id as index. This is potentially dangerous as the ID value is not documented and could

Re: [Spice-devel] [RFC spice-gtk 0/1] Direct rendering

Hi, On 03/13/2018 12:45 AM, Frediano Ziglio wrote: This patch utilize the GstVideoOverlay interface when the client is running under x window system and it receives a full-screen stream that is decoded using gstreamer (gst => 1.9.0) Some notes - It currently checks for full screen but

Re: [Spice-devel] [spice-gtk v2] channel-display: Use GHashTable to keep stream's structure

On Tue, 2018-03-13 at 08:53 +0100, Victor Toso wrote: > Hi, > > On Mon, Mar 12, 2018 at 05:04:44PM +0100, Lukáš Hrázký wrote: > > > I don't have numbers as I didn't consider it to be > > > significant at the time. My main concern was that it relies > > > on something not documented in the

Re: [Spice-devel] [spice-gtk v1 1/3] channel-display: remove id parameter from helper function

Hi, On Mon, Mar 12, 2018 at 04:28:50PM +0100, Victor Toso wrote: > On Mon, Mar 12, 2018 at 04:13:06PM +0100, Lukáš Hrázký wrote: > > On Fri, 2018-03-02 at 12:39 +0100, Victor Toso wrote: > > > From: Victor Toso > > > > > > Instead of passing the id parameter for

Re: [Spice-devel] [spice-server v2 0/7] Add test-listen test case

On Mon, Mar 12, 2018 at 01:36:59PM -0400, Frediano Ziglio wrote: > Still not convinced about the glib version bump but as > nobody complained and as is silently removing RHEL 6 support which > I agree with, I'm fine with it. +1 that it should be fine to bump it and drop RHEL 6 support. To put it

Re: [Spice-devel] [spice-gtk v2] channel-display: Use GHashTable to keep stream's structure

Hi, On Mon, Mar 12, 2018 at 05:04:44PM +0100, Lukáš Hrázký wrote: > > I don't have numbers as I didn't consider it to be > > significant at the time. My main concern was that it relies > > on something not documented in the protocol itself (stream's > > ID being a small number). > > But that is

Re: [Spice-devel] [PATCH spice-server 2/2] Rename stream-device.[ch] to red-stream-device.[ch]

> > In order to avoid confusion with file named stream-device.h, from > spice-protocol. > > Signed-off-by: Eduardo Lima (Etrunko) > --- > server/Makefile.am | 4 ++-- > server/{stream-device.c => red-stream-device.c} | 4 +--- >

Re: [Spice-devel] [PATCH spice-server 1/2] Change ENABLE_EXTRA_CHECKS statements to #ifdef

> > This patch makes it clear that this is a configure switch and not a > variable defined somewhere else in the code. > The code is intended that way to make the compiler always parse these parts. Note that that define is always defined so your code is not doing what you are intending.

[Spice-devel] [PATCH spice-server] stream-channel: Implements monitors_config

Although not necessary for a single monitor DisplayChannel implementation this make the DiisplayChannels more coherent from the client point of view. Signed-off-by: Frediano Ziglio --- server/stream-channel.c | 33 ++--- 1 file changed, 30