Following the commit disabling celt by default, it's quite easy to have
a build without both celt and opus. After this commit, Opus will have to
be installed for a successful build unless one passes --disable-opus.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This version of the CELT codec has long been obsolete, and Opus support
has been added nearly 5 years ago. It's time we move on and try to stop
using Celt ;)
This commit disables CELT by default, but since this could be an
unexpected change for packagers, if CELT 0.5.1 development headers are
installed, it will error out unless --enable-celt051/--disable-celt051
has been explicitly specified.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
lines.c uses hypot(), which is found in libm on some systems. This means
we are currently relying on getting -lm indirectly through some other
means, as otherwise linking any binary with spice-common would fail.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The image is going to network and network protocol is little endian
so the numbers has to be little endian. Note that this is already done
during decoding.
Tested on a ppc64 machine.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
In a comparison with current autotools build system, meson/ninja
provides a huge improvement in build speed, while keeping the same
functionalities currently available and being considered more user
friendly.
The new system coexists within the same repository with the current one,
so we can do more extensive testing of its functionality before deciding
if the old system can be removed, or for some reason, has to stay for
good.
- Meson: https://mesonbuild.com
This is the equivalent of autogen/configure step in autotools. It
generates the files that will be used by ninja to actually build the
source code.
The project has received lots of traction recently, with many GNOME
projects willing to move to this new build system. The following wiki
page has more details of the status of the many projects being ported:
https://wiki.gnome.org/Initiatives/GnomeGoals/MesonPorting
Meson has a python-like syntax, easy to read, and the documentation
on the project is very complete, with a dedicated page on how to port
from autotools, explaining how most common use cases can be
implemented using meson.
http://mesonbuild.com/Porting-from-autotools.html
Other important sources of information:
http://mesonbuild.com/howtox.htmlhttp://mesonbuild.com/Syntax.htmlhttp://mesonbuild.com/Reference-manual.html
- Ninja: https://ninja-build.org
Ninja is the equivalent of make in an autotools setup, which actually
builds the source code. It has being used by large and complex
projects such as Google Chrome, Android and LLVM. There is not much to
say about ninja (other than it is much faster than make) because we
won't interact directly with it as much, as meson does the middle man
job here. The reasoning for creating ninja in the first place is
explained on the following post:
http://neugierig.org/software/chromium/notes/2011/02/ninja.html
Also its manual provides more in-depth information about the design
principles:
https://ninja-build.org/manual.html
- Basic workflow:
Meson package is available for most if not all distros, so, taking
Fedora as an example, we only need to run:
# dnf -y install meson ninja-build.
With Meson, building in-tree is not possible at all, so we need to
pass a directory as argument to meson where we want the build to be
done. This has the advantage of creating builds with different options
under the same parent directory, e.g.:
$ meson ./build --prefix=/usr
$ meson ./build-extra -Dextra-checks=true -Dalignment-checks=true
After configuration is done, we call ninja to actually do the build.
$ ninja -C ./build
$ ninja -C ./build install
Ninja defaults to parallel builds, and this can be changed with the -j
flag.
$ ninja -j 10 -C ./build
- Hacking:
* meson.build: Mandatory for the project root and usually found under
each directory you want something to be built.
* meson_options.txt: Options that can interfere with the result of the
build.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
../tests/test-quic.c: In function ‘gdk_pixbuf_new_random’:
../tests/test-quic.c:205:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
for (i = 0; i < gdk_pixbuf_get_byte_length(random_pixbuf); i++) {
^
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Some assert in the code are doing some paranoid test and in code
paths quite hot.
The encoding time is reduced by 30-50% while the decoding time
is reduced by a 20-30%.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The quic code goes through a function pointer in two places in order to
try to prevent the compiler from inlining code.
Doing performance measurements this trick does not work anymore
and just make code less readable.
This patch and message was based on a previous work of
Christophe Fergeau.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This only adds a basic test relying on gdk-pixbuf.
The main limitation is that gdk-pixbuf does not handle 16bpp images,
nor 32bpp/no alpha images. I should have picked something else instead ;)
This allows at least to exercise the QUIC_IMAGE_TYPE_RGB24 and
QUIC_IMAGE_TYPE_RGBA codepaths.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
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>
Do not extract all components and compare one by one, can be easily
compared together.
Performance measurements on a set of 16 bit images shown an improve of
about 10%.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This avoids the need for a local variable with the right name (which
was decorrelate_drow in some cases in quic_tmpl.c...)
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
'correlation_row' is always set to channel->colleration_row, and we
already pass 'channel' as an argument.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The naming is odd as this is just an alias for channel->correlate_row.
This will also help in subsequent commits to make things more
consistent with quic_rgb_tmpl.c
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Most functions in quic_tmpl.c/quic_rgb_tmpl.c have only superficial
differences. One of them is using channel->state or encoder->rgb_state.
This commit adds a local CommonState *state in all template methods
which will be used instead of channel->state or encoder->rgb_state.
This makes it easier to spot the common code between the 2 template
files...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
We don't need 2 different implementations when the only difference is
the CommonState which is being used.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's always set, no need to have conditional compilation based on it.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's hardcoded at compile-time, and I don't think it was changed in
years...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It's hardcoded at compile-time, and I don't think it was changed in
years...
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
test-logging makes use of functions only available from this version
../tests/test-logging.c: In function ‘test_spice_abort_level’:
../tests/test-logging.c:50:5: error: ‘g_test_subprocess’ is deprecated: Not available before 2.38 [-Werror=deprecated-declarations]
if (g_test_subprocess()) {
^~
In file included from /usr/include/glib-2.0/glib.h:82:0,
from ../tests/test-logging.c:23:
/usr/include/glib-2.0/glib/gtestutils.h:151:10: note: declared here
gboolean g_test_subprocess (void);
^~~~~~~~~~~~~~~~~
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Make sure there are not 2 messages with the same value in the
same channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Make sure there are not 2 messages with the same name in the
same channel.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Code that handled client and server messages check was the same, just
changed some variable names.
Instead use a class to store same information and reuse the code.
This allows easier extension of the 2 path of code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
This is the only function starting with an underscore, looks
out of style.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Building with gcc 8.0.1 from Fedora 28 gives the following error:
FAILED: common/common@@spice-common@sta/marshaller.c.o
../common/marshaller.c: In function 'spice_marshaller_reserve_space':
../common/marshaller.c:311:27: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(uint8_t *, void *)' {aka 'void (*)(unsigned char *, void *)'} [-Werror=cast-function-type]
item->free_data = (spice_marshaller_item_free_func)free;
^
cc1: all warnings being treated as errors
Which can be easily fixed by creating a new function with the correct
signature and calling free() from it.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Rename struct VSCMsgReaderAdd field 'reader_name' to 'name', and struct
VSCMsgATR field 'data' to 'atr' to match their definitions in file
vscard_common.h.
The error log follows:
generated_server_demarshallers.c:1985:30: note: each undeclared identifier is reported only once for each function it appears in
generated_server_demarshallers.c:1994:15: error: ‘VSCMsgReaderAdd {aka struct VSCMsgReaderAdd}’ has no member named ‘reader_name’
memcpy(out->reader_name, in, reader_name__nelements);
^~
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Even though commit df4ec5c318 commented
out most of smartcard code which triggered this error, it still might
happen if a new message is added with an array member.
The reason is a missing declaration of mem_size, which is fixed simply
by checking if the attribute 'nocopy' is present.
The error log follows:
generated_server_demarshallers.c: In function ‘parse_msgc_smartcard_reader_add’:
generated_server_demarshallers.c:1985:30: error: ‘mem_size’ undeclared (first use in this function); did you mean ‘nw_size’?
data = (uint8_t *)malloc(mem_size);
^~~~~~~~
nw_size
This patch also updates test-marshallers so that this bug is triggered.
The diff between generated demarshallers with the patch applied follows:
--- tests/generated_test_demarshallers.c.old 2018-05-17 14:35:29.234056487 -0300
+++ tests/generated_test_demarshallers.c 2018-05-17 14:35:40.554031295 -0300
@@ -286,6 +286,7 @@ static uint8_t * parse_msg_main_ArrayMes
uint8_t *start = message_start;
uint8_t *data = NULL;
uint64_t nw_size;
+ uint64_t mem_size;
uint8_t *in, *end;
uint64_t name__nw_size;
uint64_t name__nelements;
@@ -298,6 +299,7 @@ static uint8_t * parse_msg_main_ArrayMes
}
nw_size = 0 + name__nw_size;
+ mem_size = sizeof(SpiceMsgMainArrayMessage);
/* Check if message fits in reported side */
if (nw_size > (uintptr_t) (message_end - start)) {
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This causes errors if Valgrind or sanitizer or similar memory
leak checkers are used.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
This hack is now made obsolete by the previous commit. We can safely
remove those defines and the code now builds fine for both spice server
and spice-gtk.
commit df4ec5c318
Author: Frediano Ziglio <fziglio@redhat.com>
Date: Fri May 11 16:59:46 2018 +0100
Fix generation of Smartcard channel
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This small test prove a that current generated demarshaller code
is not safe to integer overflows leading to buffer overflows.
Actually from a quick look at the protocol it seems that client
can't cause these overflows but server can quite easily at
demonstrated by this test.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Make code safe using both 32 and 64 bit machine.
Consider that this code can be compiled for machines with 32 bit.
There are some arrays length which are 32 bit.
If size_t this can cause easily an overflow. For instance message_len
sending SPICE_MSG_NOTIFY messages are 32 bit and code add a small
constant (currently 24) before doing the test for size. Now passing
(uint32_t) -20 as message_len would lead to a size of 4 after the
addition. This overflow does not happen on 64 bit machine as the length
is converted to size_t.
There are also some array length where some item are bigger than 1 byte.
For instance SPICE_MAIN_CHANNELS_LIST message have a number of channels
and each channel is composed by 2 bytes. Now the code generated try to do
length * 2 where length is still a 32 bit so if we put a value like
0x80000002u we get 4 as length. This will cause an overflow as code will
allocate very few bytes but try to fill with a huge number of elements.
This overflow happen in both 32 and 64 bit machine.
To avoid all these possible overflows this patch use only 64 bit for
nelements (number of elements), nw_size (network size) and mem_size
(memory size needed) checking the sizes to avoid other overflows
(like pointers conversions under 32 bit machines).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
The macro for both depth is the same, reuse the definition.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This change does not affect generated code but make source more
readable. Also document in a single location the range of this
type.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
When compiling spice-common with meson/ninja under "release" mode, I get
several compiler warnings about possibly-uninitialized members. For
example:
../subprojects/spice-common/common/lines.c: In function ‘miLineArc’:
../subprojects/spice-common/common/lines.c:2167:17: error: ‘edge2.dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
edge->e += edge->dx; \
^~
../subprojects/spice-common/common/lines.c:2426:24: note: ‘edge2.dx’ was declared here
PolyEdgeRec edge1, edge2;
^~~~~
Initializing these structures to zero silences the warnings.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Note that the ID limitation always existed but now we have the
limitation in the protocol itself with SPICE_MAX_NUM_STREAMS
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When testing out some experimental protocol changes, I managed to
trigger the following error:
GEN generated_client_demarshallers.c
Traceback (most recent call last):
File "../../../spice-common/spice_codegen.py", line 267, in <module>
demarshal.write_protocol_parser(writer, proto, True)
File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1270, in write_protocol_parser
parsers[channel.value] = (channel.channel_type, write_channel_parser(writer, channel.channel_type, is_server))
File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1163, in write_channel_parser
func = write_msg_parser(helpers, ids[i].message_type)
File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1061, in write_msg_parser
num_pointers = message.get_num_pointers()
File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 855, in get_num_pointers
count = count + m.get_num_pointers()
File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 662, in get_num_pointers
return self.member_type.get_num_pointers()
File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 507, in get_num_pointers
if self.is_constant_length(self):
TypeError: is_constant_length() takes exactly 1 argument (2 given)
Calling a member function will implicitly pass 'self' as the first
argument, but we were also explicitly passing it as an argument
(self.is_constant_length(self)). This resulted in the above error.
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
Allow to enable code to do additional or expensive checks.
The option should be used by higher level libraries.
By default the option is disabled.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
This message is not straight forward to grasp.
Not clear by the name why we need to wait other channels messages
before resetting the image cache.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
This is reproducible using desktop icons on Windows XP.
These drawing are sent for the icons on the desktop.
To get an extends.x1 >= 32 you have to move an icon out of the
screen on the left side. Set the icon size to 72 as the icon has
to be out of the screen quite a lot.
Disable the grid alignment on the desktop and move an icon out of
the screen. Select and unselect the icon.
Using "/ 32" the icon will have a white background instead of a
transparent one.
Using a "/ 8" the icon is rendered correctly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <cdupontd@redhat.com>
Due to different warning setting some GCC reports:
In file included from ../spice-common/common/sw_canvas.c:27:0,
from client_sw_canvas.c:20:
../spice-common/common/canvas_base.c: In function ‘canvas_get_lz’:
../spice-common/common/canvas_base.c:768:13: error: ‘palette’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
free(palette);
^~~~~~~~~~~~~
../spice-common/common/canvas_base.c:764:9: error: variable ‘free_palette’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
int free_palette = FALSE;
^~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
Some FreeBSD configurations can use LibreSSL instead of OpenSSL.
The two libraries are really similar but need some minimal adjustment.
Signed-off-by: Paweł Pękala <pawelbsd@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
