Skip to content

Conversation

@UnknownSuperficialNight
Copy link
Contributor

@UnknownSuperficialNight UnknownSuperficialNight commented Nov 12, 2025

Add ReleaseCountry to ItemKey

I propose adding ReleaseCountry to ItemKey. Please review to ensure I've covered everything.

This change adds ItemKey to a few key/ItemKey maps.

I have a question regarding ID3V2_MAP. Should it be:

"TXXX:MusicBrainz Album Release Country" => ReleaseCountry 
OR
"TXXX" => ReleaseCountry

I assume the TXXX:MusicBrainz Album Release Country one is correct?

I'm also unsure whether to map MusicBrainz Album Release Country directly to ReleaseCountry or create a separate MusicBrainzReleaseCountry. My assumption is to map it to a single ReleaseCountry.

Let me know if I missed anything, other, than those two.

Note-to-self:
Todo: ILST_MAP and ID3V2_MAP

@Serial-ATA
Copy link
Owner

I have a question regarding ID3V2_MAP. Should it be:

For keys that map to TXXX frames, you just put the description in there, no TXXX. So in this case, "MusicBrainz Album Release Country" => ReleaseCountry.

Then you just stick the ItemKey::ReleaseCountry in this list:

// Multi-valued TXXX key-to-frame mappings
for item_key in [
ItemKey::TrackArtists,
ItemKey::Director,
ItemKey::CatalogNumber,
ItemKey::MusicBrainzArtistId,
ItemKey::MusicBrainzReleaseArtistId,
ItemKey::MusicBrainzWorkId,
] {

Note that only ID3v2 has that special requirement, just because those handful of items map to a frame other than what their name specifies.

I'm also unsure whether to map MusicBrainz Album Release Country directly to ReleaseCountry or create a separate MusicBrainzReleaseCountry. My assumption is to map it to a single ReleaseCountry.

Yeah, it's unfortunate that they stuck "MusicBrainz" in there, but at the same time I believe it's the only standardized field for release country. I'd be fine just mapping it to ReleaseCountry, cause at the end of the day it's just a standard ISO 3166-1 code, typically.

And if it ever needs to change down the line, it can. No big deal.

@Serial-ATA Serial-ATA added this to the 0.23.0 milestone Nov 12, 2025
@UnknownSuperficialNight
Copy link
Contributor Author

@Serial-ATA Alright have a look if that's correct

@Serial-ATA
Copy link
Owner

Looks good, thanks for keeping the ilst map aligned as well. :)

Rebased and updated the changelog.

@Serial-ATA Serial-ATA merged commit 55b024c into Serial-ATA:main Nov 12, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants