Commit Graph

3016 Commits

Author SHA1 Message Date
Eduardo Lima (Etrunko)
b0e141b387 build: Move spice-common to subprojects/ directory
The reason for this commit is that Meson expects all submodules to be
placed in this subdirectory, and since autotools build is more flexible
in this case, we make some small adjustments to configure.ac and
Makefile.am files to accommodate for this change.

Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-23 14:49:19 -03:00
Christophe Fergeau
35488f1562 dcc: Rework COMPRESS_DEBUG macro
Rather than using
 #ifdef COMPRESS_DEBUG
   spice_info(...);
 #endif

we can #define COMPRESS_DEBUG to spice_debug() or to do nothing for a
slight readability improvement. This opportunity is used to replace
these spice_debug() calls with g_debug(). The "do nothing" macro is a bit
convoluted to ensure that we will have a compile-time check for our
g_debug args.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-23 12:01:09 +02:00
Christophe Fergeau
48179332d9 dcc: Fix QUIC fallback in get_compression_for_bitmap()
There was a small regression introduced in get_compression_for_bitmap()
by f401eb07f dcc: Rewrite dcc_image_compress.
If SPICE_IMAGE_COMPRESSION_AUTO_GLZ is specified, and the bitmap has a
stride which is bigger than its width (ie it has padding), then
get_compression_for_bitmap() will return SPICE_IMAGE_COMPRESSION_OFF
while in that case, we used to use QUIC for compression.

This happens because that function in the AUTO_GLZ case first checks if
QUIC should be used, if not, it decides to use GLZ, but then decides it
can't because of the stride, so falls back to OFF, while it used to
fall back to QUIC.

This commit only slightly reworks a preexisting if (!can_lz_compress())
check so that it's unconditional rather than depending on the previous
checks having been unsuccessful.

This issue could be observed by using a spice-html5 without support for
uncompressed bitmaps with end-of-line padding by simply starting a f28
VM and connecting to it/moving the mouse cursor in it.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-20 12:36:19 +01:00
Christophe Fergeau
25e404b4f1 dcc: Improve can_lz_compress() comment
Both glz_encode and lz_encode error out if the bitmap stride does not
match the bitmap width.

Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-20 09:15:36 +01:00
Christophe Fergeau
8559a69fa6 worker: Fix 'seemless' typo in comment
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-20 09:15:33 +01:00
Frediano Ziglio
4d162260fc test-stream-device: Check data are sent together
Check that data sent to device are collapsed in a single message.
The StreamChannel object is mocked in the test.
This checks that commit dcc3f995d9
("stream-device: handle_data: send whole message") is doing the
right thing.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-19 14:12:31 +01:00
Frediano Ziglio
91aa8ac830 test-stream-device: Factor out a function to start the test
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-19 14:12:28 +01:00
Frediano Ziglio
2f3634441f test-stream-device: Put common parts in setup/teardown functions
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-19 14:12:26 +01:00
Frediano Ziglio
e3bb59c76b test-stream-device: Check server detect and signal huge data
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-19 14:12:22 +01:00
Frediano Ziglio
c66a312137 red-stream-device: Fix and check empty data messages
If guest sent an empty data message this was not parsed correctly.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-19 14:12:17 +01:00
Lukáš Hrázký
e25e7dc00b Rename SpiceHead::id to monitor_id in the protocol
Signed-off-by: Lukáš Hrázký <lhrazky@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-18 08:17:13 +01:00
Frediano Ziglio
6f5681d4f8 red-stream-device: Fix leaks in dispose and finalize chaining parent
dispose and finalize methods have to call parent relative
cleanup method to avoid leaking resources.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-13 09:54:31 +01:00
Frediano Ziglio
bc14aaecd7 reds: Free device chain in spice_server_destroy to avoid leaks
Leak detectors did not manage to find leaks, possibly as double list
have all elements likely with a pointer to them.
The reference from the agent is necessary for inserting it into
the list.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-12 15:15:10 +01:00
Frediano Ziglio
c778c7ce93 glz-encoder-dict: Remove a warning compiling with CentOS 7
CentOS 7 compiler generate this warning:

glz-encoder-dict.c: In function 'glz_dictionary_pre_encode':
glz-encoder-dict.c:516:30: error: 'prev_seg_id' may be used uninitialized in this function [-Werror=maybe-uninitialized]
             dict->window.segs[prev_seg_id].next = seg_id;
                              ^
glz-encoder-dict.c:492:22: note: 'prev_seg_id' was declared here
     uint32_t seg_id, prev_seg_id;
                      ^

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-11 10:22:36 +01:00
Christophe Fergeau
014e4d7263 tests: Add G_PID_FORMAT to glib compat header
G_PID_FORMAT was only added in glib 2.50.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2018-07-08 16:24:35 +01:00
Uri Lublin
dcc3f995d9 stream-device: handle_data: send whole message
SPICE expects to have each frame in a single message.
So far the stream-device did not do that.
That works fine for H264 streams but not for MJPEG.

The client handles by itself MJPEG streams, and not via
gstreamer, and is assuming that a message contains the
whole frame. Since it currently not, using spice-streaming-agent
with MJPEG plugin, confuses the client which burns CPU
till it fails and keeps complaining:
  "GSpice-CRITICAL **: 15:53:36.984: need more input data"

This patch fixes that, by reading the whole message from the
device (the streaming agent) and sending it over to the client.

Signed-off-by: Uri Lublin <uril@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-07-08 15:56:59 +01:00
Christophe Fergeau
85d6d3594a build: Remove unneeded spice_common.h includes
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-07-06 07:06:34 +01:00
Christophe Fergeau
52d8738768 red-record-qxl: Change license header to LGPLv2+
It's currently using a GPLv2+ header, which was probably a mistake given
the project overall license. It was created by a Red Hat employee, and
only modified by Red Hat employees since then, so the (c) Red Hat is
correct, and there are no other copyright holders to contact.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-05 10:15:17 +02:00
Frediano Ziglio
ca4984570f red-parse-qxl: Avoid invalid flag usage
self_bitmap flag is used for some complex drawing not possible
by QXL_DRAW_COPY commands. Having this flag set causes
spice-server do draw part of the screen, copy that part on new
allocated image and reduce network optimisations with no visual
changes.
Some drivers (like Windows 10 DOD) set this flag by mistake for
this command so reset it.

More details follow.

The self_bitmap flag is used for some drawing command requiring to mix
the frame buffer with some other image. For this specific
QXL_DRAW_COPY command self_bitmap is used by spice-server code during
cachine/sending (the reason for the cache is to cache images sent to
client so the relationship between the two parts of the code).
However the self_bitmap_image (an image created in spice-server if
this flags is set) is used only if src_bitmap of SpiceCopy structure
(the structure used to store the QXL_DRAW_COPY command inside
spice-server) is NULL. But in red_get_copy_ptr (red-parse-qxl.c, the
function that parse the QXL_DRAW_COPY command form the QXL device)
not having a src_bitmap is considered an error so the
self_bitmap_image won't be used.

Why this flag affects network performance?
When spice-server see this flag it update the frame buffer according
to the pending commands (commands to be sent or still to be drawn on
frame buffer). spice-server maintain a tree of commands used to reduce
rendering and command to send. More or less if a command is covering
other commands (for instance filling the entire screen with a single
color) the pending commands can be removed from the queue and not sent
to the client. However when an update of the frame buffer is requested
spice-server update the frame buffer removing the commands from the
tree but not from the client queue.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-03 14:44:33 +01:00
Frediano Ziglio
dde5fd04ad memslot: Remove error parameter from memslot_get_virt
Pointers to memory allocated in user space are never NULL.
The only exception can be if you explicitly map memory at zero.
There is however no reasons for such requirement and this practise
was also removed from Linux due to security reasons.
This API looks copied from a kernel environment where valid virtual
addresses can be NULL.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-03 12:23:54 +01:00
Frediano Ziglio
3f6ac2bccf reds: Fix one case parsing invalid codec string
In case we pass something like "spice:mjpeg$%*" the last part is
ignore making the string parse correctly.
A single pair should end by either string terminator or pair terminator.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-03 09:32:20 +01:00
Frediano Ziglio
f4632931d5 test-codecs-parsing: Add test case
Check if encoder contains an invalid character.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-07-03 09:32:13 +01:00
Jonathon Jongsma
b3a89bca76 Rename parse_video_codecs() to parse_next_video_codec()
The new name describes the function more accurately. Also add
documentation for the function.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-07-02 09:17:57 +01:00
Frediano Ziglio
6842f799db reds: Reuse strspn and strcspn functions
These functions are in the standard C library, not well known
but quite useful for parsing strings.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-06-29 22:04:15 +01:00
Christophe Fergeau
0d38a122d6 test-agent-msg-filter: Adjust for recent logging changes
Now warnings are printed through g_warning which causes the test to
fail. We need to use g_test_expect_message() to prevent that failure.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-28 14:31:22 +01:00
Christophe Fergeau
2a97cec84c qxl: Remove red_channel_printerr()
It was only used twice, for what looks like adhoc debugging. This commit
removes it, similarly to what was done for some spice_printerr() calls.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-28 13:21:50 +01:00
Christophe Fergeau
ed44faef04 Replace remaining spice_printerr() with g_warning()
The remaining occurrences of spice_printerr() are warnings when
something unexpected happens, they can be replaced with g_warning() so
that users of spice-server can redirect them with
g_log_set_default_handler().

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-28 13:21:48 +01:00
Christophe Fergeau
2367497909 Replace spice_printerr() use with red_channel_{debug, warning}
Depending on the context, we want to output a warning or just a debug
log.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-28 13:21:46 +01:00
Christophe Fergeau
5dbd40129a Remove unneeded spice_printerr() calls
These calls seem to have been added for debugging for a very specific
purpose. At the very least, they should have been using g_debug() rather
than spice_printerr(). This commit removes these.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-28 13:21:22 +01:00
Frediano Ziglio
90cd6432da glz: Inline GET_{r,g,b} macros
With last changes are just used once and are straight forward.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-06-27 16:46:52 +01:00
Frediano Ziglio
9d2ec4d5c3 glz: Optimize SAME_PIXEL for RGB16
Do not extract all components and compare one by one, can be easily
compared together.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-06-27 16:46:50 +01:00
Frediano Ziglio
aaef481691 glz: Move some macros to a common place
The macros for both depth are the same, reuse the definition.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-06-27 16:46:30 +01:00
Christophe Fergeau
a7a16504c9 sound: Don't mute recording when client reconnects
When a new record channel is added, the code relies on a snd_send() call
in record_channel_client_constructed() to send RECORD_START to the
client. However, at this point, snd_send() is non-functional because
the red_channel_client_pipe_add() call it makes is a no-op because
prepare_pipe_add() makes a connection check through
red_channel_client_is_connected() queueing the item. This connection
check returns FALSE at ::constructed() time as the channel client will
only become connected towards the end of
red_channel_client_initable_init() which runs after the object
instantiation is complete.

This causes a bug where starting recording and then
disconnecting/reconnecting the client does not successfully reenable
recording. This is a regression introduced by commit d8dc09
'sound: Convert SndChannelClient to RedChannelClient'

This commit solves this issue by making PlaybackChannelClient and
RecordChannelClient implement GInitable, and move the code interacting
with the client in their _initable_init() function, as at this point the
objects will be able to send data.

https://bugzilla.redhat.com/show_bug.cgi?id=1549132

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-27 15:58:38 +01:00
Frediano Ziglio
4ce6439596 memslot: Return void* from memslot_get_virt
The result of this function is always cast to a pointer, there
is no reason to return an integer.
This API looks copied from a kernel environment where virtual
addresses can have different sizes compare to pointers.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 18:07:31 +01:00
Frediano Ziglio
355c510849 jpeg-encoder: Remove JPEG_IMAGE_TYPE_RGB24
Never used.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 18:06:50 +01:00
Frediano Ziglio
a8e88a991e jpeg-encoder: Avoid useless conversions
Define JpegEncoderContext as an abstract structure.
This allows to reduce casts.
Also remove some alignment warnings on some architecture like mips.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 18:04:12 +01:00
Frediano Ziglio
30f5ab5357 red-record-qxl: Remove potential leak
On some systems you need to call g_spawn_close_pid after
spawning a process to avoid leaks (currently Windows).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 11:17:25 +01:00
Frediano Ziglio
27df2afe34 replay: Use GPid and G_PID_FORMAT for portability
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 11:14:19 +01:00
Frediano Ziglio
dec1fdeab8 Add possibly missing headers for pthread.h
In some environment pthread.h is not defined but its definitions
are used in some headers.
Actually happens using MingW.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-26 10:54:01 +01:00
Frediano Ziglio
486aea984b stat-file: Exit earlier to reduce indentation
Just style change. Invert the if to exit earlier.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2018-06-25 13:12:04 +01:00
Frediano Ziglio
d701fac78f dispatcher: Define pollfd variable only if needed
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2018-06-25 13:12:04 +01:00
Frediano Ziglio
9663ae6785 Do not use bzero
Not defined in MingW.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2018-06-25 13:12:04 +01:00
Frediano Ziglio
9541cd2fec Use PRIxPTR constant for string formatting
These constants are meant to be used in format string for size_t
types. Use them for portability.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
2018-06-25 09:56:37 +01:00
Christophe Fergeau
47ca1f0da0 channel: Remove unused 3rd red_channel_register_client_cbs() arg
It was probably meant to be used as a "user_data" argument for the
various callbacks, but turns out not to be used.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-21 17:54:27 +01:00
Frediano Ziglio
0083207b25 sound: Do not pass unused pointer
Client callbacks in sound channels do not use registered
data so don't pass a valid pointer making clear from
source that the parameter is not used.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-21 17:52:00 +01:00
Christophe Fergeau
8822161833 ssl: Allow to use ECDH ciphers with OpenSSL 1.0
Without an explicit call to SSL_CTX_set_ecdh_auto(reds->ctx, 1), OpenSSL
1.0 (still used by el7) would not use ECDH ciphers (this is now
automatic with OpenSSL 1.1.0). This commit adds this missing call. It's
based on a suggestion from David Jasa

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1566597
2018-06-20 18:17:02 +02:00
Frediano Ziglio
dbc4bcb24b red-worker: Remove not used include
poll is not used anymore by this file.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
2018-06-20 14:20:59 +01:00
Frediano Ziglio
d9689030ec sound: Reduce conditional compilation
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
2018-06-20 14:20:44 +01:00
Snir Sheriber
9f5859c3ba Support h265 in stream-channel
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-06-20 03:18:45 +01:00
Frediano Ziglio
83fd1bb4ff Use "base" as pipe item base field name
Most of pipe items use this name for the base field.
This also allows to use SPICE_UPCAST macros instead of a long
SPICE_CONTAINEROF.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-06-18 13:40:51 +01:00