[Buildroot] [PATCH] package/mesa3d: Allow building patent encumbered video codecs

[Buildroot] [PATCH] package/mesa3d: Allow building patent encumbered video codecs

[Tristan van Berkom via buildroot]

This patch adds some options to allow building the patent encumbered
codecs in mesa, such as H.264/H.265 encoders/decoders which are useful
for hardware accelerated decoding via libva/gstreamer.

These codecs are now disabled by default in upstream mesa as per:
  https://gitlab.freedesktop.org/mesa/mesa/-/commit/7d969fe9e91e39e03041cdfac69bf33337bc2c96

Signed-off-by: Tristan van Berkom <tristan.vanberkom@codethink.co.uk>
---
 package/mesa3d/Config.in | 36 ++++++++++++++++++++++++++++++++++++
 package/mesa3d/mesa3d.mk | 11 +++++++++++
 2 files changed, 47 insertions(+)

diff --git a/package/mesa3d/Config.in b/package/mesa3d/Config.in
index c7ee2a8db0..7f1d74fe9a 100644
--- a/package/mesa3d/Config.in
+++ b/package/mesa3d/Config.in
@@ -56,6 +56,42 @@ config BR2_PACKAGE_MESA3D_OPENCL
        select BR2_PACKAGE_LIBCLC
        select BR2_PACKAGE_HAS_LIBOPENCL
 
+# inform the .mk file of video codec selection
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       bool
+
+comment "Patent encumbered video codecs"
+
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC_VC1DEC
+       bool "VC-1 decoder"
+       select BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       help
+         Decoder module for VC-1 video coding
+
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264DEC
+       bool "H.264 decoder"
+       select BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       help
+         Decoder module for H.264 video coding
+
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264ENC
+       bool "H.264 encoder"
+       select BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       help
+         Encoder module for H.264 video coding
+
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265DEC
+       bool "H.265 decoder"
+       select BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       help
+         Decoder module for H.265 video coding
+
+config BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265ENC
+       bool "H.265 encoder"
+       select BR2_PACKAGE_MESA3D_VIDEO_CODEC
+       help
+         Encoder module for H.265 video coding
+
 # inform the .mk file of gallium, dri, dri3 or vulkan driver selection
 config BR2_PACKAGE_MESA3D_DRI3
        bool
diff --git a/package/mesa3d/mesa3d.mk b/package/mesa3d/mesa3d.mk
index 6fa5c1c686..149e6c2e7f 100644
--- a/package/mesa3d/mesa3d.mk
+++ b/package/mesa3d/mesa3d.mk
@@ -98,6 +98,17 @@ else
 MESA3D_CONF_OPTS += -Dgallium-vc4-neon=disabled
 endif
 
+# Video codecs (patent encumbered)
+MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_VC1DEC)  += vc1dec
+MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264DEC) += h264dec
+MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264ENC) += h264enc
+MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265DEC) += h265dec
+MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265ENC) += h265enc
+
+ifeq ($(BR2_PACKAGE_MESA3D_VIDEO_CODEC),y)
+MESA3D_CONF_OPTS += -Dvideo-codecs=$(subst $(space),$(comma),$(MESA3D_VIDEO_CODECS-y))
+endif
+
 # Drivers
 
 #Gallium Drivers
-- 
2.25.1


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/mesa3d: Allow building patent encumbered video codecs

[Tristan van Berkom via buildroot]

Hi,

On Fri, 2023-08-04 at 20:12 +0900, Tristan van Berkom via buildroot
wrote:
> This patch adds some options to allow building the patent encumbered
> codecs in mesa, such as H.264/H.265 encoders/decoders which are
> useful
> for hardware accelerated decoding via libva/gstreamer.
> 

It's been a couple weeks and I've been following the mailing list, just
a friendly reminder, please let me know if there is anything wrong with
this patch, I've tested this and followed similar patterns found in the
mesa3d.mk and its Config.in.

We require H.264/H.265 support for hardware acceleration in our
gstreamer pipelines and would hope to see this support in some form in
a subsequent buildroot release rather than maintaining our downstream
patches.

I filed an issue about this today:
    https://bugs.busybox.net/show_bug.cgi?id=15736

Kind regards,
    -Tristan


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/mesa3d: Allow building patent encumbered video codecs

[Thomas Petazzoni via buildroot]

Hello Tristan,

On Fri, 04 Aug 2023 20:12:43 +0900
Tristan van Berkom via buildroot <buildroot@buildroot.org> wrote:

> This patch adds some options to allow building the patent encumbered
> codecs in mesa, such as H.264/H.265 encoders/decoders which are useful
> for hardware accelerated decoding via libva/gstreamer.
> 
> These codecs are now disabled by default in upstream mesa as per:
>   https://gitlab.freedesktop.org/mesa/mesa/-/commit/7d969fe9e91e39e03041cdfac69bf33337bc2c96
> 
> Signed-off-by: Tristan van Berkom <tristan.vanberkom@codethink.co.uk>
> ---
>  package/mesa3d/Config.in | 36 ++++++++++++++++++++++++++++++++++++
>  package/mesa3d/mesa3d.mk | 11 +++++++++++
>  2 files changed, 47 insertions(+)

Thanks for your patch. I have one implementation suggestion (which I
had already implemented locally as I was about to merge your patch) but
also a more fundamental question that I was not able to answer directly.

> +# inform the .mk file of video codec selection
> +config BR2_PACKAGE_MESA3D_VIDEO_CODEC
> +       bool

This option is not needed, and all the "select" of it can be dropped.

> +# Video codecs (patent encumbered)
> +MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_VC1DEC)  += vc1dec
> +MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264DEC) += h264dec
> +MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264ENC) += h264enc
> +MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265DEC) += h265dec
> +MESA3D_VIDEO_CODECS-$(BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265ENC) += h265enc
> +
> +ifeq ($(BR2_PACKAGE_MESA3D_VIDEO_CODEC),y)

Replace this with:

ifneq ($(MESA3D_VIDEO_CODECS-y),)

and tada, the option BR2_PACKAGE_MESA3D_VIDEO_CODEC can be dropped.

However, the more fundamental question is: what do those options do?

If I build a defconfig like this:

BR2_arm=y
BR2_cortex_a9=y
BR2_ARM_ENABLE_VFP=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_MESA3D=y
BR2_PACKAGE_MESA3D_VIDEO_CODEC_VC1DEC=y
BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264DEC=y
BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264ENC=y
BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265DEC=y
BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265ENC=y
# BR2_TARGET_ROOTFS_TAR is not set

Then absolutely nothing gets installed by mesa3d.

Looking more closely, the -Dvideo-codecs option is doing this:

_codecs = get_option('video-codecs')
foreach c : ['vc1dec', 'h264dec', 'h264enc', 'h265dec', 'h265enc']
   pre_args += '-DVIDEO_CODEC_@0@=@1@'.format(c.to_upper(), _codecs.contains(c).to_int())
endforeach

So it's basically defining VIDEO_CODEC_VC1DEC, VIDEO_CODEC_H264DEC,
etc. So I grepped in the mesa3d tree, and only found references in
src/gallium/auxiliary/vl/vl_codec.c:

bool vl_codec_supported(struct pipe_screen *screen,
                        enum pipe_video_profile profile,
                        bool encode)
{
   if (profile == PIPE_VIDEO_PROFILE_VC1_SIMPLE ||
       profile == PIPE_VIDEO_PROFILE_VC1_MAIN ||
       profile == PIPE_VIDEO_PROFILE_VC1_ADVANCED) {
      if (!VIDEO_CODEC_VC1DEC)
         return false;
   }
   if (profile >= PIPE_VIDEO_PROFILE_MPEG4_AVC_BASELINE &&
       profile <= PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH444) {
      if (encode) {
         if (!VIDEO_CODEC_H264ENC)
            return false;
      } else if (!VIDEO_CODEC_H264DEC) {
         return false;
      }
   }
   if (profile >= PIPE_VIDEO_PROFILE_HEVC_MAIN &&
       profile <= PIPE_VIDEO_PROFILE_HEVC_MAIN_444) {
      if (encode) {
         if (!VIDEO_CODEC_H265ENC)
            return false;
      } else if (!VIDEO_CODEC_H265DEC) {
         return false;
      }
   }

   return screen->get_video_param(screen, profile, encode ? PIPE_VIDEO_ENTRYPOINT_ENCODE : PIPE_VIDEO_ENTRYPOINT_BITSTREAM, PIPE_VIDEO_CAP_SUPPORTED);
}

So essentially *not* defining those options only make this function
report that some codecs are not supported. Defining those options do
not seem to add any extra functionality to mesa3d.

Could you clarify what's happening here?

Also, since it's affecting code in src/gallium/, maybe those new
options need to be conditional on having at least one Gallium driver
enabled? That being said, even if I enable the swrast Gallium driver,
vl_codec.c doesn't get compiled in, so it's not just having a Gallium
driver enabled.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/mesa3d: Allow building patent encumbered video codecs

[Tristan van Berkom via buildroot]

Hello Thomas,

My inbox has been crowded and this email has escaped my attention since
wednesday, thanks for your valuable attention to my patch :)

Also, I only realized this now, but we also have a requirement to
enable -Dgallium-va=enabled in order to use the codecs - while the
codecs might only be relevant for the vulkan interfaces without also
fixing that in buildroot.

Came to that conclusion at the end of writing this mail but thought I
should lead with this, as it may make sense to hold back on this change
until we can also safely turn on -Dgallium-va (and/or -Dgallium-vdpau).

On Tue, 2023-08-22 at 23:28 +0200, Thomas Petazzoni wrote:
> Hello Tristan,
> 
[...]
> Thanks for your patch. I have one implementation suggestion (which I
> had already implemented locally as I was about to merge your patch) but
> also a more fundamental question that I was not able to answer directly.
> 

I've made your suggested changes and tested this locally and indeed
this works well.

> However, the more fundamental question is: what do those options do?
> 
> If I build a defconfig like this:
> 
> BR2_arm=y
> BR2_cortex_a9=y
> BR2_ARM_ENABLE_VFP=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_MESA3D=y
> BR2_PACKAGE_MESA3D_VIDEO_CODEC_VC1DEC=y
> BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264DEC=y
> BR2_PACKAGE_MESA3D_VIDEO_CODEC_H264ENC=y
> BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265DEC=y
> BR2_PACKAGE_MESA3D_VIDEO_CODEC_H265ENC=y
> # BR2_TARGET_ROOTFS_TAR is not set

In one example case (we have several platforms which this affects), I
believe it is the BR2_PACKAGE_MESA3D_GALLIUM_DRIVER_RADEONSI which will
lead to the installation of radeonsi_drv_video.so.


[...]
> 
> So essentially *not* defining those options only make this function
> report that some codecs are not supported. Defining those options do
> not seem to add any extra functionality to mesa3d.
> 
> Could you clarify what's happening here?
> 

Interesting, I was not aware of the implementation detail and indeed I
would have expected excluded files or #ifdef block being conditionally
included at build time.

> Also, since it's affecting code in src/gallium/, maybe those new
> options need to be conditional on having at least one Gallium driver
> enabled? That being said, even if I enable the swrast Gallium driver,
> vl_codec.c doesn't get compiled in, so it's not just having a Gallium
> driver enabled.

I understand that your concern is whether this added option should be
conditional to other options, and this is still a bit unclear to me.

That said, I think it would be beneficial to lean towards the expressed
intent of the added -Dvideo-codecs option in mesa, rather than the
current implementation in code.

Looking at the definition of the option:

option(
  'video-codecs',
  type : 'array',
  value : [],
  choices: [
    'vc1dec', 'h264dec', 'h264enc', 'h265dec', 'h265enc'
  ],
  description : 'List of patent encumbered codecs to build support for. ' +
                'Distros might want to consult their legal department before ' +
                'enabling these. This is used for all video APIs (vaapi, ' +
                'vdpau, vulkan). Non-patent encumbered codecs will be ' +
                'enabled by default.'
)

Here the upstream is telling us that the options are relevant for
vaapi, vdpau and vulkan interfaces.

Looking at the meson options, it seems that both va / vdpau are
interfaces to the gallium drivers, and so I suppose one could say that
these video codec options are only relevant if:

  * At least one vulkan driver is selected
  * At least one gallium driver is selected
    * AND either -Dgallium-vdpau or -Dgallium-va enabled

Now I realize, that we are actually ALSO carrying an ugly downstream
patch in order to enable -Dgallium-va, and I will need to investigate
whether it is still required (the claim is that libva has a link time
requirement on mesa3d libraries while mesa3d requires the libva
headers) - this part will be a bit more tricky to cleanup and upstream
if the problem still exists.

Cheers,
    -Tristan

[0]: https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/meson_options.txt#L626


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot