Il giorno lun 2 ott 2023 alle ore 06:41 Vivek Kasireddy
<vivek.kasireddy@xxxxxxxxx> ha scritto:
>
> If we use the x264enc encoder to encode a stream, then videoconvert
> would convert the BGRx data into Y444, which is the preferred format
> for x264enc. However, some decoders particularly the ones that are
> h/w based cannot work with Y444 if it was the format used by the
> encoder. Therefore, to address these situations, we need a way to
> override the format used during the encoding stage which can be
> accomplished by using the environment variable introduced in this
> patch: SPICE_CONVERTER_PREFERRED_FORMAT.
>
> For example, using NV12 as the output format for the videoconvert
> element would allow us to pair a s/w based encoder (such as x264enc)
> with a h/w based decoder (such as msdkh264dec) for decoding the
> stream as most h/w based decoders only work with NV12 format given
> its popularity.
>
> Note that choosing an encoder format such as NV12 over Y444 would
> probably result in decreased video quality although it would be
> compatible with more decoders. Ideally, the client and server need
> to negotiate a suitable format dynamically but the current
> capabilities do not allow for such exchange.
>
> v2:
> - Add the environment variable to override encoding format
> - Augment the commit message to explain the impact of overriding
> the default encoding format (Frediano)
>
> Cc: Frediano Ziglio <freddy77@xxxxxxxxx>
> Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> Based-on-patch-by: Hazwan Arif Mazlan <hazwan.arif.mazlan@xxxxxxxxx>
> Signed-off-by: Jin Chung Teng <jin.chung.teng@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> ---
> server/gstreamer-encoder.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index d8af91f1..1619672a 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -913,13 +913,25 @@ static const gchar* get_gst_codec_name(const SpiceGstEncoder *encoder)
> }
> }
>
> +static gchar *get_gst_converter(void)
> +{
> + gchar *converter, *pref_format;
> +#ifdef HAVE_GSTREAMER_0_10
> + converter = g_strdup("ffmpegcolorspace");
> +#else
> + pref_format = getenv("SPICE_CONVERTER_PREFERRED_FORMAT");
> + if (pref_format) {
> + converter = g_strconcat("videoconvert ! video/x-raw,format=", pref_format, NULL);
> + } else {
> + converter = g_strdup("videoconvert");
> + }
> +#endif
> + return converter;
> +}
> +
> static gboolean create_pipeline(SpiceGstEncoder *encoder)
> {
> -#ifdef HAVE_GSTREAMER_0_10
> - const gchar *converter = "ffmpegcolorspace";
> -#else
> - const gchar *converter = "videoconvert";
> -#endif
> + gchar* converter = get_gst_converter();
> const gchar* gstenc_name = get_gst_codec_name(encoder);
> if (!gstenc_name) {
> return FALSE;
> @@ -979,6 +991,7 @@ static gboolean create_pipeline(SpiceGstEncoder *encoder)
> converter, gstenc_name, gstenc_opts);
> 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_opts);
> g_free(desc);
> if (!encoder->pipeline || err) {
Hi,
it sounds almost fine. I had to rebase on master, fixing some
conflicts (GStreamer 0.10 was removed mainwhile), and removed minor
leaks.
See https://gitlab.freedesktop.org/fziglio/spice/-/commits/gstreamer_hw/
branch, specifically
https://gitlab.freedesktop.org/fziglio/spice/-/commit/4b7fbdd6b00b0a7941655aefc5eb3ba46b8e3278
(updated commit) and
https://gitlab.freedesktop.org/fziglio/spice/-/commit/4d27bf0d38b47575a757f25a1616c1f42b4c957f
(minor fixup to fix CI).
Frediano