Skip to content
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

Upgrade Multimedia package base on 6.6.52 #2055

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

zelan-nxp
Copy link
Contributor

@zelan-nxp zelan-nxp commented Jan 7, 2025

Upstream MM packages:

dsp: 2.1.8 -> 2.1.9
opencl-converter: 0.5.0 ->0.6.0
imx-parser: 4.9.1- >4.9.2
alsa-lib: upstream patch.
imx-vpuwrapper: drop unneeded patch

recipes-multimedia/alsa/alsa-lib_%.bbappend Outdated Show resolved Hide resolved
@otavio
Copy link
Member

otavio commented Jan 7, 2025

I would like to clarify a comment I made earlier. It would be beneficial to improve the comments in each commit log. Specifically, I suggest describing the individual changes made to each recipe within their respective commit logs. This detailed documentation will be useful for referencing the reasons behind each change in the future.

@otavio otavio mentioned this pull request Jan 7, 2025
18 tasks
@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch from 876ea2c to bf8e076 Compare January 8, 2025 09:49
# it uses nested functions sadly, in ext/wayland/gstwaylandsink.c for GST_ELEMENT_REGISTER_DEFINE
#
TOOLCHAIN = "gcc"

Copy link
Contributor

@thochstein thochstein Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether this can be removed. It appears that we have been building without this change all along, which makes me think that we do not build whatever causes the problem. It might be better to not upstream this and instead to align meta-imx with this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely to avoid using other toolchain in case it is chosen as default. For example, it is likely to fix a problem if the distribution uses CLang as default toolchain.

Copy link
Contributor

@thochstein thochstein Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, so bottom line this is backwards again. @zelan-nxp, please remove this change here and instead add the lines to meta-imx.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zelan-nxp, I tried to clarify my comment. We need to take these lines into meta-imx, not remove them from meta-freescale.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@otavio, this item is marked resolved but is not. Can you Unresolve it?

file://0003-tests-use-a-dictionaries-for-environment.patch \
file://0004-tests-add-helper-script-to-run-the-installed_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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was dropped upstream.

@zelan-nxp, please check that our recipes in meta-imx are aligned with the latest for the next release.

Copy link
Contributor Author

@zelan-nxp zelan-nxp Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I check the next release ,It was droppd. Just align with meta-imx, It's ok as we have override them like this:

# Use i.MX fork of GST for customizations
SRC_URI:remove = "https://gstreamer.freedesktop.org/src/gstreamer/gstreamer-${PV}.tar.xz \
                file://0001-tests-respect-the-idententaion-used-in-meson.patch \
                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 \
"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really didn't understand what you meant. Do you mean that all those patches are not applied in your new version?

You can drop them from your upcoming recipe; however, I don't understand why you are dropping them. Considering that you are basing it on a specific recipe version, you should keep it as close as possible to the copy that you are doing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's happening here is a previous upstreaming did change these lines so they no longer match the copied recipe (from OE-Core/poky). So, @zelan-nxp's change here is re-aligning the copied recipe.

@zelan-nxp, we need keep our copied recipe in meta-imx up to date. The recipe in OE-Core removed ;striplevel=3 from these lines some time ago: openembedded/openembedded-core@d0a546e.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zelan-nxp see @thochstein comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be okay to take this change. It does make the copied recipe actually match the checksum it claims to be copied from. Unless there was some reason why we need to remove the change. @hiagofranco, do you remember why you removed ;striplevel=3?

@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch 2 times, most recently from 2049471 to 671393d Compare January 9, 2025 11:11
file://gst/replaygain/rganalysis.c;beginline=1;endline=23;md5=b60ebefd5b2f5a8e0cab6bfee391a5fe \
"
# Enable pulsesink in gstreamer
PACKAGECONFIG:append = "${@bb.utils.contains('DISTRO_FEATURES', 'pulseaudio', 'pulseaudio', '', d)}"
PACKAGECONFIG:append = " pulseaudio"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a wrong change because that would cause this feature to be enabled independently if it is supported by the distribution or not.

Copy link
Member

@otavio otavio Jan 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PACKAGECONFIG:append = " pulseaudio"
PACKAGECONFIG:append = "${@bb.utils.contains('DISTRO_FEATURES', 'pulseaudio', ' pulseaudio', '', d)}"

@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch from 671393d to 73aec8c Compare January 9, 2025 12:39
@otavio otavio self-requested a review January 13, 2025 20:14
@otavio
Copy link
Member

otavio commented Jan 13, 2025

Please rebase this pull request because I made the change to remove the checksum across all the files to a single commit. So you will need also to remove this from this and other pull requests.

@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch from 73aec8c to b45e4fc Compare January 14, 2025 02:17

inherit fsl-eula-unpack autotools pkgconfig
inherit fsl-eula-recent autotools pkgconfig fsl-eula2-unpack2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zelan-nxp, please don't upstream fsl-eula-recent and fsl-eula2 usage. Update the recipe without these classes. I need to work on it before upstreaming.


S = "${WORKDIR}/git"

PACKAGECONFIG[tests] = "-Dtests=enabled,-Dtests=disabled"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the extra patchset for tests is been removed we should also explicitly disable the tests because they won't work.

PACKAGECONFIG[tests] = ""
EXTRA_OEMESON += "-Dtests=disabled"

@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch from b45e4fc to 44d3478 Compare January 15, 2025 11:29
Bump version 2.1.8 -> 2.1.9

Signed-off-by: Zelan Zou <[email protected]>
Bump version 0.5.0 -> 0.6.0

Signed-off-by: Zelan Zou <[email protected]>
Bump version 4.9.1 -> 4.9.2

Signed-off-by: Zelan Zou <[email protected]>
@zelan-nxp zelan-nxp force-pushed the multimedia-package-6.6.52 branch from 44d3478 to ee407ee Compare January 15, 2025 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants