[PATCH v5] gstreamer-encoder: Use an env var to override converter format (v5)

[Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>]

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)

v3: (Frediano)
- Free converter when pipeline creation fails due to invalid codec
- Rebase on master

v4: (Frediano)
- Ensure that the preferred format obtained via the environment var
  SPICE_CONVERTER_PREFERRED_FORMAT is valid
- Use the g_once mechanism to cache and return the preferred format
  after validating it

v5: (Frediano)
- Prevent the double free by doing g_strdup(gst_once.retval) in
  get_gst_converter().

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 | 41 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index d08de35a..40882f69 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -861,13 +861,50 @@ static const gchar* get_gst_codec_name(const SpiceGstEncoder *encoder)
     }
 }
 
+/* At this time, only the following formats are supported by x264enc. */
+static const char valid_formats[][10] = {
+    { "Y444" },
+    { "Y42B" },
+    { "I420" },
+    { "YV12" },
+    { "NV12" },
+    { "GRAY8" },
+    { "Y444_10LE" },
+    { "I422_10LE" },
+    { "I420_10LE" },
+};
+
+static gpointer get_pref_format_once(gpointer data)
+{
+    const gchar *pref_format = getenv("SPICE_CONVERTER_PREFERRED_FORMAT");
+    int i;
+
+    if (pref_format) {
+        for (i = 0; i < G_N_ELEMENTS(valid_formats); i++) {
+            if (strcmp(valid_formats[i], pref_format) == 0) {
+                return g_strdup_printf("videoconvert ! video/x-raw,format=%s",
+                                       pref_format);
+            }
+        }
+    }
+    return g_strdup("videoconvert");
+}
+
+static gchar *get_gst_converter(void)
+{
+    static GOnce gst_once = G_ONCE_INIT;
+
+    g_once(&gst_once, get_pref_format_once, NULL);
+    return g_strdup(gst_once.retval);
+}
+
 static gboolean create_pipeline(SpiceGstEncoder *encoder)
 {
-    const gchar *converter = "videoconvert";
     const gchar* gstenc_name = get_gst_codec_name(encoder);
     if (!gstenc_name) {
         return FALSE;
     }
+    gchar* converter = get_gst_converter();
     gchar* gstenc_opts;
     switch (encoder->base.codec_type)
     {
@@ -910,6 +947,7 @@ static gboolean create_pipeline(SpiceGstEncoder *encoder)
     default:
         /* gstreamer_encoder_new() should have rejected this codec type */
         spice_warning("unsupported codec type %d", encoder->base.codec_type);
+        g_free(converter);
         return FALSE;
     }
 
@@ -919,6 +957,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) {
-- 
2.39.2