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 | 30 ++++++++++++++++++++++--------
ui/vnc.c | 25 ++++++++++++++++++++-----
ui/vnc.h | 6 +++++-
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 047f4a3128..0f89cafbf6 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -27,13 +27,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;
}
@@ -41,12 +49,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)
@@ -220,11 +231,13 @@ 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;
@@ -336,6 +349,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 badc7912c0..feab4c0043 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)) {
- 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,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,18 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
+#ifdef CONFIG_GSTREAMER
+ const char *h264_opt = qemu_opt_get(opts, "h264");
+ 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 e97276349e..789b18806b 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.5On Fri, Apr 18, 2025 at 01:29:49PM +0200, Dietmar Maurer wrote:
> Values can be 'on', 'off', or a space sparated list of
> allowed gstreamer encoders.
space separated list values on the command line is incredibly unpleasant
syntax as it requires quoting the args.
>
> - on: automatically select the encoder
> - off: disbale h264
> - encoder-list: select first available encoder from that list.
Overloading one config option to both turn h264 on/off,
an choose encoding is not very desirable.
IMHO there should be a "h264=<bool>" option to turn it
on/off, and a separate flag to choose the encoder.
Do we even need to give a list of encoders, as opposed
to just choosing the desired encoder ?
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> ui/vnc-enc-h264.c | 30 ++++++++++++++++++++++--------
> ui/vnc.c | 25 ++++++++++++++++++++-----
> ui/vnc.h | 6 +++++-
> 3 files changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> index 047f4a3128..0f89cafbf6 100644
> --- a/ui/vnc-enc-h264.c
> +++ b/ui/vnc-enc-h264.c
> @@ -27,13 +27,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;
> }
> @@ -41,12 +49,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)
> @@ -220,11 +231,13 @@ 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;
> @@ -336,6 +349,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 badc7912c0..feab4c0043 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)) {
> - 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,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,18 @@ void vnc_display_open(const char *id, Error **errp)
> }
> #endif
>
> +#ifdef CONFIG_GSTREAMER
> + const char *h264_opt = qemu_opt_get(opts, "h264");
> + 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 e97276349e..789b18806b 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
>
>
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 :|
> Overloading one config option to both turn h264 on/off, > an choose encoding is not very desirable. > > IMHO there should be a "h264=<bool>" option to turn it > on/off, and a separate flag to choose the encoder. will do that. > Do we even need to give a list of encoders, as opposed > to just choosing the desired encoder ? How do you know if the encoder is available? If we use a list, we can simply loop through and use the first available. This way we can also test for (and prefer) hardware encoder... - Dietmar
Hi
On Fri, Apr 18, 2025 at 3:30 PM Dietmar Maurer <dietmar@proxmox.com> wrote:
>
> Values can be 'on', 'off', or a space sparated list of
separated
> allowed gstreamer encoders.
>
> - on: automatically select the encoder
'auto' then (default?)
> - off: disbale h264
disable
'auto'/'off' but there should be 'on' too.
> - encoder-list: select first available encoder from that list.
This should be a different option.
>
> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
> ---
> ui/vnc-enc-h264.c | 30 ++++++++++++++++++++++--------
> ui/vnc.c | 25 ++++++++++++++++++++-----
> ui/vnc.h | 6 +++++-
> 3 files changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> index 047f4a3128..0f89cafbf6 100644
> --- a/ui/vnc-enc-h264.c
> +++ b/ui/vnc-enc-h264.c
> @@ -27,13 +27,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;
> }
> @@ -41,12 +49,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)
> @@ -220,11 +231,13 @@ 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;
> @@ -336,6 +349,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 badc7912c0..feab4c0043 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)) {
> - 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,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,18 @@ void vnc_display_open(const char *id, Error **errp)
> }
> #endif
>
> +#ifdef CONFIG_GSTREAMER
> + const char *h264_opt = qemu_opt_get(opts, "h264");
> + 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 e97276349e..789b18806b 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
>
© 2016 - 2025 Red Hat, Inc.