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

Auto writing to media files upon exit #12587

Closed
nvdl opened this issue Jan 17, 2024 · 14 comments
Closed

Auto writing to media files upon exit #12587

nvdl opened this issue Jan 17, 2024 · 14 comments

Comments

@nvdl
Copy link

nvdl commented Jan 17, 2024

Bug Description

Mixxx 2.4.0-beta
Git Version: 2.4-beta-347-gc323d0cd54
Platform: Linux x86_64
OS: Debian 11

Preferences -> Library -> "Synchronize library track metadata from/to file tags" is checked (enabled).

I keep my music files checksum-ed for validity of integrity.

Play a media file and then close the application.

The checksum of the file is different.
Some sort of write to the file occurred.
The application should not write to a file if the user has not explicitly updated the metadata.

I do not think, this issue exists on "Git Version: 2.4-beta-314-g71dceb27ba" as I would have got a lot of failing checksums.
I tried "Git Version: 2.4-beta-347-gc323d0cd54" today.

Version

2.4

OS

Debian 11

@nvdl nvdl added the bug label Jan 17, 2024
@daschuer
Copy link
Member

My guess is that the analyzer has updated theetadate.
This will affect the bpm and the replay gain value.

Do you have a backup of the old file to confirm?

@nvdl
Copy link
Author

nvdl commented Jan 17, 2024

The file format, for the file below, is FLAC but I have seen it updating MP3 files as well.

Original metadata:
orig

After playing and closing the application:
new

So BPM. key and replay gain information have been added.
The genre has also been "auto corrected".

I do not know at which step the metadata is written to the file.
If I revert the file to the original version from the backup and start the application, the BPM information is still there.
Presumably that is coming from the database instead of the metadata.

It is after playing that the metadata gets updated (after closing the application).

I assumed that this information was going to the database and not to the files directly.
So why keep the information in two places (database and tags)?

This will wreak havoc on my checksum-ed files.
Is there any option to disable it?

I still feel, this is a new feature that did not exist in the previous version mentioned above.

@uklotzde
Copy link
Contributor

Support for multi-valued tags is limited: #9087

If the current mapping doesn't work for you then you should disable this opt-in option. Why did you enable it if you expect that your files remain untouched?

@uklotzde
Copy link
Contributor

This is not a bug. It works as designed with the known limitations that might not be suitable for everyone.

@nvdl
Copy link
Author

nvdl commented Jan 17, 2024

Why did you enable it if you expect that your files remain untouched?

My understanding of the option was that the metadata is written to the file when I manually modify it via "Properties" menu. I did not expect it to write metadata to a file automatically.

Are you sure it has been like this in "Git Version: 2.4-beta-314-g71dceb27ba" as well?
Or has the scope of metadata been expanded to add BPM etc. information to the file as well?
It is updating metadata on the fly now (not in the analyze phase but play phase).

Even if I disable the option, can I control which fields go to the file when I enable it to do a timely sync?

@uklotzde
Copy link
Contributor

uklotzde commented Jan 17, 2024

The behavior has not changed recently. It's either all or nothing. Always was.

@daschuer
Copy link
Member

The join of the multiple Genres looks like a bug though.
It should not be written, if the genre read from the track matches the genre it will write.
We have a similar issue with the key information.
#11095
#11096

@uklotzde
Copy link
Contributor

The join of the multiple Genres looks like a bug though. It should not be written, if the genre read from the track matches the genre it will write. We have a similar issue with the key information. #11095 #11096

Read with TagLib::Tag::genre(), written by TagLib::Tag::setGenre(), and not touched by Mixxx.

@daschuer
Copy link
Member

It IS touched by Mixxx, that is the issue. The lossy roundtrip happens here:

pTag->setGenre(toTString(trackMetadata.getTrackInfo().getGenre()));

It can be solved by reading the tag before and skip the call in case the genre has not changed.

@uklotzde
Copy link
Contributor

It IS touched by Mixxx, that is the issue.

Incorrect. TagLib internally joins multi-valued fields which causes these issues:

https://github.com/taglib/taglib/blob/0318201fbd2d79319a0b74ddfefa2c9f7ef139d7/taglib/ogg/xiphcomment.cpp#L104

The behavior is not documented and also affects other fields.

@mxmilkiib
Copy link
Contributor

I spotted taglib/taglib#988

@daschuer
Copy link
Member

It does not matter at which point the merging happens. Mixxx does the calls leading to the loss of user data and the proposed solution will fix the issue.

@daschuer
Copy link
Member

daschuer commented Jan 19, 2024

Interesting: Taglib v2.0beta includes a separator " / ":
https://github.com/taglib/taglib/blob/0318201fbd2d79319a0b74ddfefa2c9f7ef139d7/taglib/tag.cpp#L195

@daschuer daschuer added this to the 2.4.1 milestone Jan 19, 2024
@uklotzde
Copy link
Contributor

It does not matter at which point the merging happens. Mixxx does the calls leading to the loss of user data and the proposed solution will fix the issue.

Sure. Adding a workaround in Mixxx would be a pragmatic solution. I didn't question that.

But the non-bidirectional mapping that causes these issues is still on TagLib's side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants