Fix handling of albumartistsort as TSO2 and TXXX:ALBUMARTISTSORT in EasyID3#649
Open
antlarr wants to merge 3 commits intoquodlibet:mainfrom
Open
Fix handling of albumartistsort as TSO2 and TXXX:ALBUMARTISTSORT in EasyID3#649antlarr wants to merge 3 commits intoquodlibet:mainfrom
antlarr wants to merge 3 commits intoquodlibet:mainfrom
Conversation
albumartistsort was already added as a registered Text key some lines above so adding it again as a TXXX key replaces the previous TSO2 tag registration. Since picard uses TSO2 for "album artist sort order" and even if it's not an official tag it seems to be generally supported, I think it's better to leave that one.
This tests that albumartistsort is handled by the TSO2 frameid.
I noticed that my previous fix for duplicate key registration of albumartistsort would break reading files already having TXXX:ALBUMARTISTSORT so this commit fixes it by making TXXX:ALBUMARTISTSORT a fallback for albumartistsort if the first getter can't get a value and also by making EasyID3 delete TXXX:ALBUMARTISTSORT frames when deleting albumartistsort. Still, when setting an albumartistsort value only TSO2 frames are used. This also adds tests for all those cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
albumartistsort is duplicated as a Text key in easyid3.py#L492 and as a TXXX key in easyid3.py#L533 thus replacing the previous registration.
Because of that, before this change, loading a file with EasyID3 that has a
TSO2frame doesn't make it available by accessingalbumartistsort.Even if
TSO2is not an official tag it seems to be generally supported, and since picard writesTSO2frames for "album artist sort order", I think it's better to give priority toTSO2overTXXX:ALBUMARTISTSORT.I also added a test that fails with the current code and runs successfully after the fix is applied to make sure it doesn't break in the future again.
Actually, we can't just remove
TXXX:ALBUMARTISTSORTregistration, since that would break reading such frames in files already written as that. So I've added a third commit that falls back to getting theTXXX:ALBUMARTISTSORTvalue ifTSO2isn't found, deletes bothTSO2andTXXX:ALBUMARTISTSORTframes when deletingalbumartistsortand added more tests for those cases.I'm aware this third commit makes the code a bit more complex, but I've tried to make it as generic as possible just in case other fallback frames need to be added in the future for some other key.