* [PATCH v2 1/6] new configure option to enable gstreamer
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
2025-04-15 7:25 ` Marc-André Lureau
2025-04-10 11:22 ` [PATCH v2 2/6] add vnc h264 encoder Dietmar Maurer
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +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 | 5 ++++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 41f68d3806..28ca37855a 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_block
+ 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())
@@ -4836,6 +4845,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..b0c273d61e 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 ;;
- *) return 1 ;;
+ --enable-gstreamer) printf "%s" -Dgstreamer=enabled ;;
+ --disable-gstreamer) printf "%s" -Dgstreamer=disabled ;;
+ *) return 1 ;;
esac
}
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/6] new configure option to enable gstreamer
2025-04-10 11:22 ` [PATCH v2 1/6] new configure option to enable gstreamer Dietmar Maurer
@ 2025-04-15 7:25 ` Marc-André Lureau
0 siblings, 0 replies; 9+ messages in thread
From: Marc-André Lureau @ 2025-04-15 7:25 UTC (permalink / raw)
To: Dietmar Maurer; +Cc: qemu-devel
Hi
On Thu, Apr 10, 2025 at 3:22 PM Dietmar Maurer <dietmar@proxmox.com> 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 | 5 ++++-
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 41f68d3806..28ca37855a 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_block
rather "or have_system"
otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> + 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())
> @@ -4836,6 +4845,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..b0c273d61e 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 ;;
> - *) return 1 ;;
> + --enable-gstreamer) printf "%s" -Dgstreamer=enabled ;;
> + --disable-gstreamer) printf "%s" -Dgstreamer=disabled ;;
> + *) return 1 ;;
> esac
> }
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/6] add vnc h264 encoder
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 1/6] new configure option to enable gstreamer Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
2025-04-15 7:25 ` Marc-André Lureau
2025-04-10 11:22 ` [PATCH v2 3/6] vnc: h264: send additional frames after the display is clean Dietmar Maurer
` (3 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +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 | 269 ++++++++++++++++++++++++++++++++++++++++++++++
ui/vnc-jobs.c | 49 ++++++---
ui/vnc.c | 21 ++++
ui/vnc.h | 21 ++++
5 files changed, 346 insertions(+), 15 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..ca8e206335
--- /dev/null
+++ b/ui/vnc-enc-h264.c
@@ -0,0 +1,269 @@
+#include "qemu/osdep.h"
+#include "vnc.h"
+
+#include <gst/gst.h>
+
+static void libavcodec_destroy_encoder_context(VncState *vs)
+{
+ if (!vs->h264) {
+ return;
+ }
+
+ if (vs->h264->source) {
+ gst_object_unref(vs->h264->source);
+ vs->h264->source = NULL;
+ }
+
+ if (vs->h264->convert) {
+ gst_object_unref(vs->h264->convert);
+ vs->h264->convert = NULL;
+ }
+
+ if (vs->h264->gst_encoder) {
+ gst_object_unref(vs->h264->gst_encoder);
+ vs->h264->sink = NULL;
+ }
+
+ if (vs->h264->sink) {
+ gst_object_unref(vs->h264->sink);
+ vs->h264->sink = NULL;
+ }
+
+ if (vs->h264->pipeline) {
+ gst_object_unref(vs->h264->pipeline);
+ vs->h264->pipeline = NULL;
+ }
+}
+
+static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
+{
+ g_assert(vs->h264 != NULL);
+
+ if (vs->h264->sink) {
+ if (w != vs->h264->width || h != vs->h264->height) {
+ libavcodec_destroy_encoder_context(vs);
+ }
+ }
+
+ if (vs->h264->sink) {
+ return TRUE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ vs->h264->convert = gst_element_factory_make("videoconvert", "convert");
+ if (!vs->h264->convert) {
+ VNC_DEBUG("Could not create gst convert element\n");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ g_object_set(vs->h264->gst_encoder, "tune", 4, NULL); /* zerolatency */
+ /* fix for zerolatency with novnc (without, noVNC displays green stripes) */
+ g_object_set(vs->h264->gst_encoder, "threads", 1, NULL);
+
+ g_object_set(vs->h264->gst_encoder, "pass", 5, NULL); /* Constant Quality */
+ g_object_set(vs->h264->gst_encoder, "quantizer", 26, NULL);
+
+ /* avoid access unit delimiters (Nal Unit Type 9) - not required */
+ g_object_set(vs->h264->gst_encoder, "aud", false, NULL);
+
+ vs->h264->sink = gst_element_factory_make("appsink", "sink");
+ if (!vs->h264->sink) {
+ VNC_DEBUG("Could not create gst sink\n");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ vs->h264->pipeline = gst_pipeline_new("vnc-h264-pipeline");
+ if (!vs->h264->pipeline) {
+ VNC_DEBUG("Could not create gst pipeline\n");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ GstCaps *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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ g_object_set(vs->h264->source, "caps", source_caps, NULL);
+ gst_caps_unref(source_caps);
+
+ 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");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ /* Start playing */
+ int ret = gst_element_set_state(vs->h264->pipeline, GST_STATE_PLAYING);
+ if (ret == GST_STATE_CHANGE_FAILURE) {
+ VNC_DEBUG("Unable to set the pipeline to the playing state.\n");
+ libavcodec_destroy_encoder_context(vs);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+int vnc_h264_encoder_init(VncState *vs)
+{
+ g_assert(vs->h264 == NULL);
+
+ vs->h264 = g_malloc0(sizeof(VncH264));
+
+ return 0;
+}
+
+int vnc_h264_send_framebuffer_update(
+ VncState *vs,
+ int _x,
+ int _y,
+ int _w,
+ int _h,
+) {
+ g_assert(vs->h264 != NULL);
+ g_assert(vs->vd != NULL);
+ g_assert(vs->vd->server != NULL);
+
+ int width = pixman_image_get_width(vs->vd->server);
+ int height = pixman_image_get_height(vs->vd->server);
+
+ g_assert(width == vs->client_width);
+ g_assert(height == vs->client_height);
+
+ int rdb_h264_flags = 0;
+
+ if (vs->h264->sink) {
+ if (width != vs->h264->width || height != vs->h264->height) {
+ rdb_h264_flags = 2;
+ }
+ } else {
+ rdb_h264_flags = 2;
+ }
+
+ if (!libavcodec_create_encoder_context(vs, width, height)) {
+ VNC_DEBUG("Create encoder context failed\n");
+ return -1;
+ }
+
+ g_assert(vs->h264->sink != NULL);
+
+ uint8_t *src_data_ptr = vnc_server_fb_ptr(vs->vd, 0, 0);
+ size_t src_data_size = width * height * VNC_SERVER_FB_BYTES;
+
+ GstBuffer *buffer = gst_buffer_new_wrapped_full(
+ 0, src_data_ptr, src_data_size, 0, src_data_size, NULL, NULL);
+
+ GstFlowReturn ret;
+ g_signal_emit_by_name(vs->h264->source, "push-buffer", buffer, &ret);
+
+ if (ret != GST_FLOW_OK) {
+ VNC_DEBUG("gst appsrc push buffer failed\n");
+ return -1;
+ }
+
+ GstSample *sample;
+ /* Retrieve the buffer */
+ g_signal_emit_by_name(vs->h264->sink, "pull-sample", &sample);
+ if (sample) {
+ GstBuffer *buf = gst_sample_get_buffer(sample);
+ GstMapInfo map;
+ if (gst_buffer_map(buf, &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(buf, &map);
+
+ return 1;
+ } else {
+ VNC_DEBUG("unable to map sample\n");
+ }
+ gst_sample_unref(sample);
+ return 1;
+ } else {
+ VNC_DEBUG("gst no data\n");
+ return 0;
+ }
+}
+
+void vnc_h264_clear(VncState *vs)
+{
+ if (!vs->h264) {
+ return;
+ }
+
+ libavcodec_destroy_encoder_context(vs);
+
+ g_free(vs->h264);
+ vs->h264 = NULL;
+}
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index fcca7ec632..853a547d9a 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -193,6 +193,7 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
local->zlib = orig->zlib;
local->hextile = orig->hextile;
local->zrle = orig->zrle;
+ local->h264 = orig->h264;
local->client_width = orig->client_width;
local->client_height = orig->client_height;
}
@@ -204,6 +205,7 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
orig->zlib = local->zlib;
orig->hextile = local->hextile;
orig->zrle = local->zrle;
+ orig->h264 = local->h264;
orig->lossy_rect = local->lossy_rect;
}
@@ -284,25 +286,42 @@ 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 (vs.vnc_encoding == VNC_ENCODING_H264) {
+ 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;
+ }
+ QLIST_FOREACH_SAFE(entry, &job->rectangles, next, tmp) {
+ g_free(entry);
+ }
+ } 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 9241caaad9..2e60b55e47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -972,6 +972,9 @@ 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;
+ case VNC_ENCODING_H264:
+ n = vnc_h264_send_framebuffer_update(vs, x, y, w, h);
+ break;
default:
vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
@@ -1326,6 +1329,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 */
@@ -2181,6 +2188,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) == 0) {
+ 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;
@@ -4291,6 +4308,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..7e232f7dac 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);
+int vnc_h264_encoder_init(VncState *vs);
+int vnc_h264_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
+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] 9+ messages in thread* Re: [PATCH v2 2/6] add vnc h264 encoder
2025-04-10 11:22 ` [PATCH v2 2/6] add vnc h264 encoder Dietmar Maurer
@ 2025-04-15 7:25 ` Marc-André Lureau
0 siblings, 0 replies; 9+ messages in thread
From: Marc-André Lureau @ 2025-04-15 7:25 UTC (permalink / raw)
To: Dietmar Maurer; +Cc: qemu-devel
Hi
On Thu, Apr 10, 2025 at 3:22 PM Dietmar Maurer <dietmar@proxmox.com> 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 | 269 ++++++++++++++++++++++++++++++++++++++++++++++
> ui/vnc-jobs.c | 49 ++++++---
> ui/vnc.c | 21 ++++
> ui/vnc.h | 21 ++++
> 5 files changed, 346 insertions(+), 15 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..ca8e206335
> --- /dev/null
> +++ b/ui/vnc-enc-h264.c
> @@ -0,0 +1,269 @@
In general, a license header is required.
> +#include "qemu/osdep.h"
> +#include "vnc.h"
> +
> +#include <gst/gst.h>
> +
> +static void libavcodec_destroy_encoder_context(VncState *vs)
please squash the next patch removing the libavcodec prefix.
> +{
> + if (!vs->h264) {
> + return;
> + }
> +
> + if (vs->h264->source) {
> + gst_object_unref(vs->h264->source);
> + vs->h264->source = NULL;
> + }
> +
> + if (vs->h264->convert) {
> + gst_object_unref(vs->h264->convert);
> + vs->h264->convert = NULL;
> + }
> +
> + if (vs->h264->gst_encoder) {
> + gst_object_unref(vs->h264->gst_encoder);
> + vs->h264->sink = NULL;
> + }
> +
> + if (vs->h264->sink) {
> + gst_object_unref(vs->h264->sink);
> + vs->h264->sink = NULL;
> + }
> +
> + if (vs->h264->pipeline) {
> + gst_object_unref(vs->h264->pipeline);
> + vs->h264->pipeline = NULL;
> + }
Let's save a few lines:
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);
(We may also rely on the fact that the pipeline will hold the elements
and avoid ref-counting them)
> +}
> +
> +static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
> +{
> + g_assert(vs->h264 != NULL);
> +
> + if (vs->h264->sink) {
> + if (w != vs->h264->width || h != vs->h264->height) {
> + libavcodec_destroy_encoder_context(vs);
> + }
> + }
> +
> + if (vs->h264->sink) {
> + return TRUE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
Given that you always destroy the context on error, I suggest you add
a "goto error" instead.
> + }
> +
> + vs->h264->convert = gst_element_factory_make("videoconvert", "convert");
> + if (!vs->h264->convert) {
> + VNC_DEBUG("Could not create gst convert element\n");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + g_object_set(vs->h264->gst_encoder, "tune", 4, NULL); /* zerolatency */
> + /* fix for zerolatency with novnc (without, noVNC displays green stripes) */
> + g_object_set(vs->h264->gst_encoder, "threads", 1, NULL);
> +
> + g_object_set(vs->h264->gst_encoder, "pass", 5, NULL); /* Constant Quality */
> + g_object_set(vs->h264->gst_encoder, "quantizer", 26, NULL);
> +
> + /* avoid access unit delimiters (Nal Unit Type 9) - not required */
> + g_object_set(vs->h264->gst_encoder, "aud", false, NULL);
Why not use a single g_object_set()?
> +
> + vs->h264->sink = gst_element_factory_make("appsink", "sink");
> + if (!vs->h264->sink) {
> + VNC_DEBUG("Could not create gst sink\n");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + vs->h264->pipeline = gst_pipeline_new("vnc-h264-pipeline");
> + if (!vs->h264->pipeline) {
> + VNC_DEBUG("Could not create gst pipeline\n");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + GstCaps *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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + g_object_set(vs->h264->source, "caps", source_caps, NULL);
> + gst_caps_unref(source_caps);
Let's declare it with g_autoptr
> +
> + 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");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + /* Start playing */
> + int ret = gst_element_set_state(vs->h264->pipeline, GST_STATE_PLAYING);
> + if (ret == GST_STATE_CHANGE_FAILURE) {
> + VNC_DEBUG("Unable to set the pipeline to the playing state.\n");
> + libavcodec_destroy_encoder_context(vs);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +int vnc_h264_encoder_init(VncState *vs)
> +{
> + g_assert(vs->h264 == NULL);
> +
> + vs->h264 = g_malloc0(sizeof(VncH264));
g_new0(VncH264, 1)
> +
> + return 0;
> +}
> +
> +int vnc_h264_send_framebuffer_update(
> + VncState *vs,
> + int _x,
> + int _y,
> + int _w,
> + int _h,
> +) {
../ui/vnc-enc-h264.c:168:1: error: expected declaration specifiers or
‘...’ before ‘)’ token
The function returns an 'int', with value -1, 0 or 1. This needs to be
documented.
Otherwise switch to a bool, with true for success.
> + g_assert(vs->h264 != NULL);
> + g_assert(vs->vd != NULL);
> + g_assert(vs->vd->server != NULL);
> +
> + int width = pixman_image_get_width(vs->vd->server);
> + int height = pixman_image_get_height(vs->vd->server);
> +
> + g_assert(width == vs->client_width);
> + g_assert(height == vs->client_height);
> +
> + int rdb_h264_flags = 0;
> +
> + if (vs->h264->sink) {
> + if (width != vs->h264->width || height != vs->h264->height) {
> + rdb_h264_flags = 2;
> + }
> + } else {
> + rdb_h264_flags = 2;
> + }
> +
> + if (!libavcodec_create_encoder_context(vs, width, height)) {
> + VNC_DEBUG("Create encoder context failed\n");
> + return -1;
> + }
> +
> + g_assert(vs->h264->sink != NULL);
> +
> + uint8_t *src_data_ptr = vnc_server_fb_ptr(vs->vd, 0, 0);
> + size_t src_data_size = width * height * VNC_SERVER_FB_BYTES;
> +
> + GstBuffer *buffer = gst_buffer_new_wrapped_full(
> + 0, src_data_ptr, src_data_size, 0, src_data_size, NULL, NULL);
> +
> + GstFlowReturn ret;
There is a bit too much mixed declaration in your code. From
docs/devel/style.rst:
Declarations
============
Mixed declarations (interleaving statements and declarations within
blocks) are generally not allowed; declarations should be at the beginning
...
> + g_signal_emit_by_name(vs->h264->source, "push-buffer", buffer, &ret);
> +
> + if (ret != GST_FLOW_OK) {
> + VNC_DEBUG("gst appsrc push buffer failed\n");
> + return -1;
> + }
> +
> + GstSample *sample;
> + /* Retrieve the buffer */
> + g_signal_emit_by_name(vs->h264->sink, "pull-sample", &sample);
Is there a guarantee that a single sample is produced per frame? I
wonder if this code would be more robust by handling samples in a
loop.
> + if (sample) {
> + GstBuffer *buf = gst_sample_get_buffer(sample);
> + GstMapInfo map;
> + if (gst_buffer_map(buf, &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(buf, &map);
> +
> + return 1;
> + } else {
> + VNC_DEBUG("unable to map sample\n");
Shouldn't this be an error?
> + }
> + gst_sample_unref(sample);
> + return 1;
> + } else {
> + VNC_DEBUG("gst no data\n");
I would use a "trace" for this.
> + return 0;
> + }
> +}
> +
> +void vnc_h264_clear(VncState *vs)
> +{
> + if (!vs->h264) {
> + return;
> + }
This condition is unneeded here.
> +
> + libavcodec_destroy_encoder_context(vs);
> +
> + g_free(vs->h264);
> + vs->h264 = NULL;
g_clear_pointer(&vs->h264, g_free);
> +}
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
You could split those changes in a different patch.
> index fcca7ec632..853a547d9a 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -193,6 +193,7 @@ static void vnc_async_encoding_start(VncState *orig, VncState *local)
> local->zlib = orig->zlib;
> local->hextile = orig->hextile;
> local->zrle = orig->zrle;
> + local->h264 = orig->h264;
> local->client_width = orig->client_width;
> local->client_height = orig->client_height;
> }
> @@ -204,6 +205,7 @@ static void vnc_async_encoding_end(VncState *orig, VncState *local)
> orig->zlib = local->zlib;
> orig->hextile = local->hextile;
> orig->zrle = local->zrle;
> + orig->h264 = local->h264;
> orig->lossy_rect = local->lossy_rect;
> }
>
> @@ -284,25 +286,42 @@ 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 (vs.vnc_encoding == VNC_ENCODING_H264) {
> + 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;
> + }
> + QLIST_FOREACH_SAFE(entry, &job->rectangles, next, tmp) {
> + g_free(entry);
> + }
> + } 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 9241caaad9..2e60b55e47 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -972,6 +972,9 @@ 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;
> + case VNC_ENCODING_H264:
> + n = vnc_h264_send_framebuffer_update(vs, x, y, w, h);
> + break;
> default:
> vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
> n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
> @@ -1326,6 +1329,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 */
> @@ -2181,6 +2188,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) == 0) {
> + 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;
> @@ -4291,6 +4308,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);
Have you tried to initialize gst during argument processing (so we get
--gst- options)?
> +#endif
> +
> assert(id);
> vnc_display_init(id, &local_err);
> if (local_err) {
> diff --git a/ui/vnc.h b/ui/vnc.h
> index acc53a2cc1..7e232f7dac 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);
>
> +int vnc_h264_encoder_init(VncState *vs);
> +int vnc_h264_send_framebuffer_update(VncState *vs, int x, int y, int w, int h);
> +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 [flat|nested] 9+ messages in thread
* [PATCH v2 3/6] vnc: h264: send additional frames after the display is clean
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 1/6] new configure option to enable gstreamer Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 2/6] add vnc h264 encoder Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 4/6] h264: remove wrong libavcodec_ prefix from function names Dietmar Maurer
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: Dietmar Maurer
The H264 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 | 25 ++++++++++++++++++++++++-
ui/vnc.h | 3 +++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 2e60b55e47..4ba0b715fd 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3239,7 +3239,30 @@ static void vnc_refresh(DisplayChangeListener *dcl)
vnc_unlock_display(vd);
QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
- rects += vnc_update_client(vs, has_dirty);
+ int client_dirty = has_dirty;
+ if (vs->h264) {
+ if (client_dirty) {
+ vs->h264->keep_dirty = VNC_H264_KEEP_DIRTY;
+ } else {
+ if (vs->h264->keep_dirty > 0) {
+ client_dirty = 1;
+ vs->h264->keep_dirty--;
+ }
+ }
+ }
+
+ int count = vnc_update_client(vs, client_dirty);
+ rects += count;
+
+ if (vs->h264 && !count && vs->h264->keep_dirty) {
+ VncJob *job = vnc_job_new(vs);
+ int height = pixman_image_get_height(vd->server);
+ int width = pixman_image_get_width(vd->server);
+ vs->job_update = vs->update;
+ vs->update = VNC_STATE_UPDATE_NONE;
+ vnc_job_add_rect(job, 0, 0, width, height);
+ vnc_job_push(job);
+ }
/* vs might be free()ed here */
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 7e232f7dac..e1b81d6bcc 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] 9+ messages in thread* [PATCH v2 4/6] h264: remove wrong libavcodec_ prefix from function names
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
` (2 preceding siblings ...)
2025-04-10 11:22 ` [PATCH v2 3/6] vnc: h264: send additional frames after the display is clean Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 5/6] h264: search for available h264 encoder Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 6/6] h264: new vnc option to configure h264 at server side Dietmar Maurer
5 siblings, 0 replies; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: Dietmar Maurer
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc-enc-h264.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index ca8e206335..9e01b8a548 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -3,7 +3,7 @@
#include <gst/gst.h>
-static void libavcodec_destroy_encoder_context(VncState *vs)
+static void destroy_encoder_context(VncState *vs)
{
if (!vs->h264) {
return;
@@ -35,13 +35,13 @@ static void libavcodec_destroy_encoder_context(VncState *vs)
}
}
-static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
+static bool create_encoder_context(VncState *vs, int w, int h)
{
g_assert(vs->h264 != NULL);
if (vs->h264->sink) {
if (w != vs->h264->width || h != vs->h264->height) {
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
}
}
@@ -55,21 +55,21 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
vs->h264->source = gst_element_factory_make("appsrc", "source");
if (!vs->h264->source) {
VNC_DEBUG("Could not create gst source\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
vs->h264->convert = gst_element_factory_make("videoconvert", "convert");
if (!vs->h264->convert) {
VNC_DEBUG("Could not create gst convert element\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
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");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -86,14 +86,14 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
vs->h264->sink = gst_element_factory_make("appsink", "sink");
if (!vs->h264->sink) {
VNC_DEBUG("Could not create gst sink\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
vs->h264->pipeline = gst_pipeline_new("vnc-h264-pipeline");
if (!vs->h264->pipeline) {
VNC_DEBUG("Could not create gst pipeline\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -101,7 +101,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
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");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -109,7 +109,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
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");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -117,7 +117,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
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");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -125,7 +125,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
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");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -139,7 +139,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
if (!source_caps) {
VNC_DEBUG("Could not create source caps filter\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -154,7 +154,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
NULL
) != TRUE) {
VNC_DEBUG("Elements could not be linked.\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -162,7 +162,7 @@ static bool libavcodec_create_encoder_context(VncState *vs, int w, int h)
int ret = gst_element_set_state(vs->h264->pipeline, GST_STATE_PLAYING);
if (ret == GST_STATE_CHANGE_FAILURE) {
VNC_DEBUG("Unable to set the pipeline to the playing state.\n");
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
return FALSE;
}
@@ -183,7 +183,7 @@ int vnc_h264_send_framebuffer_update(
int _x,
int _y,
int _w,
- int _h,
+ int _h
) {
g_assert(vs->h264 != NULL);
g_assert(vs->vd != NULL);
@@ -205,7 +205,7 @@ int vnc_h264_send_framebuffer_update(
rdb_h264_flags = 2;
}
- if (!libavcodec_create_encoder_context(vs, width, height)) {
+ if (!create_encoder_context(vs, width, height)) {
VNC_DEBUG("Create encoder context failed\n");
return -1;
}
@@ -262,7 +262,7 @@ void vnc_h264_clear(VncState *vs)
return;
}
- libavcodec_destroy_encoder_context(vs);
+ destroy_encoder_context(vs);
g_free(vs->h264);
vs->h264 = NULL;
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 5/6] h264: search for available h264 encoder
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
` (3 preceding siblings ...)
2025-04-10 11:22 ` [PATCH v2 4/6] h264: remove wrong libavcodec_ prefix from function names Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
2025-04-10 11:22 ` [PATCH v2 6/6] h264: new vnc option to configure h264 at server side Dietmar Maurer
5 siblings, 0 replies; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +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 | 76 +++++++++++++++++++++++++++++++++++++++--------
ui/vnc.h | 1 +
2 files changed, 65 insertions(+), 12 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 9e01b8a548..3eabfc2cfe 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -3,6 +3,60 @@
#include <gst/gst.h>
+const char *encoder_list[] = { "x264enc", "openh264enc", NULL };
+
+static const char *get_available_encoder(void)
+{
+ int i = 0;
+ do {
+ const char *encoder_name = encoder_list[i];
+ if (encoder_name == NULL) {
+ break;
+ }
+ GstElement *element = gst_element_factory_make(
+ encoder_name, "video-encoder");
+ if (element != NULL) {
+ gst_object_unref(element);
+ return encoder_name;
+ }
+ i = i + 1;
+ } while (true);
+
+ 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, NULL); /* zerolatency */
+ /*
+ * fix for zerolatency with novnc (without,
+ * noVNC displays green stripes)
+ */
+ g_object_set(encoder, "threads", 1, NULL);
+ g_object_set(encoder, "pass", 5, NULL); /* Constant Quality */
+ g_object_set(encoder, "quantizer", 26, NULL);
+ /* avoid access unit delimiters (Nal Unit Type 9) - not required */
+ g_object_set(encoder, "aud", false, NULL);
+ } else if (!strcmp(encoder_name, "openh264enc")) {
+ g_object_set(encoder, "usage-type", 1, NULL); /* screen content */
+ g_object_set(encoder, "complexity", 2, NULL); /* high */
+ g_object_set(encoder, "rate-control", 2, NULL); /* off (buffer) */
+ } else {
+ VNC_DEBUG("Unknown H264 encoder name '%s' - no setting any properties",
+ encoder_name);
+ }
+
+ return encoder;
+}
+
static void destroy_encoder_context(VncState *vs)
{
if (!vs->h264) {
@@ -66,23 +120,12 @@ static bool create_encoder_context(VncState *vs, int w, int h)
return FALSE;
}
- 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");
destroy_encoder_context(vs);
return FALSE;
}
- g_object_set(vs->h264->gst_encoder, "tune", 4, NULL); /* zerolatency */
- /* fix for zerolatency with novnc (without, noVNC displays green stripes) */
- g_object_set(vs->h264->gst_encoder, "threads", 1, NULL);
-
- g_object_set(vs->h264->gst_encoder, "pass", 5, NULL); /* Constant Quality */
- g_object_set(vs->h264->gst_encoder, "quantizer", 26, NULL);
-
- /* avoid access unit delimiters (Nal Unit Type 9) - not required */
- g_object_set(vs->h264->gst_encoder, "aud", false, NULL);
-
vs->h264->sink = gst_element_factory_make("appsink", "sink");
if (!vs->h264->sink) {
VNC_DEBUG("Could not create gst sink\n");
@@ -173,7 +216,16 @@ int vnc_h264_encoder_init(VncState *vs)
{
g_assert(vs->h264 == NULL);
+ const char *encoder_name = get_available_encoder();
+ if (encoder_name == NULL) {
+ VNC_DEBUG("No H264 encoder available.\n");
+ return -1;
+ }
+
vs->h264 = g_malloc0(sizeof(VncH264));
+ vs->h264->encoder_name = encoder_name;
+
+ VNC_DEBUG("Allow H264 using encoder '%s`\n", encoder_name);
return 0;
}
diff --git a/ui/vnc.h b/ui/vnc.h
index e1b81d6bcc..f39dbe21aa 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] 9+ messages in thread* [PATCH v2 6/6] h264: new vnc option to configure h264 at server side
2025-04-10 11:22 [PATCH v2 0/6] Add VNC Open H.264 Encoding Dietmar Maurer
` (4 preceding siblings ...)
2025-04-10 11:22 ` [PATCH v2 5/6] h264: search for available h264 encoder Dietmar Maurer
@ 2025-04-10 11:22 ` Dietmar Maurer
5 siblings, 0 replies; 9+ messages in thread
From: Dietmar Maurer @ 2025-04-10 11:22 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau; +Cc: Dietmar Maurer
Values can be 'on', 'off', or a space sparated list of
allowed gstreamer encoders.
- on: automatically select the encoder
- off: disbale h264
- encoder-list: select first available encoder from that list.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
ui/vnc-enc-h264.c | 28 +++++++++++++++++++++-------
ui/vnc.c | 26 +++++++++++++++++++++-----
ui/vnc.h | 6 +++++-
3 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 3eabfc2cfe..33067d3a16 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -3,13 +3,21 @@
#include <gst/gst.h>
-const char *encoder_list[] = { "x264enc", "openh264enc", NULL };
-
-static const char *get_available_encoder(void)
+static char *get_available_encoder(const char *encoder_list)
{
+ g_assert(encoder_list != NULL);
+
+ if (!strcmp(encoder_list, "")) {
+ /* use default list */
+ encoder_list = "x264enc openh264enc";
+ }
+
+ char *ret = NULL;
+ char **encoder_array = g_strsplit(encoder_list, " ", -1);
+
int i = 0;
do {
- const char *encoder_name = encoder_list[i];
+ const char *encoder_name = encoder_array[i];
if (encoder_name == NULL) {
break;
}
@@ -17,12 +25,15 @@ static const char *get_available_encoder(void)
encoder_name, "video-encoder");
if (element != NULL) {
gst_object_unref(element);
- return encoder_name;
+ ret = strdup(encoder_name);
+ break;
}
i = i + 1;
} while (true);
- return NULL;
+ g_strfreev(encoder_array);
+
+ return ret;
}
static GstElement *create_encoder(const char *encoder_name)
@@ -215,8 +226,10 @@ static bool create_encoder_context(VncState *vs, int w, int h)
int vnc_h264_encoder_init(VncState *vs)
{
g_assert(vs->h264 == NULL);
+ g_assert(vs->vd != NULL);
+ g_assert(vs->vd->h264_encoder_list != NULL);
- const char *encoder_name = get_available_encoder();
+ char *encoder_name = get_available_encoder(vs->vd->h264_encoder_list);
if (encoder_name == NULL) {
VNC_DEBUG("No H264 encoder available.\n");
return -1;
@@ -316,6 +329,7 @@ void vnc_h264_clear(VncState *vs)
destroy_encoder_context(vs);
+ g_free(vs->h264->encoder_name);
g_free(vs->h264);
vs->h264 = NULL;
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 4ba0b715fd..5a7f93e762 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2190,11 +2190,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) == 0) {
- 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) == 0) {
+ vnc_set_feature(vs, VNC_FEATURE_H264);
+ vs->vnc_encoding = enc;
+ }
}
break;
#endif
@@ -3634,6 +3634,9 @@ static QemuOptsList qemu_vnc_opts = {
},{
.name = "power-control",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "h264",
+ .type = QEMU_OPT_STRING,
},
{ /* end of list */ }
},
@@ -4196,6 +4199,19 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
+#ifdef CONFIG_GSTREAMER
+ const char *h264_opt = qemu_opt_get(opts, "h264");
+ fprintf(stderr, "GOT %s\n", h264_opt);
+ if (!strcmp(h264_opt, "off")) {
+ vd->h264_encoder_list = NULL; /* disable h264 */
+ } else if (!strcmp(h264_opt, "on")) {
+ vd->h264_encoder_list = ""; /* use default encoder list */
+ } else {
+ /* assume this is a list of endiers */
+ vd->h264_encoder_list = h264_opt;
+ }
+#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 f39dbe21aa..e459441e35 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] 9+ messages in thread