We do not want to stop a stream associated with gl_draw as a result
of timeout because we may not get another opportunity to create a
new stream if the current one gets stopped. However, when the
stream does get stopped for other reasons, we need to clear the
gl_draw_stream pointer associated with the relevant DC.
v2: (suggestions from Frediano)
- Don't stop the stream regardless of whether gl_draw is ongoing
or not
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
This patch adds a new function to enable the creation of Gst memory with
the dmabuf fd as the source by using a dmabuf allocator. And, it also
adds a mechanism to register and invoke any callbacks once the Gst memory
object is no longer used by the pipeline.
This patch also ensures that the source_fps value is always non-zero.
v2: (suggestions from Frediano)
- Moved the code associated with add_frame() and pipeline configuration
into separate functions that are used when encoding dmabuf fd
v3:
- Add the new gstreamer-allocators dependency in autoconf as well
(Frediano)
- Ensure that VIDEO_ENCODER_FRAME_UNSUPPORTED is returned when an
error is encountered in spice_gst_encoder_encode_dmabuf()
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
For remote (or non-gl) clients, if a valid gl_draw stream exists,
then we first extract the dmabuf fd associated with the scanout and
share it with the encoder along with other key parameters such as
stride, width and height. Once the encoder finishes creating an
encoded buffer (using the dmabuf fd as input), we then send it
over to the client. And, as soon as the encoder notifies that it
is no longer using the dmabuf fd, we send a gl_draw_done async to
the application.
v2: (suggestions and fixups from Frediano)
- Moved the DisplayStreamData initialization code from
red_marshall_stream_data() into a separate function that is reused
when marshalling gl_draw_stream.
- Used new/delete instead of g_new/g_free for creating and destroying
dmabuf_data object
- s/notify_mem_free/free
s/red_gst_mem_free_cb/red_free_fb
- Removed the usage of opaque from red_free_cb
v3:
- Obtain the key params such as fd, stride, etc from the stream instead
of the scanout
- Replace the switch with if in red_marshall_gl_draw_stream() to avoid
printing a warning (and spamming the console) when a frame is dropped
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
For non-gl/remote clients, if there is no stream associated with
the DisplayChannel, then we create a new stream. Otherwise, we
just update the current stream's timestamp.
v2: (suggestions and fixups from Frediano)
- Moved the gl_draw_stream object from DCC to DC
- Moved the stream initialization code from display_channel_create_stream()
into a separate function that is reused when creating gl_draw_stream
v3:
- Create a new primary surface whenever a new stream gets created
v4:
- Use nullptr instead of NULL and true instead of TRUE (Frediano)
- Create the stream as part of gl scanout instead of gl draw operation
so that if would be easily possible to obtain key params such as
stride, flags, etc
- Store key params such as fd, flags, stride, etc in the stream so that
we do not have to look at scanout again
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
We need to determine if the client is new enough to support multiple
codecs -- which might include any of the Gstreamer based ones.
v2: (suggestions and fixups from Frediano)
- Add is_gl_client() method to DisplayChannelClient instead of a
dcc_is_gl_client() function.
- Avoid the usage of XXX_CAST macro.
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Once it is determined that an Intel GPU is available/active (after
looking into udev's database), we try to see if there is a h/w
based encoder (element) available (in Gstreamer's registry cache)
for the user selected video codec. In other words, if we find that
the Intel Media SDK Gstreamer plugin (libgstmsdk.so) and associated
libraries (such as va or vaapi) are all installed properly, we add
the appropriate h/w based encoder and post-processor/converter
elements to the pipeline (along with any relevant options) instead
of the s/w based elements.
For example, if the user selects h264 as the preferred codec format,
msdkh264enc and vapostproc will be preferred instead of x264enc
and videoconvert.
v2: (addressed some review comments from Frediano)
- Moved the udev helper into spice-common
- Refactored the code to choose plugins in order msdk > va > vaapi
v3: (Frediano)
- Added relevant encoder options for mjpeg and vp9 codecs (Jin Chung)
v4: (Fixups from Frediano)
- Free the encoder when we cannot find vpp
- Change type and find plugins array length using G_N_ELEMENTS
- Fix gstenc_name UAF by freeing it at the end of the function
- Use g_str_has_prefix instead of strstr
- Include the string "_hw_" in function names that deal with
h/w based plugins
- Rebase on master
v5: rebase on master
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Co-developed-by: Jin Chung Teng <jin.chung.teng@intel.com>
Co-developed-by: Hazwan Arif Mazlan <hazwan.arif.mazlan@intel.com>
test-listen using GIO had issues running under CI for a while.
GIO is reading some desktop configuration so it's not very CI
friendly.
So instead of using GIO use OpenSSL BIO. The code does not
get much bigger or complicated.
We are already using OpenSSL so we are not adding dependencies.
This fixes CI for Fedora 39 (just released and available on docker).
This allowed to remove an old workaround for GIO in .gitlab-ci.yml
(cfr commit 89edf80821
"ci: Workaround an issue with GLib on Fedora 30")
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
If we use the x264enc encoder to encode a stream, then videoconvert
would convert the BGRx data into Y444, which is the preferred format
for x264enc. However, some decoders particularly the ones that are
h/w based cannot work with Y444 if it was the format used by the
encoder. Therefore, to address these situations, we need a way to
override the format used during the encoding stage which can be
accomplished by using the environment variable introduced in this
patch: SPICE_CONVERTER_PREFERRED_FORMAT.
For example, using NV12 as the output format for the videoconvert
element would allow us to pair a s/w based encoder (such as x264enc)
with a h/w based decoder (such as msdkh264dec) for decoding the
stream as most h/w based decoders only work with NV12 format given
its popularity.
Note that choosing an encoder format such as NV12 over Y444 would
probably result in decreased video quality although it would be
compatible with more decoders. Ideally, the client and server need
to negotiate a suitable format dynamically but the current
capabilities do not allow for such exchange.
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Based-on-patch-by: Hazwan Arif Mazlan <hazwan.arif.mazlan@intel.com>
Signed-off-by: Jin Chung Teng <jin.chung.teng@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Newer meson output a warning if "setup" is not provided.
Remove that warning.
Note that Windows build was not changed due to a bug in mingw-meson
script causing duplication of arguments.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Update to stream9 instead of stream8.
Move to Meson instead of Autoconf as build system.
Remove libcacard-devel, not available.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
This fixes the following compiler error while targeting Windows platform.
../../server/smartcard.cpp:20:10: fatal error: arpa/inet.h: No such file or directory
20 | #include <arpa/inet.h>
| ^~~~~~~~~~~~~
Signed-off-by: Biswapriyo Nath <nathbappai@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
With LibreSSL SSL_OP_NO_CLIENT_RENEGOTIATION is opaque which is not
compatible with the OpenSSL 1.0.2 and earlier code path in
red-stream.cpp while SSL_OP_NO_RENEGOTIATION is not yet defined for the
newer OpenSSL code path in reds.cpp.
So with OpenSSL 1.1.0 and later if SSL_OP_NO_RENEGOTIATION is undefined
and SSL_OP_NO_CLIENT_RENEGOTIATION is defined then define the former as
the latter. This will allow the build to succeed with LibreSSL 3.7.2 and
in the future when newer LibreSSL versions add SSL_OP_NO_RENEGOTIATION
that code path will then be used automatically.
Signed-off-by: orbea <orbea@riseup.net>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Required to build using Meson.
Also add a check to "distcheck" job to test you can build with
Meson from distribution file.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The configuration installed with mingw64-openssl cause OpenSSL
to fail to initialize during execution with Wine.
Delete it and use the compiled in options.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Using Fedora 38 the compilation fails due to this warning:
In file included from /usr/include/gstreamer-1.0/gst/video/video.h:202,
from ../../server/gstreamer-encoder.c:27:
/usr/include/gstreamer-1.0/gst/video/video-sei.h:39:21: error: 'H265_MISP_NANOSECONDS' defined but not used [-Werror=unused-const-variable=]
39 | static const guint8 H265_MISP_NANOSECONDS[] = {
| ^~~~~~~~~~~~~~~~~~~~~
Ignore the warning for Gstreamer includes.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Last wsaccel library fails to install on the container.
This test has not been maintained for a while so libraries
are getting pretty old anyway.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The variable 'now' counts in audio sample frames, but the variable
'data' is of type uint8_t *. Multiply 'now' by the size of an audio
sample frame to get the correct source pointer.
This improves the quality of audio recordings in QEMU a little bit.
Fixes: 5d5a7bd181 ("sound: Avoid cast that could cause alignment problems")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Spice uses rand() to generate the random id, but qemu (at least in the case
of qemu-system-x86) fails to initialize the RNG seed (with e.g. srand()).
The result is, that every SPICE session started (by e.g. libvirtd) has the
same client_id. Usually, this is not a problem, but running something like
a SPICE proxy, relying on the client_id to correctly route connections,
this creates problems.
Fixes:
https://gitlab.com/qemu-project/qemu/-/issues/163
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Provides LANG to avoid error detecting encoding.
Install all Wine package, installing only core is not enough.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Remove deprecation warnings like
In file included from ../../server/char-device.cpp:28:
../../server/safe-list.hpp:108:43: error: 'template<class _Category, class _Tp, class _Distance, class _Pointer, class _Reference> struct std::iterator' is deprecated [-Werror=deprecated-declarations]
108 | class safe_list<T>::iterator: public std::iterator<std::forward_iterator_tag, T>
| ^~~~~~~~
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Due to 0-size array you can have warnings like
c++ -Iserver/libspice-server.so.1.14.1.p -Iserver -I../server -I. -I.. -Isubprojects/spice-common -I../subprojects/spice-common -Isubprojects/spice-common/common -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/pixman-1 -I/usr/include/opus -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC -I/usr/include/gstreamer-1.0 -I/usr/include/orc-0.4 -fvisibility=hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -O3 -DSPICE_SERVER_INTERNAL '-DG_LOG_DOMAIN="Spice"' -Wno-sign-compare -Wno-unused-parameter -DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_38 -DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_38 -fno-exceptions -Wno-suggest-final-methods -Wno-suggest-final-types -Wno-array-bounds -Wno-narrowing -Wno-missing-field-initializers -Wno-deprecated-declarations -Wshadow -fPIC -pthread -MD -MQ server/libspice-server.so.1.14.1.p/red-channel-client.cpp.o -MF server/libspice-server.so.1.14.1.p/red-channel-client.cpp.o.d -o server/libspice-server.so.1.14.1.p/red-channel-client.cpp.o -c ../server/red-channel-client.cpp
In file included from /usr/include/c++/12/bits/shared_ptr_atomic.h:33,
from /usr/include/c++/12/memory:78,
from ../server/utils.hpp:24,
from ../server/red-pipe-item.h:27,
from ../server/red-channel-client.h:24,
from ../server/red-channel-client.cpp:37:
In member function 'std::__atomic_base<_IntTp>::__int_type std::__atomic_base<_IntTp>::operator++() [with _ITp = int]',
inlined from 'void red::shared_ptr_add_ref(shared_ptr_counted*)' at ../server/utils.hpp:280:5,
inlined from 'red::shared_ptr<T>::shared_ptr(T*) [with T = RedChannelClient]' at ../server/utils.hpp:143:31,
inlined from 'void RedChannelClient::receive()' at ../server/red-channel-client.cpp:1123:52,
inlined from 'void red_channel_client_event(int, int, RedChannelClient*)' at ../server/red-channel-client.cpp:739:21:
/usr/include/c++/12/bits/atomic_base.h:385:34: error: 'unsigned int __atomic_add_fetch_4(volatile void*, unsigned int, int)' writing 4 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
385 | { return __atomic_add_fetch(&_M_i, 1, int(memory_order_seq_cst)); }
| ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
This brings in the following changes:
Frediano Ziglio (4):
Find Python3 installation correctly on MacOS
build: Correctly check for Python modules
ci: Set WINEPATH during Windows build
Replace EVP_PKEY_cmp with EVP_PKEY_eq
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
mingw64-make is a bash script that wraps make command passing
additional arguments and setup in order to cross compiler for
MingW (to target Windows).
In Fedora 35 the script passes the arguments we provide
twice. So if we pass (like in this case)
"LOG_COMPILE=wine -C server check" the final make command will
receive
"LOG_COMPILE=wine -C server check LOG_COMPILE=wine -C server check"
arguments.
This for some arguments it's not a problem but passing "-C <dir>"
twice causes make to attempt to change directory twice.
This causes a
make: *** server: No such file or directory. Stop.
error. Use cd command before invoking mingw64-make to avoid
having to pass "-C" option.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
This leak causes a CI failure running test-listen test with
Valgrind.
Note that "check-valgrind" CI job will still fail but with
less issues.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The default centos:latest image can not install packages anymore.
The makecheck-centos test fails with the following error:
$ dnf install -y 'dnf-command(config-manager)'
Error: Failed to download metadata for repo 'appstream' \
Cannot prepare internal mirrorlist: No URLs in mirrorlist
Replace it with a centos stream 8 image from quay.io to make
the makecheck-centos test pass
Signed-off-by: Uri Lublin <uril@redhat.com>
gstreamer-plugins-base 1.20 includes a new member in the
GstAppSinkCallbacks struct:
gboolean (*new_event) (GstAppSink *appsink, gpointer user_data);
So it has to be initialized in order to build test-gst.cpp
successfully.
(added in
0a657d6db5
)
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Due to reds->vm_running being initialized to TRUE (since c302e12c
"spice.h: add entries for tracking vm state") the assumption in c23cbd6f
"reds: start QXL devices if VM is running" was wrong and we can't check
on vm_running until that initialization isn't on TRUE (it is that way for
backward compatibility).
Without this revert on qemu initializing spice we will have the
display_init side of qemu not yet ready and therefore respond badly when
spice sends an event as reaction to `red_qxl_start`:
"qxl_send_events: spice-server bug: guest stopped, ignoring."
At least with qemu > v2.0 as a spice consumer is not showing issues as
`red_qxl_start` will be called just after the qemu side is ready
`qemu_spice_display_start` -> `spice_server_vm_start` ... `red_qxl_start`.
Therefore - for now to avoid the current regression - Revert c23cbd6f
"reds: start QXL devices if VM is running" until that old (2012)
initialization is updated (probably an ABI change and therefore taking
some time).
Fixes: https://gitlab.freedesktop.org/spice/spice/-/issues/64
This reverts commit c23cbd6fa8.
Using Fedora 35 the compilation fails due to this warning:
../server/gstreamer-encoder.c: In function 'create_pipeline':
../server/gstreamer-encoder.c:994:5: error: braces around scalar initializer [-Werror]
994 | GstAppSinkCallbacks appsink_cbs = {NULL, NULL, &new_sample, {NULL}};
| ^~~~~~~~~~~~~~~~~~~
../server/gstreamer-encoder.c:994:5: note: (near initialization for 'appsink_cbs.new_event')
../server/gstreamer-encoder.c:994:5: error: missing initializer for field '_gst_reserved' of 'GstAppSinkCallbacks' [-Werror=missing-field-initializers]
In file included from ../server/gstreamer-encoder.c:26:
/usr/include/gstreamer-1.0/gst/app/gstappsink.h:81:16: note: '_gst_reserved' declared here
81 | gpointer _gst_reserved[GST_PADDING - 1];
| ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
Change structure initialisation to avoid the warning.
The same syntax is already used in server/tests/test-gst.cpp.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
A well behaved client implementing the SPICE protocol should check to see
if the server supports the capabilities it needs. This implementation of
the spice-server supports `SPICE_MAIN_CAP_NAME_AND_UUID` and `
SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS` so it should report this to the
client.
Acked-by: Frediano Ziglio <freddy77@gmail.com>
In OpenSSL3, the SSL_accept call now emits proper errors, which we dump
*before* emitting the expected "SSL_accept failed" error message. The
g_test_expect_message framework doesn't really allow us to discard
messages AFAICT, so instead we add a new expectation with fairly loose
criteria.
Fixes#63
Signed-off-by: Simon Chopin <simon.chopin@canonical.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
