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

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

 



Il giorno gio 27 apr 2023 alle ore 07:37 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 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 vaapipostproc will be added instead of x264enc
> and videoconvert.
>
> 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>
> Signed-off-by: Mazlan, Hazwan Arif <hazwan.arif.mazlan@xxxxxxxxx>
> Signed-off-by: Teng, Jin Chung <jin.chung.teng@xxxxxxxxx>
> ---
>  meson.build                |  3 ++
>  server/gstreamer-encoder.c | 93 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 94 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index d66fac10..a401fe2c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -182,6 +182,9 @@ if smartcard_dep.found()
>    spice_server_requires += 'libcacard >= 2.5.1 '
>  endif
>
> +#udev
> +spice_server_deps += dependency('libudev', required : true)
> +
>  #
>  # global C defines
>  #
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 2ceb80ba..eed359df 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -27,6 +27,7 @@
>  #include <gst/app/gstappsink.h>
>  #include <gst/video/video.h>
>  #include <orc/orcprogram.h>
> +#include <libudev.h>
>
>  #include "red-common.h"
>  #include "video-encoder.h"
> @@ -39,6 +40,7 @@
>  # define DO_ZERO_COPY
>  #endif
>
> +#define INTEL_GFX_DRV_NAME "i915"
>
>  typedef struct {
>      SpiceBitmapFmt spice_format;
> @@ -913,14 +915,97 @@ static const gchar* get_gst_codec_name(const SpiceGstEncoder *encoder)
>      }
>  }
>
> +static gboolean detect_intel_gpu()

This function is mostly the same as the other patch, why not put
common stuff in spice-common (used by both spice-gtk and spice-server)
?

> +{
> +    struct udev *udev;
> +    struct udev_device *udev_dev;
> +    struct udev_enumerate *udev_enum;
> +    struct udev_list_entry *entry, *devices;
> +    const char *path, *driver;
> +    gboolean found = FALSE;
> +
> +    udev = udev_new();
> +    if (!udev) {
> +        return FALSE;
> +    }
> +
> +    udev_enum = udev_enumerate_new(udev);
> +    if (udev_enum) {
> +        udev_enumerate_add_match_subsystem(udev_enum, "pci");
> +        udev_enumerate_scan_devices(udev_enum);
> +        devices = udev_enumerate_get_list_entry(udev_enum);
> +
> +        udev_list_entry_foreach(entry, devices) {
> +            path = udev_list_entry_get_name(entry);
> +            udev_dev = udev_device_new_from_syspath(udev, path);
> +
> +            driver = udev_device_get_driver(udev_dev);
> +            if (!g_strcmp0(driver, INTEL_GFX_DRV_NAME)) {
> +                found = TRUE;
> +                udev_device_unref(udev_dev);
> +                break;
> +            }
> +            udev_device_unref(udev_dev);
> +        }
> +        udev_enumerate_unref(udev_enum);
> +    }
> +    udev_unref(udev);
> +
> +    return found;
> +}
> +
> +static gboolean msdk_vaapi_features_lookup(const gchar *enc_name)
> +{
> +    GstRegistry *registry = NULL;
> +    GstPluginFeature *msdkenc = NULL;
> +    GstPluginFeature *vaapivpp = NULL;
> +
> +    registry = gst_registry_get();
> +    if (!registry) {
> +        return FALSE;
> +    }
> +    msdkenc = gst_registry_lookup_feature(registry, enc_name);
> +    if (!msdkenc) {
> +        return FALSE;
> +    }
> +    vaapivpp = gst_registry_lookup_feature(registry, "vaapipostproc");
> +    if (!vaapivpp) {
> +        gst_object_unref(msdkenc);
> +        return FALSE;
> +    }
> +
> +    gst_object_unref(msdkenc);
> +    gst_object_unref(vaapivpp);
> +    return TRUE;
> +}
> +
> +static const struct {
> +    const gchar name[8];
> +} video_codecs[] = {

why not a simpler "static const char video_codec_names[][8]" ? Are you
planning to extend the structure?

> +    { "" },
> +    { "mjpeg" },
> +    { "vp8" },
> +    { "h264" },
> +    { "vp9" },
> +    { "h265" },
> +};
> +
>  static gboolean create_pipeline(SpiceGstEncoder *encoder)
>  {
> +    const gchar *codec_name = video_codecs[encoder->base.codec_type].name;
> +    gchar *msdk_enc_name = g_strconcat("msdk", codec_name, "enc", NULL);

similar comment of other patch, also we could share some code here too.

> +    gboolean use_msdk = detect_intel_gpu() &&
> +                        msdk_vaapi_features_lookup(msdk_enc_name);
>  #ifdef HAVE_GSTREAMER_0_10
>      const gchar *converter = "ffmpegcolorspace";
>  #else
> -    const gchar *converter = "videoconvert";
> +    const gchar *converter = use_msdk ?
> +                             "vaapipostproc ! video/x-raw(memory:DMABuf)" :
> +                             "videoconvert";
>  #endif
> -    const gchar* gstenc_name = get_gst_codec_name(encoder);
> +    const gchar* gstenc_name = use_msdk ? g_strdup(msdk_enc_name) :
> +                               get_gst_codec_name(encoder);

Either we are getting a leak or an invalid free() I suppose.
Both names are used for the encoder name so now maybe the
get_gst_codec_name function name is misleading now.

> +    g_free(msdk_enc_name);
>      if (!gstenc_name) {
>          return FALSE;
>      }
> @@ -972,6 +1057,10 @@ static gboolean create_pipeline(SpiceGstEncoder *encoder)
>          spice_warning("unsupported codec type %d", encoder->base.codec_type);
>          return FALSE;
>      }
> +    if (use_msdk) {
> +        g_free(gstenc_opts);
> +        gstenc_opts = g_strdup("async-depth=1 rate-control=3 gop-size=1 tune=16 ref-frames=1 b-frames=0 target-usage=7 min-qp=15 max-qp=35");
> +    }
>
>      GError *err = NULL;
>      gchar *desc = g_strdup_printf("appsrc is-live=true format=time do-timestamp=true name=src !"

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]