-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update gstreamer and imx-opencl-converter from 6.6.3-1.0.0 to 6.6.23-2.0.0 #1874
Conversation
[1] openembedded/openembedded-core@d46660e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some inconsistency in the version used, some recipes uses the ${@get_gst_ver("${PV}")}
and others 1.24.0
and 1.24.5
file://0002-tests-add-support-for-install-the-tests.patch \ | ||
file://0003-tests-use-a-dictionaries-for-environment.patch;striplevel=3 \ | ||
file://0004-tests-add-helper-script-to-run-the-installed_tests.patch;striplevel=3 \ | ||
" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder who was this tested? look like all the test patches were inadvertently removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad, I copied the recipe from meta-imx layer and they explicity removed these patches there. But looking at the patch and the MR opened at gstreamer upstream, I have added them back.
The only thing is that I had to rework them, because they were not applying anymore. I also removed the "striplevel=3" from patches 0003 and 0004 because this was making them fail to apply. Please let me know if something else is needed, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not aware of the meta-imx but these patches are still available upstream in oe-core. They are so we can run the unit tests with the oe-core ptest infrastructure.
All of these patches are still in use and mandatory upstream in oe-core otherwise we can't run the unit test on the target using the ptest
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right, thanks for the explanation. So let's keep them, not sure why meta-imx removed.
|
||
S = "${WORKDIR}/git" | ||
|
||
PACKAGECONFIG[tests] = "-Dtests=enabled,-Dtests=disabled" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not needed because it is already present on line 43
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this, this recipe is copied from OE upstream and then these variables are replaced only at the end of the file, after the ########### i.MX overrides ################
.
The differences are:
PACKAGECONFIG[tests] = "-Dtests=enabled -Dinstalled_tests=true,-Dtests=disabled -Dinstalled_tests=false"
PACKAGECONFIG[tests] = "-Dtests=enabled,-Dtests=disabled"
So the flags are changing, but I do not know how relevant they are. That being said, should I remove this line anyway?
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The meson "-Dtests" doesn't make much sense without the mentioned patches above. The PACKAGECONFIG[tests] is still in use as well upstream in oe-core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the line
PACKAGECONFIG[tests] = "-Dtests=enabled,-Dtests=disabled"
thanks for clarifying.
d45ccad
to
8b23435
Compare
fixed, thanks! |
some still missing:
|
The
|
Please try to follow the pattern in all recipes:
Maybe this seems a little petty but gstreamer was one of my gateway into oe-core and I'm just trying to help :) |
Actually for gstreamer1.0-python I selected 1.24.5 on purpose, because I could not compile version 1.24.0: Called: `/home/frh/tdx/oe/master/build/tmp/work/armv8a-mx8-tdx-linux/gstreamer1.0-python/1.24.0.imx/recipe-sysroot-native/usr/bin/pkg-config --libs python-3.12-embed` -> 0
stdout:
-lpython3.12
-----------
Run-time dependency python found: YES 3.12
Message: python_abi_flags =
../gst-python-1.24.0/meson.build:84:16: ERROR: Unknown variable "python_opt". So I checked the OE upstream and I aw they did not have any version 1.24.0, instead, they are using 1.24.5 (https://github.com/openembedded/openembedded-core/blob/master/meta/recipes-multimedia/gstreamer/gstreamer1.0-python_1.24.5.bb), therefore I thought we could use the same as the upstream (and this one compile without any issues). What is your opinion? Should we stick to 1.24.0? |
Got it. I tried to compile, and I saw it was failing because the LICENSE_FLAGS_ACCEPTED was missing, so checking the meta-imx from NXP I saw they added it to their recipes (gstreamer1.0-libav, gstreamer1.0-plugins-ugly and ffmpeg), that is why I added them here too. Not sure what to do here, because without these lines we cannot use the recipes. |
So this is also something that confuses me, to be honest. For recipes that are available on meta-imx, we have this "i.MX overrides" comment, which comes from meta-imx itself. For recipes they do not have it, gstreamer1.0-python is one example, we do not use it. So it should be correct.
No worries, I really appreciate your help =) Reviewing now is better than fixing later, just part of the process, thanks for the comments. |
8b23435
to
b095253
Compare
There are some sets of gstreamer recipes on this layer :
One of the reasons is because the versions have to be the same and we cannot mix versions of different gstreamer plugins because this is not supported upstream by the gstreamer project and as far as I know it is not tested or used by anyone. GStreamer forked recipesmeta-freescale/conf/machine/include/imx-base.inc Lines 512 to 520 in 5a46df3
GStreamer copied recipesmeta-freescale/conf/machine/include/imx-base.inc Lines 522 to 534 in 5a46df3
GStreamer downgrade ffmpegmeta-freescale/conf/machine/include/imx-base.inc Lines 536 to 538 in 5a46df3
You can look on the last update to better understand the process |
b095253
to
415a068
Compare
Thanks for the nice explanation, @quaresmajose. I was able to apply a patch to gstreamer1.0-python and use the 1.24.0 version. Now it is compiling without errors. Please take a look, thank you! |
415a068
to
f58a019
Compare
Update: imx-gst1.0-plugin works fine now, I had to update imx-opencl-converter in order to fix the build, as one is dependent on the other. Therefore, I pushed one more commit to this PR. 0.2.0 -> 0.4.0 |
f58a019
to
631a3e1
Compare
@@ -183,3 +183,5 @@ INSANE_SKIP:${MLPREFIX}libpostproc = "textrel" | |||
# Downgrade for NXP BSP | |||
DEFAULT_PREFERENCE = "-1" | |||
COMPATIBLE_MACHINE = "(imx-nxp-bsp)" | |||
|
|||
LICENSE_FLAGS_ACCEPTED += " commercial " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed, it was a mistake
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. Should I also remove it from gstreamer1.0-plugins-ugly
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I didn't notice that one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, removed, thank you
|
||
COMPATIBLE_MACHINE = "(imx-nxp-bsp)" | ||
|
||
LICENSE_FLAGS_ACCEPTED += " commercial " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
"${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gles2 egl', '', d)}" | ||
PACKAGECONFIG_GL:use-mainline-bsp = \ | ||
"${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gles2 egl gbm', '', d)}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addition and the next addition are not needed, as they are in the .bbappend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right, removed it.
PACKAGECONFIG:remove = "${PACKAGECONFIG_REMOVE}" | ||
PACKAGECONFIG_REMOVE ?= "jpeg" | ||
|
||
PACKAGECONFIG_GL:imxgpu2d:append:mx6-nxp-bsp = " viv-fb" | ||
PACKAGECONFIG_GL:imxgpu2d:append:mx7-nxp-bsp = " viv-fb" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, please take this line over to the bbappend, making the override generic for all i.MX 7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it, but without the imxgpu2d
override because the last one, version 1.22, did not have it. Should I keep it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think it needs the imxgpu
override.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All right, I added them, thanks.
631a3e1
to
69c3f9e
Compare
Upgrade gstreamer and all related recipes to 1.24.0, syncing with meta-imx layer. Signed-off-by: Hiago De Franco <[email protected]>
Update imx-opencl-converter to version 0.4.0, syncing with meta-imx layer and the new NXP BSP 6.6.23-2.0.0. Signed-off-by: Hiago De Franco <[email protected]>
69c3f9e
to
0f663b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @hiagofranco and @quaresmajose!
PACKAGECONFIG_GL:append:mx6-nxp-bsp = " viv-fb " | ||
PACKAGECONFIG_GL:append:mx7ulp-nxp-bsp = " viv-fb " | ||
PACKAGECONFIG_GL:imxgpu2d:append:mx6-nxp-bsp = " viv-fb " | ||
PACKAGECONFIG_GL:imxgpu2d:append:mx7-nxp-bsp = " viv-fb " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this not works, the append
needs to be the first override in the list. Can you please verify the PACKAGECONFIG_GL
on any of the referred machines ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary, these are appending to the assignments on lines 1-4.
Perhaps it could be clearer by dropping the append
and adding a variable?
PACKAGECONFIG_GL:imxgpu2d = " \
${@bb.utils.contains('DISTRO_FEATURES', 'opengl x11', 'opengl', '', d)} \
${PACKAGECONFIG_GL_VIV} \
"
PACKAGECONFIG_GL:imxgpu3d = " \
${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gles2 egl', '', d)} \
${PACKAGECONFIG_GL_VIV} \
"
PACKAGECONFIG_GL:use-mainline-bsp = \
"${@bb.utils.contains('DISTRO_FEATURES', 'opengl', 'gles2 egl gbm', '', d)}"
# The i.MX8 uses KMS instead of the Vivante specific framebuffer API.
# The i.MX7 does not have a GPU, except for ULP.
# This leaves the i.MX6 - with the vendor BSP - as the remaining use case for viv-fb.
#
# (Note that viv-fb is about the _windowing system_. Vivante direct texture support
# does not depend on the viv-fb feature. It used to, but that was actually a bug
# which was fixed in GStreamer 1.22.5. Since then, the direct texture support is
# detected by Meson by checking for direct texture symbols like "glTexDirectVIV".)
PACKAGECONFIG_GL_VIV = ""
PACKAGECONFIG_GL_VIV:mx6-nxp-bsp = " viv-fb "
PACKAGECONFIG_GL_VIV:mx7-nxp-bsp = " viv-fb "
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern is just about the append
positions, I think it should be
-PACKAGECONFIG_GL:imxgpu2d:append:mx6-nxp-bsp = " viv-fb "
-PACKAGECONFIG_GL:imxgpu2d:append:mx7-nxp-bsp = " viv-fb "
+PACKAGECONFIG_GL:append:imxgpu2d:mx6-nxp-bsp = " viv-fb "
+PACKAGECONFIG_GL:append:imxgpu2d:mx7-nxp-bsp = " viv-fb "
but have tested locally and it works in the two cases. Sorry for the noise
#1861