Include video metadata when using DMA-BUF to provide stride and
offset. Without this metadata, GStreamer may incorrectly interpret
the buffer layout, leading to corrupted image output—especially
when the framebuffer includes padding.
This is for:
* new message gl_scanout2_unix
* changed API spice_marshaller_get_fd(s)
Signed-off-by: Qiang Yu <yuq825@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Create a new scanout struct supporting multi plane.
We'll use this struct to create different protocol
message for clients with different capability.
Signed-off-by: Qiang Yu <yuq825@gmail.com>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
This allows to fix https://gitlab.freedesktop.org/spice/spice/-/issues/88.
Some technologies like ARM64 TBI, AMD UAI or Intel LAM use higher
address bits to store some additional information.
We should preserve such bits.
In order to do that detect if the group is for host addresses and
threat the address verbatim.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Tested-by: Momin Juned
Prevent a freeze that occurs if the video stream is stopped while a
gl draw is in progress (e.g., when the client requests a new codec).
Ensure proper cleanup of the ongoing gl draw.
Signed-off-by: Michael Scherle <michael.scherle@rz.uni-freiburg.de>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
We need to convert the scanout's drm format to the correct Gstreamer
format while configuring the pipeline. This can be done using
gst_video_dma_drm_fourcc_to_format() API, which will take the drm
fourcc value and return the appropriate Gst format.
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Michael Scherle <michael.scherle@rz.uni-freiburg.de>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
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.
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.
add dmabuf encoding if `drm/drm_fourcc.h` is present and
gstreamer is at least 1.24 due to
`gst_video_dma_drm_fourcc_to_format()`.
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>
Signed-off-by: Michael Scherle <michael.scherle@rz.uni-freiburg.de>
Acked-by: Frediano Ziglio <freddy77@gmail.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.
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>
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.
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>
We need to determine if the client is new enough to support multiple
codecs -- which might include any of the Gstreamer based ones.
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.
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>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
similiar to mjpeg, when defined JCS_EXTENSIONS, there
is no need to convert BGR24/BGRX32 to RGB24.
Signed-off-by: Weishi Li <liweishi@kylinos.cn>
Acked-by: Frediano Ziglio <freddy77@gmail.com>
Do not offset the time attempting to fix client latency.
Client should handle it by itself.
This remove entirely the delay introduced by the server.
This avoids surely possible time drifts in the client.
The server just sends it's concept of time without trying to force any
delay. Only one end should handle this delay in an attempt to
synchronize audio and video instead that doing it in both ends.
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Remove:
test-display-streaming.c: In function 'get_commands':
test-display-streaming.c:218:42: error: 'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
218 | *commands = (Command*) calloc(sizeof(Command), *num_commands);
| ^~~~~~~
test-display-streaming.c:218:42: note: earlier argument should specify number of elements, later size of each element
Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
The original check ensures we do not have an outstanding GL_DRAW. However,
in QEMU, there is no guarantee that a scanout cannot happen while the async
GL_DRAW has not returned a result yet. This introduces a race where if a
scanout is called while there is an outstanding GL_DRAW, QEMU will crash.
Signed-off-by: osy <osy@turing.llc>
Acked-by: Frediano Ziglio <freddy77@gmail.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>
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>
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>
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>
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>
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>
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>