[meta-freescale] [PATCH 0/2] Fix Hantro/Gstreamer build issues

[Gary Bisson]

Hi Otavio/Tom/Carol,

Please consider this series to fix the build of the latest multimedia
components without having to change the libc-headers version.

The issue currently is that the code relies on LINUX_VERSION_CODE but it
uses the toolchain headers by default. So use the kernel builddir
verion.h instead.

If you accept this approach, please leave the ownership of the
imx-vpu-hantro patch as-is as it wasn't very "open-sourcy" of you to
keep my previous patch/title/log but remove my SOB/ownership...

This has been tested on Nitrogen8M:
- Using a 720p MIPI display (eLCDIF output)
- Decoding SKYFALL trailer [1] gives 16fps only
- Seems that some conf is missing to achieve 30fps like before
- Here is the command used to test it:
# gst-launch-1.0 filesrc location=/home/root/SKYFALL-4K.mp4 ! \
parsebin ! vpudec ! glimagesink

Regards,
Gary

[1] http://downloads.4ksamples.com/downloads/SKYFALL%204K%20(Ultra%20HD)%20(4ksamples.com).mp4

Gary Bisson (2):
imx-vpu-hantro: fix build issue
gstreamer1.0-plugins-base: fix build issue

...clusion-to-be-from-kernel-build-fold.patch | 44 +++++++++++++++++++
.../imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb | 5 ++-
.../gstreamer1.0-plugins-base_1.14.imx.bb | 1 +
3 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch

[Gary Bisson]

Just like imx-vpu-hantro, this package now depends on LINUX_VERSION_CODE
which should be retrieved from kernel build folder directly instead of
relying on the toolchain kernel headers.

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
.../gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb | 1 +
1 file changed, 1 insertion(+)

diff --git a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb
index 3d31d22f..a9259946 100644
--- a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb
+++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb
@@ -67,6 +67,7 @@ EXTRA_OECONF += " \
-I${STAGING_KERNEL_DIR}/include/uapi \
-I${STAGING_KERNEL_DIR}/include \
-I${STAGING_KERNEL_DIR}/drivers/staging/android/uapi \
+ -I${STAGING_KERNEL_BUILDDIR}/include/generated/uapi \
" \
"

[Gary Bisson]

The source code uses LINUX_VERSION_CODE at many places which constraints
the package to be built with a toolchain whose headers match the kernel.

This is a far from ideal solution, especially if one wants to use a
prebuilt toolchain (with unknown kernel header version).

So change the CFLAGS to consider the kernel build folder so that the
Linux version test actually matches the kernel built.

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
...clusion-to-be-from-kernel-build-fold.patch | 44 +++++++++++++++++++
.../imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb | 5 ++-
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch

diff --git a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch
new file mode 100644
index 00000000..e9bf9257
--- /dev/null
+++ b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch
@@ -0,0 +1,44 @@
+From 1d7b7046c8f735e150e92aeace3fe6d0686b9bc9 Mon Sep 17 00:00:00 2001
+From: Gary Bisson <***@boundarydevices.com>
+Date: Wed, 3 Oct 2018 10:52:29 +0200
+Subject: [PATCH] Fix version.h inclusion to be from kernel build folder
+
+Instead of relying on Toolchain headers which can be from newer kernel.
+
+Signed-off-by: Gary Bisson <***@boundarydevices.com>
+---
+ Makefile_G1G2 | 3 +++
+ Makefile_H1 | 3 +++
+ 2 files changed, 6 insertions(+)
+
+diff --git a/Makefile_G1G2 b/Makefile_G1G2
+index 29866a4..c473bcb 100755
+--- a/Makefile_G1G2
++++ b/Makefile_G1G2
+@@ -16,6 +16,9 @@ INCLUDE_HEADERS += -I$(LINUX_KERNEL_ROOT)/include/uapi -I$(LINUX_KERNEL_ROOT)/in
+ # ION header location
+ INCLUDE_HEADERS += -I$(LINUX_KERNEL_ROOT)/drivers/staging/android/uapi
+
++# LINUX_VERSION_CODE from kernel build folder instead of toolchain headers
++INCLUDE_HEADERS += -I$(LINUX_KERNEL_BUILD)/include/generated/uapi
++
+ CFLAGS += -DDEC_MODULE_PATH=\"/dev/mxc_hantro\" -DUSE_FAKE_RFC_TABLE -DFIFO_DATATYPE=void* -DNDEBUG -DDOWN_SCALER \
+ -DUSE_EXTERNAL_BUFFER -DUSE_FAST_EC -DUSE_VP9_EC -DGET_FREE_BUFFER_NON_BLOCK \
+ -DDEC_X170_OUTPUT_FORMAT=0 -DDEC_X170_TIMEOUT_LENGTH=-1 -DENABLE_HEVC_SUPPORT \
+diff --git a/Makefile_H1 b/Makefile_H1
+index 56b4332..0be43ce 100755
+--- a/Makefile_H1
++++ b/Makefile_H1
+@@ -23,6 +23,9 @@ ENV += -I$(SDKTARGETSYSROOT)/usr/imx/include
+ # ION header location
+ ENV += -I$(LINUX_KERNEL_ROOT)/drivers/staging/android/uapi
+
++# LINUX_VERSION_CODE from kernel build folder instead of toolchain headers
++INCLUDE_HEADERS += -I$(LINUX_KERNEL_BUILD)/include/generated/uapi
++
+ LIBENCNAME = libcodec_enc
+ LIBSENC = -L./ -lhantro_h1 -lpthread
+
+--
+2.19.0
+
diff --git a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
index 243f1f35..b5fc8468 100644
--- a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
+++ b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
@@ -12,6 +12,7 @@ PROVIDES = "virtual/imxvpu"
SRC_URI = " \
${FSL_MIRROR}/${PN}-${PV}.bin;fsl-eula=true \
file://0001-Fix-ion.h-header-inclusion-to-be-standard.patch \
+ file://0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch \
"
SRC_URI[md5sum] = "140796ddd6f1be47cffb7e5e2bfe0fb6"
SRC_URI[sha256sum] = "c092a5b0f8897bae54154f58e47b6d2de033da01ee231a8cd779a51bbe962606"
@@ -24,7 +25,9 @@ PLATFORM_mx8mm = "IMX8MM"
PLATFORM_mx8mq = "IMX8MQ"

do_compile () {
- oe_runmake CROSS_COMPILE="${HOST_PREFIX}" LINUX_KERNEL_ROOT="${STAGING_KERNEL_DIR}" SDKTARGETSYSROOT="${STAGING_DIR_TARGET}" PLATFORM="${PLATFORM}" all
+ oe_runmake CROSS_COMPILE="${HOST_PREFIX}" LINUX_KERNEL_BUILD="${STAGING_KERNEL_BUILDDIR}" \
+ LINUX_KERNEL_ROOT="${STAGING_KERNEL_DIR}" SDKTARGETSYSROOT="${STAGING_DIR_TARGET}" \
+ PLATFORM="${PLATFORM}" all
}

do_install () {

[Carlos Rafael Giani]

In addition to this, I think adding a "hantro_config.h" header would be
useful. That's because we can find this in the Makefile_codec file:

  CFLAGS += -DSET_OUTPUT_CROP_RECT -DUSE_EXTERNAL_BUFFER
-DUSE_OUTPUT_RELEASE -DVSI_API -DIS_G1_DECODER -DENABLE_CODEC_VP8
-DVP8_HWTIMEOUT_WORKAROUND -DENABLE_CODEC_MJPEG
-DGET_FREE_BUFFER_NON_BLOCK -DDOWN_SCALER

and you need to set these flags in your code as well, otherwise you get
stack corruption. For example, SET_OUTPUT_CROP_RECT enables certain
fields in openmax_il/source/decoder/codec.h :

    typedef struct STREAM_INFO
    {
        OMX_COLOR_FORMATTYPE format;    // stream color format
        OMX_U32 framesize;              // framesize in bytes
        OMX_U32 width;                  // picture display width
        OMX_U32 height;                 // picture display height
        OMX_U32 sliceheight;            // picture slice height
        OMX_U32 stride;                 // picture scan line width
        OMX_BOOL interlaced;            // sequence is interlaced
        OMX_U32 imageSize;              // size of image in memory
        OMX_BOOL isVc1Stream;           // sequence is VC1 stream
#ifdef SET_OUTPUT_CROP_RECT
        OMX_BOOL crop_available;        // crop information
        OMX_U32 crop_width;
        OMX_U32 crop_height;
        OMX_U32 crop_left;
        OMX_U32 crop_top;
#endif
        OMX_U32 frame_buffers;
        OMX_U32 bit_depth;
        OMX_BOOL hdr10_available;
        METADATA_HDR10 hdr10_metadata;   // HDR10 metadata
        OMX_U32 video_full_range_flag;   // black level and range of
luma chroma signals
        OMX_BOOL colour_desc_available;  // indicate
colour_primaries/transfer_characteristics/matrix_coeffs present or not
        OMX_U32 colour_primaries;        // chromaticity coordinates of
the source primaries
        OMX_U32 transfer_characteristics;// opto-electronic transfer
function
        OMX_U32 matrix_coeffs;           // matrix coefficients used in
deriving luma and chroma signals
        OMX_BOOL chroma_loc_info_available; // indicate
chroma_sample_loc_type_top_field or bottom field are present or not
        OMX_U32 chroma_sample_loc_type_top_field; // specify the
location of chroma samples
        OMX_U32 chroma_sample_loc_type_bottom_field; // specify the
location of chroma samples
    } STREAM_INFO;

As a result, imx-vpuwrap has to set these same cflags. If you don't know
this already, it is easy to miss, and causes ABI breakage.

Post by Gary Bisson
The source code uses LINUX_VERSION_CODE at many places which constraints
the package to be built with a toolchain whose headers match the kernel.
This is a far from ideal solution, especially if one wants to use a
prebuilt toolchain (with unknown kernel header version).
So change the CFLAGS to consider the kernel build folder so that the
Linux version test actually matches the kernel built.
---
...clusion-to-be-from-kernel-build-fold.patch | 44 +++++++++++++++++++
.../imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb | 5 ++-
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch
diff --git a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch
new file mode 100644
index 00000000..e9bf9257
--- /dev/null
+++ b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro/0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch
@@ -0,0 +1,44 @@
+From 1d7b7046c8f735e150e92aeace3fe6d0686b9bc9 Mon Sep 17 00:00:00 2001
+Date: Wed, 3 Oct 2018 10:52:29 +0200
+Subject: [PATCH] Fix version.h inclusion to be from kernel build folder
+
+Instead of relying on Toolchain headers which can be from newer kernel.
+
+---
+ Makefile_G1G2 | 3 +++
+ Makefile_H1 | 3 +++
+ 2 files changed, 6 insertions(+)
+
+diff --git a/Makefile_G1G2 b/Makefile_G1G2
+index 29866a4..c473bcb 100755
+--- a/Makefile_G1G2
++++ b/Makefile_G1G2
+ # ION header location
+ INCLUDE_HEADERS += -I$(LINUX_KERNEL_ROOT)/drivers/staging/android/uapi
+
++# LINUX_VERSION_CODE from kernel build folder instead of toolchain headers
++INCLUDE_HEADERS += -I$(LINUX_KERNEL_BUILD)/include/generated/uapi
++
+ CFLAGS += -DDEC_MODULE_PATH=\"/dev/mxc_hantro\" -DUSE_FAKE_RFC_TABLE -DFIFO_DATATYPE=void* -DNDEBUG -DDOWN_SCALER \
+ -DUSE_EXTERNAL_BUFFER -DUSE_FAST_EC -DUSE_VP9_EC -DGET_FREE_BUFFER_NON_BLOCK \
+ -DDEC_X170_OUTPUT_FORMAT=0 -DDEC_X170_TIMEOUT_LENGTH=-1 -DENABLE_HEVC_SUPPORT \
+diff --git a/Makefile_H1 b/Makefile_H1
+index 56b4332..0be43ce 100755
+--- a/Makefile_H1
++++ b/Makefile_H1
+ # ION header location
+ ENV += -I$(LINUX_KERNEL_ROOT)/drivers/staging/android/uapi
+
++# LINUX_VERSION_CODE from kernel build folder instead of toolchain headers
++INCLUDE_HEADERS += -I$(LINUX_KERNEL_BUILD)/include/generated/uapi
++
+ LIBENCNAME = libcodec_enc
+ LIBSENC = -L./ -lhantro_h1 -lpthread
+
+--
+2.19.0
+
diff --git a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
index 243f1f35..b5fc8468 100644
--- a/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
+++ b/recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
@@ -12,6 +12,7 @@ PROVIDES = "virtual/imxvpu"
SRC_URI = " \
${FSL_MIRROR}/${PN}-${PV}.bin;fsl-eula=true \
file://0001-Fix-ion.h-header-inclusion-to-be-standard.patch \
+ file://0002-Fix-version.h-inclusion-to-be-from-kernel-build-fold.patch \
"
SRC_URI[md5sum] = "140796ddd6f1be47cffb7e5e2bfe0fb6"
SRC_URI[sha256sum] = "c092a5b0f8897bae54154f58e47b6d2de033da01ee231a8cd779a51bbe962606"
@@ -24,7 +25,9 @@ PLATFORM_mx8mm = "IMX8MM"
PLATFORM_mx8mq = "IMX8MQ"
do_compile () {
- oe_runmake CROSS_COMPILE="${HOST_PREFIX}" LINUX_KERNEL_ROOT="${STAGING_KERNEL_DIR}" SDKTARGETSYSROOT="${STAGING_DIR_TARGET}" PLATFORM="${PLATFORM}" all
+ oe_runmake CROSS_COMPILE="${HOST_PREFIX}" LINUX_KERNEL_BUILD="${STAGING_KERNEL_BUILDDIR}" \
+ LINUX_KERNEL_ROOT="${STAGING_KERNEL_DIR}" SDKTARGETSYSROOT="${STAGING_DIR_TARGET}" \
+ PLATFORM="${PLATFORM}" all
}
do_install () {

--

[Otavio Salvador]

On Wed, Oct 3, 2018 at 10:03 AM Carlos Rafael Giani

Post by Carlos Rafael Giani
In addition to this, I think adding a "hantro_config.h" header would be
CFLAGS += -DSET_OUTPUT_CROP_RECT -DUSE_EXTERNAL_BUFFER
-DUSE_OUTPUT_RELEASE -DVSI_API -DIS_G1_DECODER -DENABLE_CODEC_VP8
-DVP8_HWTIMEOUT_WORKAROUND -DENABLE_CODEC_MJPEG
-DGET_FREE_BUFFER_NON_BLOCK -DDOWN_SCALER
and you need to set these flags in your code as well, otherwise you get
stack corruption. For example, SET_OUTPUT_CROP_RECT enables certain

We are preparing a headers package that will try to reduce the
dependency on the kernel and reduce a lot the machine specific
packages. I think we could make a package for it, and add it as
dependency.

Do you wish to propose one?

[Carlos Rafael Giani]

I am not aware of the kernel dependencies, so I need to read up on the
problems there. Do you have a summary somewhere? Or something half
finished that I can use as inspiration and/or informational document?

Post by Otavio Salvador
On Wed, Oct 3, 2018 at 10:03 AM Carlos Rafael Giani

Post by Carlos Rafael Giani
In addition to this, I think adding a "hantro_config.h" header would be
CFLAGS += -DSET_OUTPUT_CROP_RECT -DUSE_EXTERNAL_BUFFER
-DUSE_OUTPUT_RELEASE -DVSI_API -DIS_G1_DECODER -DENABLE_CODEC_VP8
-DVP8_HWTIMEOUT_WORKAROUND -DENABLE_CODEC_MJPEG
-DGET_FREE_BUFFER_NON_BLOCK -DDOWN_SCALER
and you need to set these flags in your code as well, otherwise you get
stack corruption. For example, SET_OUTPUT_CROP_RECT enables certain

We are preparing a headers package that will try to reduce the
dependency on the kernel and reduce a lot the machine specific
packages. I think we could make a package for it, and add it as
dependency.
Do you wish to propose one?

--

[Otavio Salvador]

On Thu, Oct 4, 2018 at 4:49 PM Carlos Rafael Giani

Post by Carlos Rafael Giani
I am not aware of the kernel dependencies, so I need to read up on the
problems there. Do you have a summary somewhere? Or something half
finished that I can use as inspiration and/or informational document?

Not yet; Tom and I are working on getting a set of headers out of the
kernel so we can share them among machines. I hope it works well ...

[Gary Bisson]

Hi Tom,

Hi Gary,
Thanks for this fix and for letting us know about the ownership mistake.
Related to this, I am working on a dedicated linux-imx-headers recipe with
Otavio. With such a recipe, do you see a problem with copying ion.h from
drivers/staging/android to /usr/include/linux?

Where do you plan on installing those headers? They shouldn't be mixed with
the libc/toolchain headers.

Then I'm not sure to see the benefit of such linux-imx-headers recipe vs.
pointing to the kernel dir as you do it already for many packages:
$ grep -r STAGING_KERNEL_ recipes-{bsp,multimedia} | wc -l
20
$ grep -r STAGING_KERNEL_ recipes-{bsp,multimedia} | sed 's/:.*//' | uniq
recipes-bsp/imx-vpu/imx-vpu_5.4.38.bb
recipes-bsp/imx-test/imx-test_git.bb
recipes-bsp/imx-vpu-hantro/imx-vpu-hantro_1.8.0.bb
recipes-bsp/imx-lib/imx-lib_git.bb
recipes-multimedia/gstreamer/gstreamer1.0-plugins-base_1.14.imx.bb
recipes-multimedia/gstreamer/gstreamer1.0-plugins-imx_git.bb
recipes-multimedia/gstreamer/imx-gst1.0-plugin_4.4.2.bb
recipes-multimedia/alsa/imx-alsa-plugins_1.0.26.bb

Regards,
Gary

[Otavio Salvador]

Related to this, I am working on a dedicated linux-imx-headers recipe with Otavio. With such a recipe, do you see a problem with copying ion.h from drivers/staging/android to /usr/include/linux?

/usr/include/imx/linux

[Otavio Salvador]

Post by Otavio Salvador
/usr/include/imx/linux

For new headers, is there a reason why we can't just install to /usr/include/linux? Will be much nicer than having to modify source code.

Yes; we must know what is specific of i.MX. The best way to fix it is
upstreaming it to mainline Linux and get rid of forked headers ;-)

[Carlos Rafael Giani]

Yeah, but that won't happen until the API is changed to not allow for
physical addresses and to use FDs instead (like from ION or DMA-BUF).
But that will take a while I guess.

Post by Otavio Salvador

Post by Otavio Salvador
/usr/include/imx/linux

For new headers, is there a reason why we can't just install to /usr/include/linux? Will be much nicer than having to modify source code.

Yes; we must know what is specific of i.MX. The best way to fix it is
upstreaming it to mainline Linux and get rid of forked headers ;-)

--