On Mon, Apr 16, 2018 at 02:07:57PM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Apr 16, 2018 at 05:02:33PM +0100, Frediano Ziglio wrote:
> > > Due to a bug in current packaged Valgrind check-valgrind is failing
> > > with:
> > >
> > > ==17986== Source and destination overlap in memcpy_chk(0x72c0
From: Victor Toso
The 'end' label is used only once and can be replaced by moving the
code into the existing 'if (!devices)'.
For convenience this patch also:
* Explicit check against NULL
* Added curly brackets to the moved 'for'
* Moved variable 'i' to inner scope
Signed-off-by: Victor Toso
Hi,
On Mon, Apr 16, 2018 at 05:44:14PM -0300, Eduardo Lima (Etrunko) wrote:
> This looks like a leftover from the following commit, which removed the
> only reference for this file.
>
> commit 30986505ba6041c293c38cb4b7f4b618a59f4716
> Author: Marc-André Lureau
> Date: Fri May 10 17:05:49 2013
On Mon, Apr 16, 2018 at 08:38:00AM -0400, Frediano Ziglio wrote:
> >
> > On Mon, Apr 16, 2018 at 06:58:18AM -0400, Frediano Ziglio wrote:
> > > Don't like this, lot of structures in this file use "qxl", for coherence
> > > I would change all or nothing but changing all would mean a lot of changes
On Tue, 2018-04-17 at 10:49 +0200, Christophe Fergeau wrote:
> On Mon, Apr 16, 2018 at 08:38:00AM -0400, Frediano Ziglio wrote:
> > >
> > > On Mon, Apr 16, 2018 at 06:58:18AM -0400, Frediano Ziglio wrote:
> > > > Don't like this, lot of structures in this file use "qxl", for coherence
> > > > I wo
Due to a bug in current packaged Valgrind in the CI (1:3.13.0-13.fc27)
check-valgrind is failing with:
==17986== Source and destination overlap in memcpy_chk(0x72c060, 0x72c068, 33)
==17986==at 0x4C344F0: __memcpy_chk (vg_replace_strmem.c:1581)
==17986==by 0x40E7E9: check_vmc_error_message
Hi,
On 04/16/2018 12:37 PM, Frediano Ziglio wrote:
Differences from v1
-Recognize streaming mode by the streaming-mode surface flag
The propagation of this property (as English word, not gobject) is weird.
Seems that is a SpiceDisplay property but in reality is a surface propery
(which is stor
>
> Hi,
>
> On Tue, Apr 10, 2018 at 07:46:46PM +0300, Uri Lublin wrote:
> > On 04/10/2018 02:58 PM, Victor Toso wrote:
> > > On Mon, Apr 09, 2018 at 02:14:15PM +0300, Uri Lublin wrote:
> > > > Some thoughts:
> > > > 1. It make sense to make it 64 on the server side.
> > >
> > > Why? Just so we c
Hi,
On Tue, Apr 17, 2018 at 06:54:27AM -0400, Frediano Ziglio wrote:
> >
> > Hi,
> >
> > On Tue, Apr 10, 2018 at 07:46:46PM +0300, Uri Lublin wrote:
> > > On 04/10/2018 02:58 PM, Victor Toso wrote:
> > > > On Mon, Apr 09, 2018 at 02:14:15PM +0300, Uri Lublin wrote:
> > > > > Some thoughts:
> > >
Acked-by: Christophe Fergeau
On Tue, Apr 17, 2018 at 11:19:16AM +0100, Frediano Ziglio wrote:
> Due to a bug in current packaged Valgrind in the CI (1:3.13.0-13.fc27)
> check-valgrind is failing with:
>
> ==17986== Source and destination overlap in memcpy_chk(0x72c060, 0x72c068, 33)
> ==17986==
>
> 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 rem
> 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 insertions(+), 8 deletions(-)
>
>
>
> 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 66a6eb8..380df7a 100644
> --- a/sr
>
>
> Acked-by: Christophe Fergeau
>
Just for ML reference.
I discovered this issue quite long time ago.
In another project and with Ubuntu I encountered it. I tried to understand,
proved Valgrind had the issue (disassembly and so on).
Did some search on internet and the issue occurred time a
>
> 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: channel-displa
>
> 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 loggi
The decoder_queue_frame now owns frame.
Signed-off-by: Uri Lublin
---
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 0d7aabb..ae59292 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
Signed-off-by: Uri Lublin
---
src/channel-display-mjpeg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 8c4c0aa..7b2f775 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -262,6 +262,7 @@ static gboo
>
> Signed-off-by: Uri Lublin
> ---
> src/channel-display-mjpeg.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
> index 8c4c0aa..7b2f775 100644
> --- a/src/channel-display-mjpeg.c
> +++ b/src/channel-display-mjpeg.c
> @@ -262,
>
> The decoder_queue_frame now owns frame.
>
The queue is named decoding_queue and actually the reason you have to free frame
is that is not owned by decoding_queue in this path of code.
Note that the same issue happens some lines below in the
#if GST_CHECK_VERSION(1,9,0)
if (decoder->app
This structure is potentially used in multiple thread.
Currently in Gstreamer thread using streaming data and coroutine
thread.
Signed-off-by: Frediano Ziglio
---
I would avoid the atomic penalty but better safe then sorry
---
src/spice-channel-priv.h | 2 +-
src/spice-channel.c | 5 ++---
>
> On 13/04/18 18:20, Victor Toso wrote:
> > Hi,
> >
> > On Fri, Apr 13, 2018 at 03:14:56PM -0300, Eduardo Lima (Etrunko) wrote:
> >> On 13/04/18 05:50, Victor Toso wrote:
> >>> From: goldengdeng <907246...@qq.com>
> >>>
> >>> The spice_usb_device_manager_get_devices() is only checking for NULL
ping
>
> ping
>
> >
> > This fix https://bugzilla.redhat.com/show_bug.cgi?id=1501200.
> >
> > Frediano Ziglio (3):
> > Fix integer overflows computing sizes
> > Write a small test to test possible crash
> > Avoid integer overflow computing image sizes
> >
> > python_modules/demarshal.p
On Tue, Apr 17, 2018 at 01:32:53PM +0100, Frediano Ziglio wrote:
> This structure is potentially used in multiple thread.
> Currently in Gstreamer thread using streaming data and coroutine
> thread.
But only GStreamer uses another thread, so this might be
suboptimal for all other channels.
>
> S
>
> On Tue, Apr 17, 2018 at 01:32:53PM +0100, Frediano Ziglio wrote:
> > This structure is potentially used in multiple thread.
> > Currently in Gstreamer thread using streaming data and coroutine
> > thread.
>
> But only GStreamer uses another thread, so this might be
> suboptimal for all other
>
> From: Christophe de Dinechin
>
> This change addresses three 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, the mjpeg fallback sets a bad example)
>
> 2. The current plugin version check viola
On Fri, Apr 13, 2018 at 01:25:08PM +0100, Frediano Ziglio wrote:
> In order to use the new TCP_CORK feature, disable auto flush.
>
> Depending on channel implementation and purpose of the channel enabling
> blindly for all channels could cause performance issues, specifically if
> flush is not don
On 04/17/2018 03:13 PM, Frediano Ziglio wrote:
The decoder_queue_frame now owns frame.
The queue is named decoding_queue and actually the reason you have to free frame
is that is not owned by decoding_queue in this path of code.
I was talking about the function not the queue itself.
I'll r
On 04/13/2018 03:25 PM, Frediano Ziglio wrote:
Cork is a system interface implemented by Linux and some *BSD systems to
tell the system that other data are expected to be written to a socket.
This allows the system to reduce network fragmentation waiting for network
packets to be complete.
Using
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 drawable->release_info_ext
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
---
server/cursor-ch
Hey,
Currently, after parsing a QXL command through red-parse-qxl, the code which
got the command has to tell red-parse-qxl when it no longer needs the
command, but also to remember to release the command QXL resources
itself. This series moves this 'release resource' logic to
red-parse-qxl.
Chan
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
---
server/red-parse-qxl.c | 17
Signed-off-by: Christophe Fergeau
---
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 a5e363579..afd136e43 100644
--- a/
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 drawable->release_info_ext properly.
This commit checks the QXLReleaseInfo in release_info_ext is non-0
before attempting to release it.
Signed-off-by
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
---
server/red-parse-qxl.c | 21 +
server/red-parse-qxl.h | 2
Signed-off-by: Christophe Fergeau
---
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..a5e363579 100644
--- a/server/re
All the methods related to QXL command parsing have a 'qxl' argument
which designates the guest side resources, and a 'red' argument, which
designates the spice-server side data which has been parsed from 'qxl'.
When reading random functions in red-parse-qxl.c, it's quite easy to
wonder what a 'red
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
---
server/red-parse-qxl.c | 7 +--
1 file changed, 1 inserti
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
---
server/red-parse-qxl.c | 25 -
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/serv
> On 17 Apr 2018, at 16:12, Frediano Ziglio wrote:
>
>>
>> From: Christophe de Dinechin
>>
>> This change addresses three 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, the mjpeg fallback se
>
> On 04/13/2018 03:25 PM, Frediano Ziglio wrote:
> > Cork is a system interface implemented by Linux and some *BSD systems to
> > tell the system that other data are expected to be written to a socket.
> > This allows the system to reduce network fragmentation waiting for network
> > packets to
>
> > On 17 Apr 2018, at 16:12, Frediano Ziglio wrote:
> >
> >>
> >> From: Christophe de Dinechin
> >>
> >> This change addresses three 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, th
>
> 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
> ---
> server/red-parse-qxl.c | 25 -
> 1 file changed, 4 insertions(+), 21 deletions(-)
>
>
> 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
>
Does not seem a great reason to have different APIs from red-parse-qxl,
this and next patch seems to mix p
>
> 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
> ---
> server/red-pa
>
> 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 drawable->release_info_ext properly.
> This commit checks the QXLReleaseInfo in release_info_ext is non-0
You forget to update the comment
>
>
> 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
> ---
>
> Signed-off-by: Christophe Fergeau
> ---
> 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..a5e363579 1
49 matches
Mail list logo
