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 | 89 +++++++++++++++++++++++++++++++++++++++--------
ui/vnc.h | 1 +
2 files changed, 75 insertions(+), 15 deletions(-)
diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
index 3abe6a1528..047f4a3128 100644
--- a/ui/vnc-enc-h264.c
+++ b/ui/vnc-enc-h264.c
@@ -27,6 +27,68 @@
#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, /* 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);
@@ -68,26 +130,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");
@@ -172,9 +220,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 a5ea134de8..e97276349e 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.5On Fri, Apr 18, 2025 at 01:29:48PM +0200, Dietmar Maurer wrote:
> 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 | 89 +++++++++++++++++++++++++++++++++++++++--------
> ui/vnc.h | 1 +
> 2 files changed, 75 insertions(+), 15 deletions(-)
>
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> index 3abe6a1528..047f4a3128 100644
> --- a/ui/vnc-enc-h264.c
> +++ b/ui/vnc-enc-h264.c
> @@ -172,9 +220,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;
This method has return type 'bool', not 'int', this needs
to be 'false'.
> + }
> +
> vs->h264 = g_new0(VncH264, 1);
> + vs->h264->encoder_name = encoder_name;
> +
> + VNC_DEBUG("Allow H264 using encoder '%s`\n", encoder_name);
>
> return true;
> }
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 :|
On Fri, Apr 18, 2025 at 01:29:48PM +0200, Dietmar Maurer wrote:
> 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 | 89 +++++++++++++++++++++++++++++++++++++++--------
> ui/vnc.h | 1 +
> 2 files changed, 75 insertions(+), 15 deletions(-)
>
> diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c
> index 3abe6a1528..047f4a3128 100644
> --- a/ui/vnc-enc-h264.c
> +++ b/ui/vnc-enc-h264.c
> @@ -27,6 +27,68 @@
>
> #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);
This while loop is incredibly verbose as written.
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];
}
}
and get rid of the trailing "NULL" in encoder_list as it
isn't required
> +
> + return NULL;
> +}
> +
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 :|
© 2016 - 2025 Red Hat, Inc.