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

Require TagLib 1.11 or newer #4251

Merged
merged 4 commits into from
Aug 28, 2021
Merged

Require TagLib 1.11 or newer #4251

merged 4 commits into from
Aug 28, 2021

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Aug 28, 2021

...to get rid of untested legacy code.

Do we need a redundant check in CMakeLists.txt in addition to the static_assert in the code? I have left a TODO comment.

This might also fix https://bugs.launchpad.net/bugs/1940777: The invocation of SoundSource::importTrackMetadataAndCoverImage() in SoundSourceOpus::importTrackMetadataAndCoverImage() was accepted by all compilers even though this is slightly misleading. It should have been MetadataSource::importTrackMetadataAndCoverImage().

@uklotzde uklotzde added this to the 2.4.0 milestone Aug 28, 2021
@github-actions github-actions bot added the build label Aug 28, 2021
CMakeLists.txt Outdated Show resolved Hide resolved
@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

Ubuntu 20.04 ships TagLib 1.11.1. To maintain adherence to our minimum requirements policy we need to either:

  • Wait on removing these hacks for old TagLib versions.
  • Vendor TagLib 1.12
  • Tell developers on Ubuntu 20.04 to build TagLib from source or vcpkg. This could be a decent option if we add a vcpkg.json to this repository.
  • Tell developers on Ubuntu 20.04 to build a Flatpak manifest. GNOME Builder has nice integration with this, but I don't know of other IDEs that do.

@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

Disregard the last comment. I mistakenly thought TagLib 1.12 was required.

@uklotzde
Copy link
Contributor Author

This PR does not conflict with the minimum requirements policy. We only require TagLib 1.11.x here. This version has serious, known bugs (e.g. OGG corruption) and we still maintain workarounds.

@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

+38 −228 :)

@uklotzde
Copy link
Contributor Author

Why does the build fail on Windows? We package version 1.12.

@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

Are you sure the current Windows build environment has TagLib 1.12? This might need to wait for #4225.

@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

Oh I think it is an issue with the version detection.

On Windows, the pkgconfig file is found and the variable is
defined but somehow set to empty string.
@Be-ing
Copy link
Contributor

Be-ing commented Aug 28, 2021

I fixed the version detection on Windows: https://github.com/uklotzde/mixxx/pull/18

CMake: fix version detection for TagLib
@Be-ing Be-ing merged commit 0dba5ac into mixxxdj:main Aug 28, 2021
@uklotzde uklotzde deleted the taglib-1.11 branch August 29, 2021 16:57
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.

2 participants