Re: [PATCH 2/2] gstreamer-encoder: Use a h/w based encoder with Intel GPUs if possible (v3)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il giorno lun 2 ott 2023 alle ore 06:41 Vivek Kasireddy
<vivek.kasireddy@xxxxxxxxx> ha scritto:
>
> Once it is determined that an Intel GPU is available/active (after
> looking into udev's database), we try to see if there is a h/w
> based encoder (element) available (in Gstreamer's registry cache)
> for the user selected video codec. In other words, if we find that
> the Intel Media SDK Gstreamer plugin (libgstmsdk.so) and associated
> libraries (such as va or vaapi) are all installed properly, we add
> the appropriate h/w based encoder and post-processor/converter
> elements to the pipeline (along with any relevant options) instead
> of the s/w based elements.
>
> For example, if the user selects h264 as the preferred codec format,
> msdkh264enc and vapostproc will be preferred instead of x264enc
> and videoconvert.
>
> v2: (addressed some review comments from Frediano)
> - Moved the udev helper into spice-common
> - Refactored the code to choose plugins in order msdk > va > vaapi
>
> v3: (Frediano)
> - Added relevant encoder options for mjpeg and vp9 codecs (Jin Chung)
>
> Cc: Frediano Ziglio <freddy77@xxxxxxxxx>
> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Cc: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> Co-developed-by: Jin Chung Teng <jin.chung.teng@xxxxxxxxx>
> Co-developed-by: Hazwan Arif Mazlan <hazwan.arif.mazlan@xxxxxxxxx>
> ---
>  server/gstreamer-encoder.c | 120 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 1619672a..952c2e87 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -31,6 +31,7 @@
>  #include "red-common.h"
>  #include "video-encoder.h"
>  #include "utils.h"
> +#include "common/udev.h"
>
>
>  #define SPICE_GST_DEFAULT_FPS 30
> @@ -913,6 +914,115 @@ static const gchar* get_gst_codec_name(const SpiceGstEncoder *encoder)
>      }
>  }
>
> +static const char video_codecs[][8] = {
> +    { "" },
> +    { "mjpeg" },
> +    { "vp8" },
> +    { "h264" },
> +    { "vp9" },
> +    { "h265" },
> +};
> +
> +static bool gst_features_lookup(const gchar *feature_name)
> +{
> +    GstRegistry *registry;
> +    GstPluginFeature *feature;
> +
> +    registry = gst_registry_get();
> +    if (!registry) {
> +        return false;
> +    }
> +
> +    feature = gst_registry_lookup_feature(registry, feature_name);
> +    if (!feature) {
> +        return false;
> +    }
> +
> +    gst_object_unref(feature);
> +    return true;
> +}
> +
> +static gchar *find_best_plugin(const gchar *codec_name)
> +{
> +    const char *plugins[3] = {"msdk", "va", "vaapi"};
> +    gchar *feature_name;
> +    int i;
> +
> +    for (i = 0; i < 3; i++) {
> +        feature_name = !codec_name ? g_strconcat(plugins[i], "postproc", NULL) :
> +                       g_strconcat(plugins[i], codec_name, "enc", NULL);
> +        if (!gst_features_lookup(feature_name)) {
> +            g_free(feature_name);
> +            feature_name = NULL;
> +            continue;
> +        }
> +        break;
> +    }
> +    return feature_name;
> +}
> +
> +static gchar *get_gstenc_opts(gchar *encoder, const gchar *codec_name)
> +{
> +    gchar *gstenc_opts;
> +
> +    if (strcmp(codec_name, "mjpeg") == 0) {
> +        return g_strdup("");
> +    }
> +
> +    if (strstr(encoder, "msdk")) {
> +        if (strcmp(codec_name, "vp9") == 0) {
> +            gstenc_opts = g_strdup("async-depth=1 b-frames=0 rate-control=3 target-usage=7");
> +        } else {
> +            gstenc_opts = g_strdup("async-depth=1 rate-control=3 gop-size=1 tune=16 b-frames=0 target-usage=7 min-qp=15 max-qp=35");
> +        }
> +    } else if (strstr(encoder, "vaapi")) {
> +        if (strcmp(codec_name, "vp9") == 0) {
> +            gstenc_opts = g_strdup("tune=3 rate-control=1");
> +        } else {
> +            gstenc_opts = g_strdup("rate-control=cqp max-bframes=0 min-qp=15 max-qp=35");
> +        }
> +    } else {
> +        if (strcmp(codec_name, "vp9") == 0) {
> +            gstenc_opts = g_strdup("min-qp=15 max-qp=35 rate-control=16 ref-frames=0 target-usage=7");
> +        } else {
> +            gstenc_opts = g_strdup("rate-control=16 b-frames=0 target-usage=7 min-qp=15 max-qp=35");
> +        }
> +    }
> +    return gstenc_opts;
> +}
> +
> +static void try_intel_hw_plugins(const gchar *codec_name, gchar **converter,
> +                                 gchar **gstenc_name, gchar **gstenc_opts)
> +{
> +    gchar *encoder, *vpp;
> +
> +    if (strcmp(codec_name, "vp8") == 0) {
> +        return;
> +    }
> +
> +    encoder = find_best_plugin(codec_name);
> +    if (!encoder) {
> +        return;
> +    }
> +    vpp = find_best_plugin(NULL);
> +    if (!vpp) {
> +        return;
> +    }
> +
> +    g_free(*converter);
> +    g_free(*gstenc_name);
> +    g_free(*gstenc_opts);
> +    *gstenc_name = encoder;
> +    *gstenc_opts = get_gstenc_opts(encoder, codec_name);
> +
> +    if (strstr(vpp, "vaapi")) {
> +        *converter = g_strconcat(vpp, " ! video/x-raw(memory:VASurface),format=NV12", NULL);
> +    } else {
> +        *converter = g_strconcat(vpp, " ! video/x-raw(memory:VAMemory),format=NV12", NULL);
> +    }
> +    g_free(vpp);
> +}
> +
>  static gchar *get_gst_converter(void)
>  {
>      gchar *converter, *pref_format;
> @@ -932,7 +1042,7 @@ static gchar *get_gst_converter(void)
>  static gboolean create_pipeline(SpiceGstEncoder *encoder)
>  {
>      gchar* converter = get_gst_converter();
> -    const gchar* gstenc_name = get_gst_codec_name(encoder);
> +    gchar* gstenc_name = g_strdup(get_gst_codec_name(encoder));
>      if (!gstenc_name) {
>          return FALSE;
>      }
> @@ -985,6 +1095,13 @@ static gboolean create_pipeline(SpiceGstEncoder *encoder)
>          return FALSE;
>      }
>
> +    const char *codec_name = video_codecs[encoder->base.codec_type];
> +    GpuVendor vendor = spice_udev_detect_gpu(INTEL_VENDOR_ID);
> +    if (vendor == VENDOR_GPU_DETECTED) {
> +        try_intel_hw_plugins(codec_name, &converter, &gstenc_name,
> +                             &gstenc_opts);
> +    }
> +
>      GError *err = NULL;
>      gchar *desc = g_strdup_printf("appsrc is-live=true format=time do-timestamp=true name=src !"
>                                    " %s ! %s name=encoder %s ! appsink name=sink",
> @@ -992,6 +1109,7 @@ static gboolean create_pipeline(SpiceGstEncoder *encoder)
>      spice_debug("GStreamer pipeline: %s", desc);
>      encoder->pipeline = gst_parse_launch_full(desc, NULL, GST_PARSE_FLAG_FATAL_ERRORS, &err);
>      g_free(converter);
> +    g_free(gstenc_name);
>      g_free(gstenc_opts);
>      g_free(desc);
>      if (!encoder->pipeline || err) {


Hi,
   rebased on master, added some fixup, see
https://gitlab.freedesktop.org/fziglio/spice/-/commits/gstreamer_hw/.

Specifically
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/54eba373a0d2c34b65fc5f2551fc406a6058962b
to fix CI;
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/2a55b6dd52eb138839160e7e78c977a3c978fd35
update submodule (well, we need to changed when final spice-common
patches will be merged);
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/8cabd4de61244ef28f47eee4105cd55924065727
removes minor leak;
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/a1c9d3adde2710b43bebcfbe43bb23608d8e037e
minor style/optimization changes, not really important;
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/a7876c878234e0ca14cf0a94cf25b9b34b7c39ae
fixes a leak and a use-after-free;
- https://gitlab.freedesktop.org/fziglio/spice/-/commit/7640409b0d5ec87c51441e46df7fa61281669df5
some "styles" changes. I added a "_hw_" in some function names to make
clear they are for hardware codecs. Using g_str_has_prefix instead of
strstr, it seems more clear to me the intention. Added a const to a
pointer, we are not changing the string.

get_gstenc_opts seems too dependent on the output and flow of other
functions making the implementation a bit confusing but I don't have
any practical suggestions to make it more clear... so in the end I'm
fine with it.

Regards,
   Frediano




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]