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