gstreamer1.0-plugins-base: set default videosink for IMXQM/IMXQXP

From: Ming Liu <liu.ming50@gmail.com>

The default 'autovideosink' gives very bad performance when playing
H264/H265 videos on IMXQM/IMXQXP machines. Let's choose
'imxvideoconvert_g2d ! autovideosink' as the default videosink, so the
end users dont need manually pass 'imxvideoconvert_g2d' pipeline to
gst-play-1.0.

Signed-off-by: Ming Liu <liu.ming50@gmail.com>
---
 ...e-default-videosink-autosink-options.patch | 106 ++++++++++++++++++
 .../gstreamer1.0-plugins-base_1.20.3.imx.bb   |  14 ++-
 2 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100644 recipes-multimedia/gstreamer/gstreamer1.0-plugins-base/0001-Introduce-default-videosink-autosink-options.patch

Hello Ming,

On Tue, Mar 14, 2023 at 9:56 AM Ming Liu <liu.ming50@gmail.com> wrote:
>
> From: Ming Liu <liu.ming50@gmail.com>
>
> The default 'autovideosink' gives very bad performance when playing
> H264/H265 videos on IMXQM/IMXQXP machines. Let's choose
> 'imxvideoconvert_g2d ! autovideosink' as the default videosink, so the
> end users dont need manually pass 'imxvideoconvert_g2d' pipeline to
> gst-play-1.0.
>
> Signed-off-by: Ming Liu <liu.ming50@gmail.com>
> ---
>  ...e-default-videosink-autosink-options.patch | 106 ++++++++++++++++++
>  .../gstreamer1.0-plugins-base_1.20.3.imx.bb   |  14 ++-
>  2 files changed, 118 insertions(+), 2 deletions(-)
>  create mode 100644 recipes-multimedia/gstreamer/gstreamer1.0-plugins-base/0001-Introduce-default-videosink-autosink-options.patch
>
> diff --git a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base/0001-Introduce-default-videosink-autosink-options.patch b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base/0001-Introduce-default-videosink-autosink-options.patch
> new file mode 100644
> index 00000000..9ad0385d
> --- /dev/null
> +++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base/0001-Introduce-default-videosink-autosink-options.patch
> @@ -0,0 +1,106 @@
> +From c929a3fd4a10a2d5f57933b40447aa76ce0b6c5a Mon Sep 17 00:00:00 2001
> +From: Ming Liu <liu.ming50@gmail.com>
> +Date: Sun, 5 Mar 2023 13:52:05 +0100
> +Subject: [PATCH] Introduce default videosink/autosink options
> +
> +So the end users can choose the default videosink/autosink at build
> +time. It now supports pipeline as default videosink/autosink.
> +
> +Upstream-Status: Pending [https://github.com/nxp-imx/gst-plugins-base/pull/2]

I'm sorry, but this not the upstream URL this patch refers to.
GStreamer upstream development happens here [1], and what the patch is
pointing to is (as the description suggests) NXP fork of the upstream
GStreamer repository.

I do not think that the 'Upstream-Status:' tag can be used here like
this, as it shall be rather:
'Upstream-Status: Inappropriate [nxp-imx specific]'

> +
> +Signed-off-by: Ming Liu <liu.ming50@gmail.com>
> +---
> + gst/playback/gstplaysink.c | 10 ++++++++--
> + meson.build                |  5 ++---
> + meson_options.txt          |  5 +++++
> + tools/gst-play.c           |  7 +++++++
> + 4 files changed, 22 insertions(+), 5 deletions(-)
> +
> +diff --git a/gst/playback/gstplaysink.c b/gst/playback/gstplaysink.c
> +index dc8bfdacd..1abb4ac7e 100755
> +--- a/gst/playback/gstplaysink.c
> ++++ b/gst/playback/gstplaysink.c
> +@@ -1770,7 +1770,10 @@ gen_video_chain (GstPlaySink * playsink, gboolean raw, gboolean async)
> +       /* if default sink from config.h is different then try it too */
> +       if (strcmp (DEFAULT_VIDEOSINK, "autovideosink")) {
> +         GST_DEBUG_OBJECT (playsink, "trying " DEFAULT_VIDEOSINK);
> +-        elem = gst_element_factory_make (DEFAULT_VIDEOSINK, "videosink");
> ++        if (strchr (DEFAULT_VIDEOSINK, ' ') != NULL)
> ++            elem = gst_parse_bin_from_description (DEFAULT_VIDEOSINK, TRUE, NULL);
> ++        else
> ++            elem = gst_element_factory_make (DEFAULT_VIDEOSINK, "videosink");
> +         chain->sink = try_element (playsink, elem, TRUE);
> +       }
> +     }
> +@@ -2713,7 +2716,10 @@ gen_audio_chain (GstPlaySink * playsink, gboolean raw)
> +       /* if default sink from config.h is different then try it too */
> +       if (strcmp (DEFAULT_AUDIOSINK, "autoaudiosink")) {
> +         GST_DEBUG_OBJECT (playsink, "trying " DEFAULT_AUDIOSINK);
> +-        elem = gst_element_factory_make (DEFAULT_AUDIOSINK, "audiosink");
> ++        if (strchr (DEFAULT_AUDIOSINK, ' ') != NULL)
> ++            elem = gst_parse_bin_from_description (DEFAULT_AUDIOSINK, TRUE, NULL);
> ++        else
> ++            elem = gst_element_factory_make (DEFAULT_AUDIOSINK, "audiosink");
> +         chain->sink = try_element (playsink, elem, TRUE);
> +       }
> +     }
> +diff --git a/meson.build b/meson.build
> +index 5ad1ab3ad..a6280eccd 100644
> +--- a/meson.build
> ++++ b/meson.build
> +@@ -280,9 +280,8 @@ endif
> + core_conf.set_quoted('GST_PACKAGE_NAME', gst_package_name)
> + core_conf.set_quoted('GST_PACKAGE_ORIGIN', get_option('package-origin'))
> +
> +-# FIXME: These should be configure options
> +-core_conf.set_quoted('DEFAULT_VIDEOSINK', 'autovideosink')
> +-core_conf.set_quoted('DEFAULT_AUDIOSINK', 'autoaudiosink')
> ++core_conf.set_quoted('DEFAULT_VIDEOSINK', get_option('defaultvideosink'))
> ++core_conf.set_quoted('DEFAULT_AUDIOSINK', get_option('defaultaudiosink'))
> +
> + # Set whether the audioresampling method should be detected at runtime
> + core_conf.set('AUDIORESAMPLE_FORMAT_' + get_option('audioresample_format').to_upper(), true)
> +diff --git a/meson_options.txt b/meson_options.txt
> +index 7010b9162..49bd1a728 100644
> +--- a/meson_options.txt
> ++++ b/meson_options.txt
> +@@ -21,6 +21,11 @@ option('opengl_module_name', type : 'string', value : '',
> + option('gles2_module_name', type : 'string', value : '',
> +        description : 'The file to pass to g_module_open to open the libGLESv2 library (default: libGLESv2)')
> +
> ++option('defaultaudiosink', type : 'string', value : 'autoaudiosink',
> ++       description : 'The default audiosink')
> ++option('defaultvideosink', type : 'string', value : 'autovideosink',
> ++       description : 'The default videosink')
> ++
> + # Feature option for opengl plugin and integration library
> + option('gl', type : 'feature', value : 'auto', description : 'OpenGL integration library and OpenGL plugin')
> + option('gl-graphene', type : 'feature', value : 'auto', description : 'Use Graphene in OpenGL plugin')
> +diff --git a/tools/gst-play.c b/tools/gst-play.c
> +index 0e858d282..4c1e92e42 100644
> +--- a/tools/gst-play.c
> ++++ b/tools/gst-play.c
> +@@ -205,6 +205,7 @@ play_new (gchar ** uris, const gchar * audio_sink, const gchar * video_sink,
> +     else
> +       g_warning ("Couldn't create specified audio sink '%s'", audio_sink);
> +   }
> ++
> +   if (video_sink != NULL) {
> +     if (strchr (video_sink, ' ') != NULL)
> +       sink = gst_parse_bin_from_description (video_sink, TRUE, NULL);
> +@@ -1674,6 +1675,12 @@ main (int argc, char **argv)
> +     playlist_file = NULL;
> +   }
> +
> ++  if (audio_sink == NULL)
> ++    audio_sink = g_strdup(DEFAULT_AUDIOSINK);
> ++
> ++  if (video_sink == NULL)
> ++    video_sink = g_strdup(DEFAULT_VIDEOSINK);
> ++
> +   if (playlist->len == 0 && (filenames == NULL || *filenames == NULL)) {
> +     gst_printerr (_("Usage: %s FILE1|URI1 [FILE2|URI2] [FILE3|URI3] ..."),
> +         "gst-play-" GST_API_VERSION);
> +--
> +2.25.1
> +
> diff --git a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.20.3.imx.bb b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.20.3.imx.bb
> index 0ef9daea..b3da7438 100644
> --- a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.20.3.imx.bb
> +++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.20.3.imx.bb
> @@ -121,7 +121,9 @@ SRC_URI:remove = " \
>      file://0003-viv-fb-Make-sure-config.h-is-included.patch \
>      file://0002-ssaparse-enhance-SSA-text-lines-parsing.patch"
>  SRC_URI:prepend = "${GST1.0-PLUGINS-BASE_SRC};branch=${SRCBRANCH} "
> -SRC_URI += "file://0001-gstallocatorphymem.c-Typecast-result-of-gst_phymem_g.patch"
> +SRC_URI:append = " \
> +    file://0001-gstallocatorphymem.c-Typecast-result-of-gst_phymem_g.patch \
> +    file://0001-Introduce-default-videosink-autosink-options.patch"
>  GST1.0-PLUGINS-BASE_SRC ?= "gitsm://github.com/nxp-imx/gst-plugins-base.git;protocol=https"
>  SRCBRANCH = "MM_04.07.02_2210_L5.15.y"
>  SRCREV = "cbf542ce3e0bad1009d5ecf72707e870c375c3f0"
> @@ -130,6 +132,9 @@ S = "${WORKDIR}/git"
>
>  inherit use-imx-headers
>
> +DEFAULT_AUDIOSINK ?= "autoaudiosink"
> +DEFAULT_VIDEOSINK ?= "autovideosink"
> +
>  PACKAGECONFIG_GL:imxgpu2d = \
>      "${@bb.utils.contains('DISTRO_FEATURES', 'opengl x11', 'opengl viv-fb', '', d)}"
>  PACKAGECONFIG_GL:imxgpu3d = \
> @@ -144,7 +149,12 @@ PACKAGECONFIG:append:imxgpu2d = " g2d"
>  PACKAGECONFIG[g2d] = ",,virtual/libg2d"
>  PACKAGECONFIG[viv-fb] = ",,virtual/libgles2"
>
> -EXTRA_OEMESON += "-Dc_args="${CFLAGS} -I${STAGING_INCDIR_IMX}""
> +DEFAULT_AUDIOSINK ?= "autoaudiosink"
> +DEFAULT_VIDEOSINK ?= "autovideosink"
> +DEFAULT_VIDEOSINK:mx8qm-nxp-bsp = "imxvideoconvert_g2d ! autovideosink"
> +DEFAULT_VIDEOSINK:mx8qxp-nxp-bsp = "imxvideoconvert_g2d ! autovideosink"
> +
> +EXTRA_OEMESON += "-Ddefaultaudiosink="${DEFAULT_AUDIOSINK}" -Ddefaultvideosink="${DEFAULT_VIDEOSINK}" -Dc_args="${CFLAGS} -I${STAGING_INCDIR_IMX}""

This all can be solved with PACKAGECONFIG, why it is needed to extend
the Meson options?

>
>  # links with imx-gpu libs which are pre-built for glibc
>  # gcompat will address it during runtime
> --
> 2.25.1
>

Link: [1]: https://gitlab.freedesktop.org/gstreamer/gstreamer


--
Regards,
Andrey.