There is a red_init() methods, we can group all the red_init_*() calls
in it rather than calling red_init() followed by all these calls in our
main function.
This has been renamed to SpiceImageCompression in order to avoid clashes
with older spice-server in the SPICE_IMAGE_COMPRESS_ namespace. This
commit is a straight rename of SpiceImageCompress to
SpiceImageCompression and SPICE_IMAGE_COMPRESS_ to
SPICE_IMAGE_COMPRESSION_
red_marshall_image() allows to use other than QUIC compression only
when auto_lz or auto_glz image compression is set. Other images don't
have the problem because they are compressed using red_compress_image()
Locking the individual calls that access the pixmap cache in fill_bits is
not adequately thread safe. Often a windows guest with multiple monitors
will be sending the same image via different threads. Both threads can
be in fill_bits at the same time making changes to the cache for the same
image. This can result in images being deleted before all the client
channels are finished with them or with the same image being send multiple
times. Here's what can happen with out the lock in fill_bits
On the server in red_worker.c:fill_bits
Thread 1 calls pixmap_cache_hit for Image A and finds it isn't in cache
Thread 2 calls pixmap_cache_hit for Image A and finds it isn't in cache
Thread 1 adds Image 1 to pixmap_cache (1x)
Thread 2 adds Image 1 to pixmap_cache (2x)
On the client
Channel 1 adds Image A to image_cache (1x)
Channel 2 replaces Image A in image_cache (1x)
On server
Thread 1 sends Image A rendering commands
Thread N removes Image A from pixmap_cache (image remains - 1x)
Thread 2 sends Image A rendering commands
On client
Channe1 renders from Image A
Channel N removes Image a from image_cache (image is completely removed)
Channel2 render command hangs waiting for Image A
spice-server will attempt to limit number of monitors.
Guest machine can send monitor list it accepts. Limiting the number sent
by guest will limit the number of monitors client will try to enable.
The guest usually see client monitors enabled and start using it so
not seeing client monitor won't try to enable more monitor.
In this case the additional monitor guest can support will always be
seen as heads with no attached monitors.
This allows limiting monitors number without changing guest drivers.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Do not just check and give warning before crashing the program
accessing a NULL pointer but use spice_malloc which exits with a
proper message.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
There is already a enumeration in a public header that defines the
different streaming options, so there's no need to duplicate that
enumeration internally. Just use the public enum values.
Reversing the bottom-up images in the server is not needed since Pixman,
in the client, is able to deal with them. As a result, the previous code
was more complex and wrong. This commit fixes and cleans it.
Currently, the LZ4 encoding only (partially) supports RGB images, so
we must check the image format before using it. In the future, indexed
formats may be implemented too, but their use is usually very small
compared to RGB.
- Add lz4 encoder to compress an image of type LZ4 (see spice_common).
- Add code in red_worker to use LZ4 when it is enabled, and the client
supports it through its display capability, or fallback to LZ.
- Add enable_lz4 switch in the configure script. Show LZ4 support at the
end.
The multimedia time is defined by the server side monotonic time [1],
but the drawing time-stamp is done in guest side, so it requires
synchronization between host and guest. This is expensive, when no audio
is playing, there is a ~30x/sec wakeup to update the qxl device mmtime,
and it requires marking dirty the rom region.
Instead, the video timestamping can be done more efficiently on server
side, without visible drawbacks.
[1] a better timestamp could be the audio time, since audio players are
usually sync with audio time)
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=912763
It's possible for a buggy guest driver to pass invalid bounding box
dimensions in QXL commands, which would then cause spice-server to
segfault. This patch checks the size of the bounding box of the QXL
command right after it has been parsed.
This fixes rhbz#1135372
The beginning of the surface data needs to be computed correctly if the
stride is negative, otherwise, it should point already to the beginning
of the surface data. This bug seems to exists since 4a208b (0.5.2)
https://bugzilla.redhat.com/show_bug.cgi?id=1029646
https://bugs.freedesktop.org/show_bug.cgi?id=79246
As a developer, I maybe want to see the detail compress stat of spice, like this:
Method count orig_size(MB) enc_size(MB) enc_time(s)
QUIC 846 948.02 147.22 7.51
GLZ 2895 594.90 26.60 1.33
ZLIB GLZ 0 0.00 0.00 0.00
LZ 1 3.15 0.01 0.00
JPEG 0 0.00 0.00 0.00
JPEG-RGBA 0 0.00 0.00 0.00
----------------------------------------------------------------------------
Total 3742 1546.07 173.83 8.84
But when I uncommented the COMPRESS_STAT and COMPRESS_DEBUG in red_worker.c and make.
I got some error(in Bugzilla). This error because of some simple syntax errors.
Commit this patch to fix this issue.
Signed-off-by: Wang Qiang <wangqiang.hunan@gmail.com>
gcc's some integer type definitions are different between 32/64bit system.
This causes platform dependency problem with printf function. However,
we can avoid this problem by using PRI macros that supports platform
independent printf.
rhbz#1004443
The methods that trigger waitings on the client pipe require that
the waiting will succeed in order to continue, or otherwise, that
all the living pipe items will be released (e.g., when
we must destroy a surface, we need that all its related pipe items will
be released). Shutdown of the socket will eventually trigger
red_channel_client_disconnect (*), which will empty the pipe. However,
if the blocking method failed, we need to empty the pipe synchronously.
It is not safe(**) to call red_channel_client_disconnect from ChannelCbs
, but all the blocking calls in red_worker are done from callbacks that
are triggered from the device.
To summarize, calling red_channel_client_disconnect instead of calling
red_channel_client_shutdown will immediately release all the pipe items that are
held by the channel client (by calling red_channel_client_pipe_clear).
If red_clear_surface_drawables_from_pipe timeouts,
red_channel_client_disconnect will make sure that the surface we wish to
release is not referenced by any pipe-item.
(*) After a shutdown of a socket, we expect that later, when
red_peer_handle_incoming is called, it will encounter a socket
error and will call the channel's on_error callback which calls
red_channel_client_disconnect.
(**) I believe it was not safe before commit 2d2121a170 (before adding ref
count to ChannelClient). However, I think it might still be unsafe, because
red_channel_client_disconnect sets rcc->stream to NULL, and rcc->stream
may be referred later inside a red_channel_client method unsafely. So instead
of checking if (stream != NULL) after calling callbacks, we try to avoid
calling red_channel_client_disconnect from callbacks.
(1) merge 'force' and 'wait_for_outgoing_item' to one parameter.
'wait_for_outgoing_item' is a derivative of 'force'.
(2) move the call to red_wait_outgoing_item to red_clear_surface_drawables_from_pipe
Three blocking functions, one was split to leave the display channel
specific referencing of the DrawablePipeItem being sent inside
red_worker, but the rest (most) of the timeout logic was moved to
red_channel, including the associated constants.
Moved functions:
red_channel_client_wait_pipe_item_sent
red_wait_outgoing_item
red_wait_all_sent
Introduces red_time.h & red_time.c for a small helper function dealing
with time.h
setting DRAW_ALL define doesn't produce correct rendering. Using
update_area instead of red_draw_qxl_drawable will work but it shouldn't
be required. This is not work I intend to do right now, so marking it
for anyone looking at this in the future.
150 seconds is way too long period for holding the guest driver and
waiting for a response for the client. This timeout was 15 seconds, but
when off-screen surfaces ware introduced it was arbitrarily multiplied by
10.
Other existing related bugs emphasize why it is important to decrease
the timeout:
(1) 994211 - the qxl driver waits for an async-io reponse for 60 seconds
and after that, it switches to sync-io mode. Not only that the
driver might use invalid data (since it didn't wait for the query to
complete), falling back to sync-io mode introduces other errors.
(2) 994175 - spice server sometimes doesn't recognize that the client
has disconnected.
(3) There might be cache inconsistency between the client and the server,
and then the display channel waits indefinitely for a cache item (e.g., bug
977998)
This patch changes the timeout to 30 seconds. I tested it under wifi +emulating 2.5Mbps network,
together with playing video on the guest and changing resolutions in a loop. The timeout didn't expired
during my tests.
This bug is related to rhbz#964136 (but from rhbz#964136 info it is still not
clear why the client wasn't responsive).
