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

Disable track metadata export for .ogg files and TagLib 1.11.1 #2180

Merged
merged 2 commits into from
Jul 7, 2019
Merged

Disable track metadata export for .ogg files and TagLib 1.11.1 #2180

merged 2 commits into from
Jul 7, 2019

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Jun 18, 2019

@uklotzde uklotzde added this to the 2.2.2 milestone Jun 18, 2019
@Pegasus-RPG
Copy link
Member

Wait, do we know if this only affects OGG/Vorbis or any OGG container file?

@uklotzde uklotzde changed the title Disable track metadata export for .ogg files and TagLib 1.11.1 [WiP] Disable track metadata export for .ogg files and TagLib 1.11.1 Jun 19, 2019
@uklotzde
Copy link
Contributor Author

According to the analysis in taglib/taglib#864 only Ogg files are affected.

I will restrict the workaround to Ogg file access instead of writing VorbisComment tags as now.

@uklotzde
Copy link
Contributor Author

I've force pushed a different version that restricts the workaround to write access of TagLib::Ogg::Vorbis::File

@uklotzde uklotzde changed the title [WiP] Disable track metadata export for .ogg files and TagLib 1.11.1 Disable track metadata export for .ogg files and TagLib 1.11.1 Jun 19, 2019
@uklotzde uklotzde changed the title Disable track metadata export for .ogg files and TagLib 1.11.1 [WiP] Disable track metadata export for .ogg files and TagLib 1.11.1 Jun 19, 2019
@uklotzde
Copy link
Contributor Author

This compile-time check is still insufficient and the wrong strategy. What we actually need is a runtime check, because on most platforms TagLib is linked dynamically. I'm not aware of any function that accomplishes that.

I'm really concerned about the way such a critical bug in one of our basic dependencies is handled. Unfortunately there is currently no alternative for TagLib.

@uklotzde uklotzde changed the title [WiP] Disable track metadata export for .ogg files and TagLib 1.11.1 Disable track metadata export for .ogg files and TagLib 1.11.1 Jun 19, 2019
@daschuer
Copy link
Member

This compile-time check is still insufficient and the wrong strategy. What we actually need is a runtime check, because on most platforms TagLib is linked dynamically. I'm not aware of any function that accomplishes that.

What is the scenario, such a check is required? On Linux we can rely on the package manager.
On the other systems we install the right version ourselves. If one messes around with the libraries, it is his own responds to set up a working version or not.

@daschuer
Copy link
Member

@Pegasus-RPG merge?
And release 2.2.2 short after?

@uklotzde
Copy link
Contributor Author

uklotzde commented Jul 6, 2019

Let's just rely on the static build time check. It should be sufficient for most use cases.

Anything else to do? We should release this asap to prevent further damage.

@daschuer daschuer merged commit aa27a36 into mixxxdj:2.2 Jul 7, 2019
@daschuer
Copy link
Member

daschuer commented Jul 7, 2019

OK

@uklotzde
Copy link
Contributor Author

uklotzde commented Jul 7, 2019

I'll resolve the conflicts with master, caused by reformatting.

@uklotzde uklotzde deleted the lp1833190_taglib_corrupt_ogg_files branch July 21, 2019 11:09
Be-ing added a commit to Be-ing/org.mixxx.Mixxx that referenced this pull request Aug 15, 2021
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.

3 participants