-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
MP4: Unexpected upgrade of gnre
to ©gen
#409
Comments
Are you saying after parsing a tag you end up with both I'm thinking about adding I'd rather make conversions a toggle than remove them outright, since they make it possible to get files on the same page simplifying the implementation of |
No. But you can't tell the Fortunately, I didn't write any tags back with lofty yet and so I could fix the original files first by removing the legacy |
The ambiguities only occur for files with both |
I'm not sure what you mean, the atoms contain no additional metadata to separate them from one another. You can iterate all values like this: let genre_atom = ilst.get(&AtomIdent::Fourcc(*b"\xa9gen"));
for data in genre_atom.data() {
if let AtomData::UTF8(genre) = data {
println!("Genre: {genre}");
}
} Unless I'm misunderstanding what you mean?
With ilst, it is expected that atoms be merged. I took a look at TagLib, mutagen, and music-metadata and they all do the same thing with the None of them let you toggle that though, so I'll definitely look into making that an option. It'll just have to be made very clear that it will break |
I am referring to this situation: Before
After
The value from the legacy atom should be ignored if the official genre atom is present. Otherwise you end up with multiple genres and could no longer decide which one of them is the "real" one. The legacy atom was probably just a left over that has not been removed when adding I decided to strip the |
TagLib maps Also not optimal but at least unambiguous. |
Ah, yeah that's a confusing situation. We can't really signal which genres have been converted. I've already added |
Reproducer
parse_ilst()
Summary
This opinionated behavior not only prevents applications from detecting and fixing/resolving those issues themselves. It causes ambiguities and mixes up metadata which is much more concerning.
Afterwards the imported
Ilst
contains both regular©gen
atoms side by side with implicitly migratedgnre
atoms. They are indistinguishable and alter the metadata unexpectedly when written back to the file.The only situation when such an implicit migration would be acceptable is if the original
Ilst
does only containgnre
but no©gen
atoms. But conditional, data-dependent behavior is harder to reason about, so better not try to be clever.Expected behavior
This feature should either be made optional or provided as a manual, post-processing step on the imported
Ilst
. I would prefer the latter, because import and migration should be independent.Assets
No response
The text was updated successfully, but these errors were encountered: