* [PATCH v5 1/7] new configure option to enable gstreamer
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-04-30 7:38 ` Thomas Huth
2025-05-13 14:27 ` Daniel P. Berrangé
2025-04-30 7:25 ` [PATCH v5 2/7] add vnc h264 encoder Dietmar Maurer
` (5 subsequent siblings)
6 siblings, 2 replies; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note
that QEMU already depends on this library when you enable Spice.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
meson.build | 10 ++++++++++
meson_options.txt | 2 ++
scripts/meson-buildoptions.sh | 3 +++
3 files changed, 15 insertions(+)
diff --git a/meson.build b/meson.build
index bcb9d39a38..50a9a2b036 100644
--- a/meson.build
+++ b/meson.build
@@ -1348,6 +1348,14 @@ if not get_option('zstd').auto() or have_block
required: get_option('zstd'),
method: 'pkg-config')
endif
+
+gstreamer = not_found
+if not get_option('gstreamer').auto() or have_system
+ gstreamer = dependency('gstreamer-1.0 gstreamer-base-1.0', version: '>=1.22.0',
+ required: get_option('gstreamer'),
+ method: 'pkg-config')
+endif
+
qpl = not_found
if not get_option('qpl').auto() or have_system
qpl = dependency('qpl', version: '>=1.5.0',
@@ -2563,6 +2571,7 @@ config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
config_host_data.set('CONFIG_STATX', has_statx)
config_host_data.set('CONFIG_STATX_MNT_ID', has_statx_mnt_id)
config_host_data.set('CONFIG_ZSTD', zstd.found())
+config_host_data.set('CONFIG_GSTREAMER', gstreamer.found())
config_host_data.set('CONFIG_QPL', qpl.found())
config_host_data.set('CONFIG_UADK', uadk.found())
config_host_data.set('CONFIG_QATZIP', qatzip.found())
@@ -4895,6 +4904,7 @@ summary_info += {'snappy support': snappy}
summary_info += {'bzip2 support': libbzip2}
summary_info += {'lzfse support': liblzfse}
summary_info += {'zstd support': zstd}
+summary_info += {'gstreamer support': gstreamer}
summary_info += {'Query Processing Library support': qpl}
summary_info += {'UADK Library support': uadk}
summary_info += {'qatzip support': qatzip}
diff --git a/meson_options.txt b/meson_options.txt
index 59d973bca0..11cd132be5 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -254,6 +254,8 @@ option('vnc_sasl', type : 'feature', value : 'auto',
description: 'SASL authentication for VNC server')
option('vte', type : 'feature', value : 'auto',
description: 'vte support for the gtk UI')
+option('gstreamer', type : 'feature', value : 'auto',
+ description: 'for VNC H.264 encoding with gstreamer')
# GTK Clipboard implementation is disabled by default, since it may cause hangs
# of the guest VCPUs. See gitlab issue 1150:
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 3e8e00852b..f88475f707 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -229,6 +229,7 @@ meson_options_help() {
printf "%s\n" ' Xen PCI passthrough support'
printf "%s\n" ' xkbcommon xkbcommon support'
printf "%s\n" ' zstd zstd compression support'
+ printf "%s\n" ' gstreamer gstreamer support (H264 for VNC)'
}
_meson_option_parse() {
case $1 in
@@ -581,6 +582,8 @@ _meson_option_parse() {
--disable-xkbcommon) printf "%s" -Dxkbcommon=disabled ;;
--enable-zstd) printf "%s" -Dzstd=enabled ;;
--disable-zstd) printf "%s" -Dzstd=disabled ;;
+ --enable-gstreamer) printf "%s" -Dgstreamer=enabled ;;
+ --disable-gstreamer) printf "%s" -Dgstreamer=disabled ;;
*) return 1 ;;
esac
}
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v5 1/7] new configure option to enable gstreamer
2025-04-30 7:25 ` [PATCH v5 1/7] new configure option to enable gstreamer Dietmar Maurer
@ 2025-04-30 7:38 ` Thomas Huth
2025-05-13 14:27 ` Daniel P. Berrangé
1 sibling, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-04-30 7:38 UTC (permalink / raw)
To: Dietmar Maurer, marcandre.lureau, qemu-devel
On 30/04/2025 09.25, Dietmar Maurer wrote:
> GStreamer is required to implement H264 encoding for VNC. Please note
> that QEMU already depends on this library when you enable Spice.
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> meson.build | 10 ++++++++++
> meson_options.txt | 2 ++
> scripts/meson-buildoptions.sh | 3 +++
> 3 files changed, 15 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index bcb9d39a38..50a9a2b036 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1348,6 +1348,14 @@ if not get_option('zstd').auto() or have_block
> required: get_option('zstd'),
> method: 'pkg-config')
> endif
> +
> +gstreamer = not_found
> +if not get_option('gstreamer').auto() or have_system
> + gstreamer = dependency('gstreamer-1.0 gstreamer-base-1.0', version: '>=1.22.0',
> + required: get_option('gstreamer'),
> + method: 'pkg-config')
> +endif
> +
> qpl = not_found
> if not get_option('qpl').auto() or have_system
> qpl = dependency('qpl', version: '>=1.5.0',
> @@ -2563,6 +2571,7 @@ config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
> config_host_data.set('CONFIG_STATX', has_statx)
> config_host_data.set('CONFIG_STATX_MNT_ID', has_statx_mnt_id)
> config_host_data.set('CONFIG_ZSTD', zstd.found())
> +config_host_data.set('CONFIG_GSTREAMER', gstreamer.found())
> config_host_data.set('CONFIG_QPL', qpl.found())
> config_host_data.set('CONFIG_UADK', uadk.found())
> config_host_data.set('CONFIG_QATZIP', qatzip.found())
> @@ -4895,6 +4904,7 @@ summary_info += {'snappy support': snappy}
> summary_info += {'bzip2 support': libbzip2}
> summary_info += {'lzfse support': liblzfse}
> summary_info += {'zstd support': zstd}
> +summary_info += {'gstreamer support': gstreamer}
Should this maybe rather go into the "user interface" section, next to the
VNC options?
> summary_info += {'Query Processing Library support': qpl}
> summary_info += {'UADK Library support': uadk}
> summary_info += {'qatzip support': qatzip}
> diff --git a/meson_options.txt b/meson_options.txt
> index 59d973bca0..11cd132be5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -254,6 +254,8 @@ option('vnc_sasl', type : 'feature', value : 'auto',
> description: 'SASL authentication for VNC server')
> option('vte', type : 'feature', value : 'auto',
> description: 'vte support for the gtk UI')
> +option('gstreamer', type : 'feature', value : 'auto',
> + description: 'for VNC H.264 encoding with gstreamer')
>
> # GTK Clipboard implementation is disabled by default, since it may cause hangs
> # of the guest VCPUs. See gitlab issue 1150:
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 3e8e00852b..f88475f707 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -229,6 +229,7 @@ meson_options_help() {
> printf "%s\n" ' Xen PCI passthrough support'
> printf "%s\n" ' xkbcommon xkbcommon support'
> printf "%s\n" ' zstd zstd compression support'
> + printf "%s\n" ' gstreamer gstreamer support (H264 for VNC)'
> }
> _meson_option_parse() {
> case $1 in
> @@ -581,6 +582,8 @@ _meson_option_parse() {
> --disable-xkbcommon) printf "%s" -Dxkbcommon=disabled ;;
> --enable-zstd) printf "%s" -Dzstd=enabled ;;
> --disable-zstd) printf "%s" -Dzstd=disabled ;;
> + --enable-gstreamer) printf "%s" -Dgstreamer=enabled ;;
> + --disable-gstreamer) printf "%s" -Dgstreamer=disabled ;;
> *) return 1 ;;
> esac
> }
Please keep the scripts/meson-buildoptions.sh file sorted alphabetically.
It's an auto-generated file, so if you edit it this way manually, it will be
changed by the build system the next time someone touches meson_options.txt.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v5 1/7] new configure option to enable gstreamer
2025-04-30 7:25 ` [PATCH v5 1/7] new configure option to enable gstreamer Dietmar Maurer
2025-04-30 7:38 ` Thomas Huth
@ 2025-05-13 14:27 ` Daniel P. Berrangé
1 sibling, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 14:27 UTC (permalink / raw)
To: Dietmar Maurer; +Cc: marcandre.lureau, qemu-devel
On Wed, Apr 30, 2025 at 09:25:18AM +0200, Dietmar Maurer wrote:
> GStreamer is required to implement H264 encoding for VNC. Please note
> that QEMU already depends on this library when you enable Spice.
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> meson.build | 10 ++++++++++
> meson_options.txt | 2 ++
> scripts/meson-buildoptions.sh | 3 +++
> 3 files changed, 15 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 2/7] add vnc h264 encoder
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 1/7] new configure option to enable gstreamer Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-05-13 14:31 ` Daniel P. Berrangé
2025-04-30 7:25 ` [PATCH v5 3/7] vnc: h264: send additional frames after the display is clean Dietmar Maurer
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
This patch implements H264 support for VNC. The RFB protocol
extension is defined in:
https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#open-h-264-encoding
Currently the Gstreamer x264enc plugin (software encoder) is used
to encode the video stream.
The gstreamer pipe is:
appsrc -> videoconvert -> x264enc -> appsink
Note: videoconvert is required for RGBx to YUV420 conversion.
The code still use the VNC server framebuffer change detection,
and only encodes and sends video frames if there are changes.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/meson.build | 1 +
ui/vnc-enc-h264.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++
ui/vnc-jobs.c | 53 +++++++---
ui/vnc.c | 37 ++++++-
ui/vnc.h | 21 ++++
5 files changed, 350 insertions(+), 17 deletions(-)
create mode 100644 ui/vnc-enc-h264.c
diff --git a/ui/meson.build b/ui/meson.build
index 35fb04cadf..34f1f33699 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -46,6 +46,7 @@ vnc_ss.add(files(
))
vnc_ss.add(zlib, jpeg)
vnc_ss.add(when: sasl, if_true: files('vnc-auth-sasl.c'))
+vnc_ss.add(when: gstreamer, if_true: files('vnc-enc-h264.c'))
system_ss.add_all(when: [vnc, pixman], if_true: vnc_ss)
system_ss.add(when: vnc, if_false: files('vnc-stubs.c'))
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
new file mode 100644
index 0000000000..26e8c19270
--- /dev/null
+++ b/ui/vnc-enc-h264.c
@@ -0,0 +1,255 @@
+/*
+ * QEMU VNC display driver: H264 encoding
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "vnc.h"
+
+#include <gst/gst.h>
+
+static void destroy_encoder_context(VncState *vs)
+{
+ gst_clear_object(&vs->h264->source);
+ gst_clear_object(&vs->h264->convert);
+ gst_clear_object(&vs->h264->gst_encoder);
+ gst_clear_object(&vs->h264->sink);
+ gst_clear_object(&vs->h264->pipeline);
+}
+
+static bool create_encoder_context(VncState *vs, int w, int h)
+{
+ g_autoptr(GstCaps) source_caps = NULL;
+ GstStateChangeReturn state_change_ret;
+
+ g_assert(vs->h264 != NULL);
+
+ if (vs->h264->sink && w == vs->h264->width && h == vs->h264->height) {
+ return TRUE;
+ }
+
+ destroy_encoder_context(vs);
+
+ vs->h264->width = w;
+ vs->h264->height = h;
+
+ vs->h264->source = gst_element_factory_make("appsrc", "source");
+ if (!vs->h264->source) {
+ VNC_DEBUG("Could not create gst source\n");
+ goto error;
+ }
+
+ vs->h264->convert = gst_element_factory_make("videoconvert", "convert");
+ if (!vs->h264->convert) {
+ VNC_DEBUG("Could not create gst convert element\n");
+ goto error;
+ }
+
+ vs->h264->gst_encoder = gst_element_factory_make("x264enc", "gst-encoder");
+ if (!vs->h264->gst_encoder) {
+ VNC_DEBUG("Could not create gst x264 encoder\n");
+ goto error;
+ }
+
+ g_object_set(
+ vs->h264->gst_encoder,
+ "tune", 4, /* zerolatency */
+ /*
+ * fix for zerolatency with novnc (without, noVNC displays
+ * green stripes)
+ */
+ "threads", 1,
+ "pass", 5, /* Constant Quality */
+ "quantizer", 26,
+ /* avoid access unit delimiters (Nal Unit Type 9) - not required */
+ "aud", false,
+ NULL);
+
+ vs->h264->sink = gst_element_factory_make("appsink", "sink");
+ if (!vs->h264->sink) {
+ VNC_DEBUG("Could not create gst sink\n");
+ goto error;
+ }
+
+ vs->h264->pipeline = gst_pipeline_new("vnc-h264-pipeline");
+ if (!vs->h264->pipeline) {
+ VNC_DEBUG("Could not create gst pipeline\n");
+ goto error;
+ }
+
+ gst_object_ref(vs->h264->source);
+ if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->source)) {
+ gst_object_unref(vs->h264->source);
+ VNC_DEBUG("Could not add source to gst pipeline\n");
+ goto error;
+ }
+
+ gst_object_ref(vs->h264->convert);
+ if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->convert)) {
+ gst_object_unref(vs->h264->convert);
+ VNC_DEBUG("Could not add convert to gst pipeline\n");
+ goto error;
+ }
+
+ gst_object_ref(vs->h264->gst_encoder);
+ if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->gst_encoder)) {
+ gst_object_unref(vs->h264->gst_encoder);
+ VNC_DEBUG("Could not add encoder to gst pipeline\n");
+ goto error;
+ }
+
+ gst_object_ref(vs->h264->sink);
+ if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->sink)) {
+ gst_object_unref(vs->h264->sink);
+ VNC_DEBUG("Could not add sink to gst pipeline\n");
+ goto error;
+ }
+
+ source_caps = gst_caps_new_simple(
+ "video/x-raw",
+ "format", G_TYPE_STRING, "BGRx",
+ "framerate", GST_TYPE_FRACTION, 33, 1,
+ "width", G_TYPE_INT, w,
+ "height", G_TYPE_INT, h,
+ NULL);
+
+ if (!source_caps) {
+ VNC_DEBUG("Could not create source caps filter\n");
+ goto error;
+ }
+
+ g_object_set(vs->h264->source, "caps", source_caps, NULL);
+
+ if (gst_element_link_many(
+ vs->h264->source,
+ vs->h264->convert,
+ vs->h264->gst_encoder,
+ vs->h264->sink,
+ NULL
+ ) != TRUE) {
+ VNC_DEBUG("Elements could not be linked.\n");
+ goto error;
+ }
+
+ /* Start playing */
+ state_change_ret = gst_element_set_state(
+ vs->h264->pipeline, GST_STATE_PLAYING);
+
+ if (state_change_ret == GST_STATE_CHANGE_FAILURE) {
+ VNC_DEBUG("Unable to set the pipeline to the playing state.\n");
+ goto error;
+ }
+
+ return TRUE;
+
+ error:
+ destroy_encoder_context(vs);
+ return FALSE;
+}
+
+bool vnc_h264_encoder_init(VncState *vs)
+{
+ g_assert(vs->h264 == NULL);
+
+ vs->h264 = g_new0(VncH264, 1);
+
+ return true;
+}
+
+/*
+ * Returns the number of generated framebuffer updates,
+ * or -1 in case of errors
+ */
+int vnc_h264_send_framebuffer_update(VncState *vs)
+{
+ int n = 0;
+ int rdb_h264_flags = 0;
+ int width, height;
+ uint8_t *src_data_ptr = NULL;
+ size_t src_data_size;
+ GstFlowReturn flow_ret = GST_FLOW_ERROR;
+ GstBuffer *src_buffer = NULL;
+
+ g_assert(vs->h264 != NULL);
+ g_assert(vs->vd != NULL);
+ g_assert(vs->vd->server != NULL);
+
+ width = pixman_image_get_width(vs->vd->server);
+ height = pixman_image_get_height(vs->vd->server);
+
+ g_assert(width == vs->client_width);
+ g_assert(height == vs->client_height);
+
+ if (vs->h264->sink) {
+ if (width != vs->h264->width || height != vs->h264->height) {
+ rdb_h264_flags = 2;
+ }
+ } else {
+ rdb_h264_flags = 2;
+ }
+
+ if (!create_encoder_context(vs, width, height)) {
+ VNC_DEBUG("Create encoder context failed\n");
+ return -1;
+ }
+
+ g_assert(vs->h264->sink != NULL);
+
+ src_data_ptr = vnc_server_fb_ptr(vs->vd, 0, 0);
+ src_data_size = width * height * VNC_SERVER_FB_BYTES;
+
+ src_buffer = gst_buffer_new_wrapped_full(
+ 0, src_data_ptr, src_data_size, 0, src_data_size, NULL, NULL);
+
+ g_signal_emit_by_name(
+ vs->h264->source, "push-buffer", src_buffer, &flow_ret);
+
+ if (flow_ret != GST_FLOW_OK) {
+ VNC_DEBUG("gst appsrc push buffer failed\n");
+ return -1;
+ }
+
+ do {
+ GstSample *sample = NULL;
+ GstMapInfo map;
+ GstBuffer *out_buffer;
+
+ /* Retrieve the buffer */
+ g_signal_emit_by_name(vs->h264->sink, "try-pull-sample", 0, &sample);
+ if (!sample) {
+ break;
+ }
+ out_buffer = gst_sample_get_buffer(sample);
+ if (gst_buffer_map(out_buffer, &map, 0)) {
+ vnc_framebuffer_update(vs, 0, 0, width, height, VNC_ENCODING_H264);
+ vnc_write_s32(vs, map.size); /* write data length */
+ vnc_write_s32(vs, rdb_h264_flags); /* write flags */
+ rdb_h264_flags = 0;
+
+ VNC_DEBUG("GST vnc_h264_update send %ld\n", map.size);
+
+ vnc_write(vs, map.data, map.size);
+
+ gst_buffer_unmap(out_buffer, &map);
+
+ n += 1;
+ } else {
+ VNC_DEBUG("unable to map sample\n");
+ }
+ gst_sample_unref(sample);
+ } while (true);
+
+ return n;
+}
+
+void vnc_h264_clear(VncState *vs)
+{
+ if (!vs->h264) {
+ return;
+ }
+
+ destroy_encoder_context(vs);
+
+ g_clear_pointer(&vs->h264, g_free);
+}
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index fcca7ec632..75a3bbed3d 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -111,7 +111,7 @@ int vnc_job_add_rect(VncJob *job, int x, int y, int w, int h)
void vnc_job_push(VncJob *job)
{
vnc_lock_queue(queue);
- if (queue->exit || QLIST_EMPTY(&job->rectangles)) {
+ if (queue->exit) {
g_free(job);
} else {
QTAILQ_INSERT_TAIL(&queue->jobs, job, next);
@@ -193,6 +193,9 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
local->zlib = orig->zlib;
local->hextile = orig->hextile;
local->zrle = orig->zrle;
+#ifdef CONFIG_GSTREAMER
+ local->h264 = orig->h264;
+#endif
local->client_width = orig->client_width;
local->client_height = orig->client_height;
}
@@ -204,6 +207,9 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
orig->zlib = local->zlib;
orig->hextile = local->hextile;
orig->zrle = local->zrle;
+#ifdef CONFIG_GSTREAMER
+ orig->h264 = local->h264;
+#endif
orig->lossy_rect = local->lossy_rect;
}
@@ -284,25 +290,40 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
vnc_write_u16(&vs, 0);
vnc_lock_display(job->vs->vd);
- QLIST_FOREACH_SAFE(entry, &job->rectangles, next, tmp) {
- int n;
-
- if (job->vs->ioc == NULL) {
- vnc_unlock_display(job->vs->vd);
- /* Copy persistent encoding data */
- vnc_async_encoding_end(job->vs, &vs);
- goto disconnected;
- }
- if (vnc_worker_clamp_rect(&vs, job, &entry->rect)) {
- n = vnc_send_framebuffer_update(&vs, entry->rect.x, entry->rect.y,
- entry->rect.w, entry->rect.h);
+ if (QLIST_EMPTY(&job->rectangles)) {
+ /* Send full screen update (used by h264 encoder) */
+ int width = pixman_image_get_width(vs.vd->server);
+ int height = pixman_image_get_height(vs.vd->server);
+ int n = vnc_send_framebuffer_update(&vs, 0, 0, width, height);
+ if (n >= 0) {
+ n_rectangles += n;
+ }
+ } else {
+ QLIST_FOREACH_SAFE(entry, &job->rectangles, next, tmp) {
+ int n;
+
+ if (job->vs->ioc == NULL) {
+ vnc_unlock_display(job->vs->vd);
+ /* Copy persistent encoding data */
+ vnc_async_encoding_end(job->vs, &vs);
+ goto disconnected;
+ }
- if (n >= 0) {
- n_rectangles += n;
+ if (vnc_worker_clamp_rect(&vs, job, &entry->rect)) {
+ n = vnc_send_framebuffer_update(
+ &vs,
+ entry->rect.x,
+ entry->rect.y,
+ entry->rect.w,
+ entry->rect.h);
+
+ if (n >= 0) {
+ n_rectangles += n;
+ }
}
+ g_free(entry);
}
- g_free(entry);
}
trace_vnc_job_nrects(&vs, job, n_rectangles);
vnc_unlock_display(job->vs->vd);
diff --git a/ui/vnc.c b/ui/vnc.c
index 9e097dc4b4..ba71589c6f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -970,6 +970,11 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
case VNC_ENCODING_ZYWRLE:
n = vnc_zywrle_send_framebuffer_update(vs, x, y, w, h);
break;
+#ifdef CONFIG_GSTREAMER
+ case VNC_ENCODING_H264:
+ n = vnc_h264_send_framebuffer_update(vs);
+ break;
+#endif
default:
vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
@@ -1152,6 +1157,14 @@ static int vnc_update_client(VncState *vs, int has_dirty)
return 0;
}
+ if (vs->vnc_encoding == VNC_ENCODING_H264) {
+ vs->job_update = vs->update;
+ vs->update = VNC_STATE_UPDATE_NONE;
+ vnc_job_push(vnc_job_new(vs)); /* fullscreen update */
+ vs->has_dirty = 0;
+ return 1;
+ }
+
vs->has_dirty += has_dirty;
if (!vnc_should_update(vs)) {
return 0;
@@ -1204,7 +1217,11 @@ static int vnc_update_client(VncState *vs, int has_dirty)
vs->job_update = vs->update;
vs->update = VNC_STATE_UPDATE_NONE;
- vnc_job_push(job);
+ if (QLIST_EMPTY(&job->rectangles)) {
+ g_free(job);
+ } else {
+ vnc_job_push(job);
+ }
vs->has_dirty = 0;
return n;
}
@@ -1324,6 +1341,10 @@ void vnc_disconnect_finish(VncState *vs)
vnc_tight_clear(vs);
vnc_zrle_clear(vs);
+#ifdef CONFIG_GSTREAMER
+ vnc_h264_clear(vs);
+#endif
+
#ifdef CONFIG_VNC_SASL
vnc_sasl_client_cleanup(vs);
#endif /* CONFIG_VNC_SASL */
@@ -2179,6 +2200,16 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
vnc_set_feature(vs, VNC_FEATURE_ZYWRLE);
vs->vnc_encoding = enc;
break;
+#ifdef CONFIG_GSTREAMER
+ case VNC_ENCODING_H264:
+ if (vnc_h264_encoder_init(vs)) {
+ vnc_set_feature(vs, VNC_FEATURE_H264);
+ vs->vnc_encoding = enc;
+ } else {
+ VNC_DEBUG("vnc_h264_encoder_init failed\n");
+ }
+ break;
+#endif
case VNC_ENCODING_DESKTOPRESIZE:
vnc_set_feature(vs, VNC_FEATURE_RESIZE);
break;
@@ -4289,6 +4320,10 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
Error *local_err = NULL;
char *id = (char *)qemu_opts_id(opts);
+#ifdef CONFIG_GSTREAMER
+ gst_init(NULL, NULL);
+#endif
+
assert(id);
vnc_display_init(id, &local_err);
if (local_err) {
diff --git a/ui/vnc.h b/ui/vnc.h
index acc53a2cc1..0fe9a9ab16 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -46,6 +46,10 @@
#include "vnc-enc-zrle.h"
#include "ui/kbd-state.h"
+#ifdef CONFIG_GSTREAMER
+#include <gst/gst.h>
+#endif
+
// #define _VNC_DEBUG 1
#ifdef _VNC_DEBUG
@@ -231,6 +235,14 @@ typedef struct VncZywrle {
int buf[VNC_ZRLE_TILE_WIDTH * VNC_ZRLE_TILE_HEIGHT];
} VncZywrle;
+#ifdef CONFIG_GSTREAMER
+typedef struct VncH264 {
+ GstElement *pipeline, *source, *gst_encoder, *sink, *convert;
+ size_t width;
+ size_t height;
+} VncH264;
+#endif
+
struct VncRect
{
int x;
@@ -344,6 +356,9 @@ struct VncState
VncHextile hextile;
VncZrle *zrle;
VncZywrle zywrle;
+#ifdef CONFIG_GSTREAMER
+ VncH264 *h264;
+#endif
Notifier mouse_mode_notifier;
@@ -404,6 +419,7 @@ enum {
#define VNC_ENCODING_TRLE 0x0000000f
#define VNC_ENCODING_ZRLE 0x00000010
#define VNC_ENCODING_ZYWRLE 0x00000011
+#define VNC_ENCODING_H264 0x00000032 /* 50 */
#define VNC_ENCODING_COMPRESSLEVEL0 0xFFFFFF00 /* -256 */
#define VNC_ENCODING_QUALITYLEVEL0 0xFFFFFFE0 /* -32 */
#define VNC_ENCODING_XCURSOR 0xFFFFFF10 /* -240 */
@@ -464,6 +480,7 @@ enum VncFeatures {
VNC_FEATURE_XVP,
VNC_FEATURE_CLIPBOARD_EXT,
VNC_FEATURE_AUDIO,
+ VNC_FEATURE_H264,
};
@@ -625,6 +642,10 @@ int vnc_zrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
int vnc_zywrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
void vnc_zrle_clear(VncState *vs);
+bool vnc_h264_encoder_init(VncState *vs);
+int vnc_h264_send_framebuffer_update(VncState *vs);
+void vnc_h264_clear(VncState *vs);
+
/* vnc-clipboard.c */
void vnc_server_cut_text_caps(VncState *vs);
void vnc_client_cut_text(VncState *vs, size_t len, uint8_t *text);
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v5 2/7] add vnc h264 encoder
2025-04-30 7:25 ` [PATCH v5 2/7] add vnc h264 encoder Dietmar Maurer
@ 2025-05-13 14:31 ` Daniel P. Berrangé
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 14:31 UTC (permalink / raw)
To: Dietmar Maurer; +Cc: marcandre.lureau, qemu-devel
On Wed, Apr 30, 2025 at 09:25:19AM +0200, Dietmar Maurer wrote:
> This patch implements H264 support for VNC. The RFB protocol
> extension is defined in:
>
> https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#open-h-264-encoding
>
> Currently the Gstreamer x264enc plugin (software encoder) is used
> to encode the video stream.
>
> The gstreamer pipe is:
>
> appsrc -> videoconvert -> x264enc -> appsink
>
> Note: videoconvert is required for RGBx to YUV420 conversion.
>
> The code still use the VNC server framebuffer change detection,
> and only encodes and sends video frames if there are changes.
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> ui/meson.build | 1 +
> ui/vnc-enc-h264.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++
> ui/vnc-jobs.c | 53 +++++++---
> ui/vnc.c | 37 ++++++-
> ui/vnc.h | 21 ++++
> 5 files changed, 350 insertions(+), 17 deletions(-)
> create mode 100644 ui/vnc-enc-h264.c
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> new file mode 100644
> index 0000000000..26e8c19270
> --- /dev/null
> +++ b/ui/vnc-enc-h264.c
> @@ -0,0 +1,255 @@
> +/*
> + * QEMU VNC display driver: H264 encoding
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "vnc.h"
> +
> +#include <gst/gst.h>
> +
> +static void destroy_encoder_context(VncState *vs)
> +{
> + gst_clear_object(&vs->h264->source);
> + gst_clear_object(&vs->h264->convert);
> + gst_clear_object(&vs->h264->gst_encoder);
> + gst_clear_object(&vs->h264->sink);
> + gst_clear_object(&vs->h264->pipeline);
> +}
> +
> +static bool create_encoder_context(VncState *vs, int w, int h)
> +{
> + g_autoptr(GstCaps) source_caps = NULL;
> + GstStateChangeReturn state_change_ret;
> +
> + g_assert(vs->h264 != NULL);
> +
> + if (vs->h264->sink && w == vs->h264->width && h == vs->h264->height) {
> + return TRUE;
> + }
> +
> + destroy_encoder_context(vs);
> +
> + vs->h264->width = w;
> + vs->h264->height = h;
> +
> + vs->h264->source = gst_element_factory_make("appsrc", "source");
> + if (!vs->h264->source) {
> + VNC_DEBUG("Could not create gst source\n");
> + goto error;
> + }
> +
> + vs->h264->convert = gst_element_factory_make("videoconvert", "convert");
> + if (!vs->h264->convert) {
> + VNC_DEBUG("Could not create gst convert element\n");
> + goto error;
> + }
> +
> + vs->h264->gst_encoder = gst_element_factory_make("x264enc", "gst-encoder");
> + if (!vs->h264->gst_encoder) {
> + VNC_DEBUG("Could not create gst x264 encoder\n");
> + goto error;
> + }
> +
> + g_object_set(
> + vs->h264->gst_encoder,
> + "tune", 4, /* zerolatency */
> + /*
> + * fix for zerolatency with novnc (without, noVNC displays
> + * green stripes)
> + */
> + "threads", 1,
> + "pass", 5, /* Constant Quality */
> + "quantizer", 26,
> + /* avoid access unit delimiters (Nal Unit Type 9) - not required */
> + "aud", false,
> + NULL);
> +
> + vs->h264->sink = gst_element_factory_make("appsink", "sink");
> + if (!vs->h264->sink) {
> + VNC_DEBUG("Could not create gst sink\n");
> + goto error;
> + }
> +
> + vs->h264->pipeline = gst_pipeline_new("vnc-h264-pipeline");
> + if (!vs->h264->pipeline) {
> + VNC_DEBUG("Could not create gst pipeline\n");
> + goto error;
> + }
> +
> + gst_object_ref(vs->h264->source);
> + if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->source)) {
> + gst_object_unref(vs->h264->source);
> + VNC_DEBUG("Could not add source to gst pipeline\n");
> + goto error;
> + }
> +
> + gst_object_ref(vs->h264->convert);
> + if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->convert)) {
> + gst_object_unref(vs->h264->convert);
> + VNC_DEBUG("Could not add convert to gst pipeline\n");
> + goto error;
> + }
> +
> + gst_object_ref(vs->h264->gst_encoder);
> + if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->gst_encoder)) {
> + gst_object_unref(vs->h264->gst_encoder);
> + VNC_DEBUG("Could not add encoder to gst pipeline\n");
> + goto error;
> + }
> +
> + gst_object_ref(vs->h264->sink);
> + if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->sink)) {
> + gst_object_unref(vs->h264->sink);
> + VNC_DEBUG("Could not add sink to gst pipeline\n");
> + goto error;
> + }
> +
> + source_caps = gst_caps_new_simple(
> + "video/x-raw",
> + "format", G_TYPE_STRING, "BGRx",
> + "framerate", GST_TYPE_FRACTION, 33, 1,
> + "width", G_TYPE_INT, w,
> + "height", G_TYPE_INT, h,
> + NULL);
> +
> + if (!source_caps) {
> + VNC_DEBUG("Could not create source caps filter\n");
> + goto error;
> + }
> +
> + g_object_set(vs->h264->source, "caps", source_caps, NULL);
> +
> + if (gst_element_link_many(
> + vs->h264->source,
> + vs->h264->convert,
> + vs->h264->gst_encoder,
> + vs->h264->sink,
> + NULL
> + ) != TRUE) {
> + VNC_DEBUG("Elements could not be linked.\n");
> + goto error;
> + }
> +
> + /* Start playing */
> + state_change_ret = gst_element_set_state(
> + vs->h264->pipeline, GST_STATE_PLAYING);
> +
> + if (state_change_ret == GST_STATE_CHANGE_FAILURE) {
> + VNC_DEBUG("Unable to set the pipeline to the playing state.\n");
> + goto error;
> + }
> +
> + return TRUE;
> +
> + error:
> + destroy_encoder_context(vs);
> + return FALSE;
> +}
> +
> +bool vnc_h264_encoder_init(VncState *vs)
> +{
> + g_assert(vs->h264 == NULL);
> +
> + vs->h264 = g_new0(VncH264, 1);
> +
> + return true;
> +}
> +
> +/*
> + * Returns the number of generated framebuffer updates,
> + * or -1 in case of errors
> + */
> +int vnc_h264_send_framebuffer_update(VncState *vs)
> +{
> + int n = 0;
> + int rdb_h264_flags = 0;
> + int width, height;
> + uint8_t *src_data_ptr = NULL;
> + size_t src_data_size;
> + GstFlowReturn flow_ret = GST_FLOW_ERROR;
> + GstBuffer *src_buffer = NULL;
> +
> + g_assert(vs->h264 != NULL);
> + g_assert(vs->vd != NULL);
> + g_assert(vs->vd->server != NULL);
> +
> + width = pixman_image_get_width(vs->vd->server);
> + height = pixman_image_get_height(vs->vd->server);
> +
> + g_assert(width == vs->client_width);
> + g_assert(height == vs->client_height);
> +
> + if (vs->h264->sink) {
> + if (width != vs->h264->width || height != vs->h264->height) {
> + rdb_h264_flags = 2;
> + }
> + } else {
> + rdb_h264_flags = 2;
> + }
> +
> + if (!create_encoder_context(vs, width, height)) {
> + VNC_DEBUG("Create encoder context failed\n");
> + return -1;
> + }
> +
> + g_assert(vs->h264->sink != NULL);
> +
> + src_data_ptr = vnc_server_fb_ptr(vs->vd, 0, 0);
> + src_data_size = width * height * VNC_SERVER_FB_BYTES;
> +
> + src_buffer = gst_buffer_new_wrapped_full(
> + 0, src_data_ptr, src_data_size, 0, src_data_size, NULL, NULL);
> +
> + g_signal_emit_by_name(
> + vs->h264->source, "push-buffer", src_buffer, &flow_ret);
> +
> + if (flow_ret != GST_FLOW_OK) {
> + VNC_DEBUG("gst appsrc push buffer failed\n");
> + return -1;
> + }
> +
> + do {
> + GstSample *sample = NULL;
> + GstMapInfo map;
> + GstBuffer *out_buffer;
> +
> + /* Retrieve the buffer */
> + g_signal_emit_by_name(vs->h264->sink, "try-pull-sample", 0, &sample);
> + if (!sample) {
> + break;
> + }
> + out_buffer = gst_sample_get_buffer(sample);
> + if (gst_buffer_map(out_buffer, &map, 0)) {
> + vnc_framebuffer_update(vs, 0, 0, width, height, VNC_ENCODING_H264);
> + vnc_write_s32(vs, map.size); /* write data length */
> + vnc_write_s32(vs, rdb_h264_flags); /* write flags */
> + rdb_h264_flags = 0;
> +
> + VNC_DEBUG("GST vnc_h264_update send %ld\n", map.size);
> +
> + vnc_write(vs, map.data, map.size);
> +
> + gst_buffer_unmap(out_buffer, &map);
> +
> + n += 1;
> + } else {
> + VNC_DEBUG("unable to map sample\n");
> + }
> + gst_sample_unref(sample);
> + } while (true);
> +
> + return n;
> +}
> +
> +void vnc_h264_clear(VncState *vs)
> +{
> + if (!vs->h264) {
> + return;
> + }
> +
> + destroy_encoder_context(vs);
> +
> + g_clear_pointer(&vs->h264, g_free);
> +}
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 9e097dc4b4..ba71589c6f 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4289,6 +4320,10 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
> Error *local_err = NULL;
> char *id = (char *)qemu_opts_id(opts);
>
> +#ifdef CONFIG_GSTREAMER
> + gst_init(NULL, NULL);
This method aborts the process on failure. We need to use
gst_init_check(NULL, NULL, &err)
and report the 'err' result with error_report() so it
integrates with QEMU's normal error reporting mechanisms.
> @@ -625,6 +642,10 @@ int vnc_zrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> int vnc_zywrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> void vnc_zrle_clear(VncState *vs);
>
> +bool vnc_h264_encoder_init(VncState *vs);
> +int vnc_h264_send_framebuffer_update(VncState *vs);
> +void vnc_h264_clear(VncState *vs);
These ought to be wrapped in '#ifdef CONFIG_GSTREAMER' too, so that is
obvious they're conditionally available, and it'll trigger a compiler
error if someone tries to call them outside a matching #ifdef
> +
> /* vnc-clipboard.c */
> void vnc_server_cut_text_caps(VncState *vs);
> void vnc_client_cut_text(VncState *vs, size_t len, uint8_t *text);
> --
> 2.39.5
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 3/7] vnc: h264: send additional frames after the display is clean
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 1/7] new configure option to enable gstreamer Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 2/7] add vnc h264 encoder Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 4/7] h264: search for available h264 encoder Dietmar Maurer
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
The vnc implementation only sends frames when it detects changes in
the server's framebuffer. This leads to artifacts when there are no
further changes, as the internal H264 encoder may still contain data.
This patch modifies the code to send a few additional frames in such
situations to flush the H264 encoder data.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc.c | 13 ++++++++++++-
ui/vnc.h | 3 +++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index ba71589c6f..975f3325e1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3234,6 +3234,7 @@ static void vnc_refresh(DisplayChangeListener *dcl)
VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
VncState *vs, *vn;
int has_dirty, rects = 0;
+ bool keep_dirty = false;
if (QTAILQ_EMPTY(&vd->clients)) {
update_displaychangelistener(&vd->dcl, VNC_REFRESH_INTERVAL_MAX);
@@ -3251,11 +3252,21 @@ static void vnc_refresh(DisplayChangeListener *dcl)
vnc_unlock_display(vd);
QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
+#ifdef CONFIG_GSTREAMER
+ if (vs->h264) {
+ if (has_dirty) {
+ vs->h264->keep_dirty = VNC_H264_KEEP_DIRTY;
+ } else if (vs->h264->keep_dirty > 0) {
+ keep_dirty = true;
+ vs->h264->keep_dirty--;
+ }
+ }
+#endif
rects += vnc_update_client(vs, has_dirty);
/* vs might be free()ed here */
}
- if (has_dirty && rects) {
+ if ((has_dirty && rects) || keep_dirty) {
vd->dcl.update_interval /= 2;
if (vd->dcl.update_interval < VNC_REFRESH_INTERVAL_BASE) {
vd->dcl.update_interval = VNC_REFRESH_INTERVAL_BASE;
diff --git a/ui/vnc.h b/ui/vnc.h
index 0fe9a9ab16..29012b75c7 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -236,10 +236,13 @@ typedef struct VncZywrle {
} VncZywrle;
#ifdef CONFIG_GSTREAMER
+/* Number of frames we send after the display is clean. */
+#define VNC_H264_KEEP_DIRTY 10
typedef struct VncH264 {
GstElement *pipeline, *source, *gst_encoder, *sink, *convert;
size_t width;
size_t height;
+ guint keep_dirty;
} VncH264;
#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v5 4/7] h264: search for available h264 encoder
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
` (2 preceding siblings ...)
2025-04-30 7:25 ` [PATCH v5 3/7] vnc: h264: send additional frames after the display is clean Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 5/7] h264: new vnc options to configure h264 at server side Dietmar Maurer
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
The search list is currently hardcoded to: ["x264enc", "openh264enc"]
x264enc: is probably the best available software encoder
openh264enc: lower quality, but available on more systems.
We restrict encoders to a known list because each encoder requires
fine tuning to get reasonable/usable results.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc-enc-h264.c | 82 ++++++++++++++++++++++++++++++++++++++---------
ui/vnc.h | 1 +
2 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 26e8c19270..191e3aeb39 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -9,6 +9,61 @@
#include <gst/gst.h>
+const char *encoder_list[] = { "x264enc", "openh264enc" };
+
+static const char *get_available_encoder(void)
+{
+ for (int i = 0; i < G_N_ELEMENTS(encoder_list); i++) {
+ GstElement *element = gst_element_factory_make(
+ encoder_list[i], "video-encoder");
+ if (element != NULL) {
+ gst_object_unref(element);
+ return encoder_list[i];
+ }
+ }
+ return NULL;
+}
+
+static GstElement *create_encoder(const char *encoder_name)
+{
+ GstElement *encoder = gst_element_factory_make(
+ encoder_name, "video-encoder");
+ if (!encoder) {
+ VNC_DEBUG("Could not create gst '%s' video encoder\n", encoder_name);
+ return NULL;
+ }
+
+ if (!strcmp(encoder_name, "x264enc")) {
+ g_object_set(
+ encoder,
+ "tune", 4, /* zerolatency */
+ /*
+ * fix for zerolatency with novnc (without,
+ * noVNC displays green stripes)
+ */
+ "threads", 1,
+ "pass", 5, /* Constant Quality */
+ "quantizer", 26,
+ /* avoid access unit delimiters (Nal Unit Type 9) - not required */
+ "aud", false,
+ NULL);
+ } else if (!strcmp(encoder_name, "openh264enc")) {
+ g_object_set(
+ encoder,
+ "usage-type", 1, /* screen content */
+ "complexity", 0, /* low, high speed */
+ "rate-control", 0, /* quality mode */
+ "qp-min", 20,
+ "qp-max", 27,
+ NULL);
+ } else {
+ VNC_DEBUG("Unknown H264 encoder name '%s' - not setting any properties",
+ encoder_name);
+ }
+
+ return encoder;
+}
+
static void destroy_encoder_context(VncState *vs)
{
gst_clear_object(&vs->h264->source);
@@ -46,26 +101,12 @@ static bool create_encoder_context(VncState *vs, int w, int h)
goto error;
}
- vs->h264->gst_encoder = gst_element_factory_make("x264enc", "gst-encoder");
+ vs->h264->gst_encoder = create_encoder(vs->h264->encoder_name);
if (!vs->h264->gst_encoder) {
VNC_DEBUG("Could not create gst x264 encoder\n");
goto error;
}
- g_object_set(
- vs->h264->gst_encoder,
- "tune", 4, /* zerolatency */
- /*
- * fix for zerolatency with novnc (without, noVNC displays
- * green stripes)
- */
- "threads", 1,
- "pass", 5, /* Constant Quality */
- "quantizer", 26,
- /* avoid access unit delimiters (Nal Unit Type 9) - not required */
- "aud", false,
- NULL);
-
vs->h264->sink = gst_element_factory_make("appsink", "sink");
if (!vs->h264->sink) {
VNC_DEBUG("Could not create gst sink\n");
@@ -150,9 +191,20 @@ static bool create_encoder_context(VncState *vs, int w, int h)
bool vnc_h264_encoder_init(VncState *vs)
{
+ const char *encoder_name;
+
g_assert(vs->h264 == NULL);
+ encoder_name = get_available_encoder();
+ if (encoder_name == NULL) {
+ VNC_DEBUG("No H264 encoder available.\n");
+ return -1;
+ }
+
vs->h264 = g_new0(VncH264, 1);
+ vs->h264->encoder_name = encoder_name;
+
+ VNC_DEBUG("Allow H264 using encoder '%s`\n", encoder_name);
return true;
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 29012b75c7..4afc68d6ec 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -239,6 +239,7 @@ typedef struct VncZywrle {
/* Number of frames we send after the display is clean. */
#define VNC_H264_KEEP_DIRTY 10
typedef struct VncH264 {
+ const char *encoder_name;
GstElement *pipeline, *source, *gst_encoder, *sink, *convert;
size_t width;
size_t height;
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v5 5/7] h264: new vnc options to configure h264 at server side
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
` (3 preceding siblings ...)
2025-04-30 7:25 ` [PATCH v5 4/7] h264: search for available h264 encoder Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 6/7] h264: add hardware encoders Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit Dietmar Maurer
6 siblings, 0 replies; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
h264: on/off (default is on)
h264-encoders: A colon separated list of allowed gstreamer
encoders. Select the first available encoder from that
list (default is "x264enc:openh264enc").
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc-enc-h264.c | 40 ++++++++++++++++++++++++++++++----------
ui/vnc.c | 29 ++++++++++++++++++++++++-----
ui/vnc.h | 6 +++++-
3 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 191e3aeb39..09b974a787 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -9,19 +9,36 @@
#include <gst/gst.h>
-const char *encoder_list[] = { "x264enc", "openh264enc" };
-
-static const char *get_available_encoder(void)
+static char *get_available_encoder(const char *encoder_list)
{
- for (int i = 0; i < G_N_ELEMENTS(encoder_list); i++) {
+ int i = 0;
+ char *ret = NULL;
+ char **encoder_array = NULL;
+ const char *encoder_name = NULL;
+
+ g_assert(encoder_list != NULL);
+
+ if (!strcmp(encoder_list, "")) {
+ /* use default list */
+ encoder_list = "x264enc:openh264enc";
+ }
+
+ encoder_array = g_strsplit(encoder_list, ":", -1);
+
+ while ((encoder_name = encoder_array[i])) {
GstElement *element = gst_element_factory_make(
- encoder_list[i], "video-encoder");
+ encoder_name, "video-encoder");
if (element != NULL) {
gst_object_unref(element);
- return encoder_list[i];
+ ret = strdup(encoder_name);
+ break;
}
+ i++;
}
- return NULL;
+
+ g_strfreev(encoder_array);
+
+ return ret;
}
static GstElement *create_encoder(const char *encoder_name)
@@ -191,14 +208,16 @@ static bool create_encoder_context(VncState *vs, int w, int h)
bool vnc_h264_encoder_init(VncState *vs)
{
- const char *encoder_name;
+ char *encoder_name;
g_assert(vs->h264 == NULL);
+ g_assert(vs->vd != NULL);
+ g_assert(vs->vd->h264_encoder_list != NULL);
- encoder_name = get_available_encoder();
+ encoder_name = get_available_encoder(vs->vd->h264_encoder_list);
if (encoder_name == NULL) {
VNC_DEBUG("No H264 encoder available.\n");
- return -1;
+ return false;
}
vs->h264 = g_new0(VncH264, 1);
@@ -302,6 +321,7 @@ void vnc_h264_clear(VncState *vs)
}
destroy_encoder_context(vs);
+ g_free(vs->h264->encoder_name);
g_clear_pointer(&vs->h264, g_free);
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 975f3325e1..c707b9da37 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2202,11 +2202,11 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
break;
#ifdef CONFIG_GSTREAMER
case VNC_ENCODING_H264:
- if (vnc_h264_encoder_init(vs)) {
- vnc_set_feature(vs, VNC_FEATURE_H264);
- vs->vnc_encoding = enc;
- } else {
- VNC_DEBUG("vnc_h264_encoder_init failed\n");
+ if (vs->vd->h264_encoder_list != NULL) { /* if h264 is enabled */
+ if (vnc_h264_encoder_init(vs)) {
+ vnc_set_feature(vs, VNC_FEATURE_H264);
+ vs->vnc_encoding = enc;
+ }
}
break;
#endif
@@ -3634,6 +3634,12 @@ static QemuOptsList qemu_vnc_opts = {
},{
.name = "power-control",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "h264",
+ .type = QEMU_OPT_BOOL,
+ },{
+ .name = "h264-encoders",
+ .type = QEMU_OPT_STRING,
},
{ /* end of list */ }
},
@@ -4196,6 +4202,19 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
+#ifdef CONFIG_GSTREAMER
+ if (qemu_opt_get_bool(opts, "h264", true)) {
+ const char *h264_encoders = qemu_opt_get(opts, "h264-encoders");
+ if (h264_encoders) {
+ vd->h264_encoder_list = h264_encoders;
+ } else {
+ vd->h264_encoder_list = ""; /* use default encoder list */
+ }
+ } else {
+ vd->h264_encoder_list = NULL; /* disable h264 */
+ }
+#endif
+
if (vnc_display_setup_auth(&vd->auth, &vd->subauth,
vd->tlscreds, password,
sasl, false, errp) < 0) {
diff --git a/ui/vnc.h b/ui/vnc.h
index 4afc68d6ec..d69ca710ab 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -188,6 +188,10 @@ struct VncDisplay
VncDisplaySASL sasl;
#endif
+#ifdef CONFIG_GSTREAMER
+ const char *h264_encoder_list;
+#endif
+
AudioState *audio_state;
};
@@ -239,7 +243,7 @@ typedef struct VncZywrle {
/* Number of frames we send after the display is clean. */
#define VNC_H264_KEEP_DIRTY 10
typedef struct VncH264 {
- const char *encoder_name;
+ char *encoder_name;
GstElement *pipeline, *source, *gst_encoder, *sink, *convert;
size_t width;
size_t height;
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v5 6/7] h264: add hardware encoders
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
` (4 preceding siblings ...)
2025-04-30 7:25 ` [PATCH v5 5/7] h264: new vnc options to configure h264 at server side Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-04-30 7:25 ` [PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit Dietmar Maurer
6 siblings, 0 replies; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
add most common hardware encoders:
- nvh264enc: for NVidia hardware
- vaapih264enc: for common AMD and Intel cards
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc-enc-h264.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 09b974a787..98055c095f 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -20,7 +20,7 @@ static char *get_available_encoder(const char *encoder_list)
if (!strcmp(encoder_list, "")) {
/* use default list */
- encoder_list = "x264enc:openh264enc";
+ encoder_list = "nvh264enc:vaapih264enc:x264enc:openh264enc";
}
encoder_array = g_strsplit(encoder_list, ":", -1);
@@ -50,7 +50,19 @@ static GstElement *create_encoder(const char *encoder_name)
return NULL;
}
- if (!strcmp(encoder_name, "x264enc")) {
+ if (!strcmp(encoder_name, "nvh264enc")) {
+ g_object_set(
+ encoder,
+ "preset", 8, /* p1 - fastest */
+ "multi-pass", 1, /* multipass disabled */
+ "tune", 2, /* low latency */
+ "zerolatency", true, /* low latency */
+ /* avoid access unit delimiters (Nal Unit Type 9) - not required */
+ "aud", false,
+ NULL);
+ } else if (!strcmp(encoder_name, "vaapih264enc")) {
+ g_object_set(encoder, "tune", 1, NULL); /* high compression */
+ } else if (!strcmp(encoder_name, "x264enc")) {
g_object_set(
encoder,
"tune", 4, /* zerolatency */
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit
2025-04-30 7:25 [PATCH v5 0/7] Add VNC Open H.264 Encoding Dietmar Maurer
` (5 preceding siblings ...)
2025-04-30 7:25 ` [PATCH v5 6/7] h264: add hardware encoders Dietmar Maurer
@ 2025-04-30 7:25 ` Dietmar Maurer
2025-05-13 14:34 ` Daniel P. Berrangé
6 siblings, 1 reply; 12+ messages in thread
From: Dietmar Maurer @ 2025-04-30 7:25 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: Dietmar Maurer
Some encoders can hang indefinitely (i.e. nvh264enc) if
the pipeline is not stopped before it is destroyed
(Observed on Debian bookworm).
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
include/system/system.h | 1 +
include/ui/console.h | 1 +
system/runstate.c | 2 ++
system/vl.c | 7 +++++++
ui/vnc-enc-h264.c | 18 ++++++++++++++++++
ui/vnc.c | 15 +++++++++++++++
6 files changed, 44 insertions(+)
diff --git a/include/system/system.h b/include/system/system.h
index a7effe7dfd..9226e60050 100644
--- a/include/system/system.h
+++ b/include/system/system.h
@@ -109,6 +109,7 @@ bool defaults_enabled(void);
void qemu_init(int argc, char **argv);
int qemu_main_loop(void);
void qemu_cleanup(int);
+void qemu_cleanup_displays(void);
extern QemuOptsList qemu_legacy_drive_opts;
extern QemuOptsList qemu_common_drive_opts;
diff --git a/include/ui/console.h b/include/ui/console.h
index 46b3128185..ff46e9fe98 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -458,6 +458,7 @@ int vnc_display_password(const char *id, const char *password);
int vnc_display_pw_expire(const char *id, time_t expires);
void vnc_parse(const char *str);
int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
+void vnc_cleanup(void);
bool vnc_display_reload_certs(const char *id, Error **errp);
bool vnc_display_update(DisplayUpdateOptionsVNC *arg, Error **errp);
diff --git a/system/runstate.c b/system/runstate.c
index 272801d307..0cb3ba5ec1 100644
--- a/system/runstate.c
+++ b/system/runstate.c
@@ -924,6 +924,8 @@ void qemu_cleanup(int status)
job_cancel_sync_all();
bdrv_close_all();
+ qemu_cleanup_displays();
+
/* vhost-user must be cleaned up before chardevs. */
tpm_cleanup();
net_cleanup();
diff --git a/system/vl.c b/system/vl.c
index c17945c493..a781ebd77b 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2679,6 +2679,13 @@ static void qemu_maybe_daemonize(const char *pid_file)
}
}
+void qemu_cleanup_displays(void)
+{
+#ifdef CONFIG_VNC
+ vnc_cleanup();
+#endif
+}
+
static void qemu_init_displays(void)
{
DisplayState *ds;
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 98055c095f..6618f156b4 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -95,6 +95,24 @@ static GstElement *create_encoder(const char *encoder_name)
static void destroy_encoder_context(VncState *vs)
{
+ GstStateChangeReturn state_change_ret;
+
+ VNC_DEBUG("Destroy h264 context.\n");
+
+ /*
+ * Some encoders can hang indefinitely (i.e. nvh264enc) if
+ * the pipeline is not stopped before it is destroyed
+ * (Observed on Debian bookworm).
+ */
+ if (vs->h264->pipeline != NULL) {
+ state_change_ret = gst_element_set_state(
+ vs->h264->pipeline, GST_STATE_NULL);
+
+ if (state_change_ret == GST_STATE_CHANGE_FAILURE) {
+ VNC_DEBUG("Unable to stop the GST pipeline\n");
+ }
+ }
+
gst_clear_object(&vs->h264->source);
gst_clear_object(&vs->h264->convert);
gst_clear_object(&vs->h264->gst_encoder);
diff --git a/ui/vnc.c b/ui/vnc.c
index c707b9da37..578d9eea32 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4368,6 +4368,21 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
return 0;
}
+void vnc_cleanup(void)
+{
+ VncDisplay *vd;
+ VncState *vs;
+
+ QTAILQ_FOREACH(vd, &vnc_displays, next) {
+ QTAILQ_FOREACH(vs, &vd->clients, next) {
+#ifdef CONFIG_GSTREAMER
+ /* correctly close all h264 encoder pipelines */
+ vnc_h264_clear(vs);
+#endif
+ }
+ }
+}
+
static void vnc_register_config(void)
{
qemu_add_opts(&qemu_vnc_opts);
--
2.39.5
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit
2025-04-30 7:25 ` [PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit Dietmar Maurer
@ 2025-05-13 14:34 ` Daniel P. Berrangé
0 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-05-13 14:34 UTC (permalink / raw)
To: Dietmar Maurer; +Cc: marcandre.lureau, qemu-devel
On Wed, Apr 30, 2025 at 09:25:24AM +0200, Dietmar Maurer wrote:
> Some encoders can hang indefinitely (i.e. nvh264enc) if
> the pipeline is not stopped before it is destroyed
> (Observed on Debian bookworm).
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> include/system/system.h | 1 +
> include/ui/console.h | 1 +
> system/runstate.c | 2 ++
> system/vl.c | 7 +++++++
> ui/vnc-enc-h264.c | 18 ++++++++++++++++++
> ui/vnc.c | 15 +++++++++++++++
> 6 files changed, 44 insertions(+)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index c707b9da37..578d9eea32 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4368,6 +4368,21 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
> return 0;
> }
>
> +void vnc_cleanup(void)
> +{
> + VncDisplay *vd;
> + VncState *vs;
> +
> + QTAILQ_FOREACH(vd, &vnc_displays, next) {
> + QTAILQ_FOREACH(vs, &vd->clients, next) {
> +#ifdef CONFIG_GSTREAMER
> + /* correctly close all h264 encoder pipelines */
> + vnc_h264_clear(vs);
> +#endif
How is this safe to do ?
If we have an active client, we have to assume that the jobs
thread may be in the middle of processing an update, and thus
the jobs thread wil be using the h264 encoder. If we blindly
tear down the encoder I think we're liable to trigger
non-deterministic crashes in QEMU in the vnc jobs thread
accessing free'd memory.
> + }
> + }
> +}
> +
> static void vnc_register_config(void)
> {
> qemu_add_opts(&qemu_vnc_opts);
> --
> 2.39.5
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 12+ messages in thread