Closed JuniorJPDJ closed 3 years ago
FWIW the tests seem to need some attention (according to CI results).
I was looking at those (already fixed style check) and it didn't look like it failed on my changes. I need someone to clarify this then and maybe suggest some changes.
EDIT: It fails on my changes: just rolled back this commit in my tree and it worked before, but I don't really know which change made it fail.
EDIT2: I managed to fix it. Commits will come tomorrow. :D
albumartists
is suggested here: https://kodi.wiki/view/Music_tagging#albumartists and I'd like to make it default in Picard too
EDIT: Picard is probably gonna have it! https://community.metabrainz.org/t/multiple-album-artists/532302
I think it's ready ;D
I rebased changes on top of latest commit.
Thanks for continuing to push this forward! I have to admit that I'm a tiny bit lost in navigating all the changes at this point, so I thought I'd try to sum things up:
mb_{album}artistid
fields have become list-valued (depending on the above new feature).And of course the tests are updated as well, but that's neither here nor there. Is that an accurate summary?
With apologies for being a picky maintainer, I would love to consider all of these changes individually. It would be great, for example, to carefully consider whether we do actually want to change those ID fields to be lists—unfortunately, I can imagine this change breaking software that depends on their current list behavior. And when we add new fields, we should consider adding them to the documentation. So is there any chance we could break this up into individual steps?
Broadly speaking, adding the new fields should be fairly uncontroversial, although I do want to track down the source of truth (instead of just trusting the HydrogenAudio wiki alone). The IDs-as-lists change could be problematic, however, and needs some careful consideration.
Thanks for considering it!
Of course ;)
The mb_{album}artistid fields have become list-valued (depending on the above new feature).
Will moving this to another PR satisfy you? Other changes are pretty much depending on themselves so I don't feel like splitting them out is good idea, but if you have other opinion I could do this.
I moved multivalue musicbrainz to #46.
Is there any chance you could include in the thread here any links to documentation you have about where all these tags came from?
Of course! I'll do detailed writeup, just gimme an hour or two ;D
artists
: whole thing is defined in Picard tag mapping
copyright
: same as above
albumartists
: I used Kodi's scheme as it's non-standard at the moment and it seems reasonable
ATM adding this tag to Picard is being discussed here and here and I'd like to support all variants, that's why I created #47 (it needs more changes to code)
I decided it's better to support main variant first, then fix issues with multivariant and then support all variants
barcode
changes: I already shown my point of view in discussion above
isrc
: also based on Picard tag mapping
url
: there things are going a bit worse
Most of standards seem not to support URL mapping so I was basing on files tagged by Tag&Rename, some people from my environment were using, decoded by mutagen
WXXX
frame and it's defined in spec©url
and it's based on output of Tag&Rename
but I wasn't able to find much info about it in google; ffprobe
decoded it correctly so it seems to be some standard:
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'kdu4r.mp4':
Metadata:
major_brand : isom
minor_version : 512
compatible_brands: isomiso2avc1mp41
encoder : Lavf57.83.100
compilation : 0
URL : https://www.aliexpress.com/
media_type : 0
>>> dict(mutagen.File('kdu4r.mp4'))
{'©too': ['Lavf57.83.100'],
'cpil': False,
'©url': ['https://www.aliexpress.com/'],
'furl': ['https://basewin.pl/'],
'aurl': ['http://chomikuj.pl/'],
'surl': ['https://drive.google.com/drive/my-drive'],
'rurl': ['https://ekipatonosi.pl'],
'burl': ['facebook.com'],
'stik': [0]}
wma
file tagged by Tag&Rename:
>>> dict(mutagen.File('01. MAROON STRONG.wma'))
{'WM/AudioFileURL': [ASFUnicodeAttribute('https://www.instagram.com/')],
'Description': [ASFUnicodeAttribute('')],
'WM/EncodingSettings': [ASFUnicodeAttribute('Lavf58.49.100')],
'WM/RadioStationURL': [ASFUnicodeAttribute('https://www.netflix.com')],
'WM/URL': [ASFUnicodeAttribute('https://hostuje.net/')],
'WM/AuthorURL': [ASFUnicodeAttribute('https://jakdojade.pl')],
'ID3': [ASFByteArrayAttribute(b'ID3\x03\x00\x00\x00\x00\x00/WXXX\x00\x00\x00\x16\x00\x00\x00\x00https://hostuje.net/COMM\x00\x00\x00\x05\x00\x00\x00eng\x00')],
'WM/UserWebURL': [ASFUnicodeAttribute('https://hostuje.net/')],
'WM/MusicBuyUrl': [ASFUnicodeAttribute('https://odrabiamy.pl/')],
'WM/AudioSourceURL': [ASFUnicodeAttribute('https://www.messenger.com')]}
This is however not translated by ffprobe:
Input #0, asf, from '01. MAROON STRONG.wma':
Metadata:
encoder : Lavf58.49.100
WM/URL : https://hostuje.net/
WM/UserWebURL : https://hostuje.net/
WM/AudioFileURL : https://www.instagram.com/
WM/AuthorURL : https://jakdojade.pl
WM/AudioSourceURL: https://www.messenger.com
WM/RadioStationURL: https://www.netflix.com
WM/MusicBuyUrl : https://odrabiamy.pl/
Nor Windows details view show it. Windows however shows another URL tag. This is AuthorURL. But purpose of this tag is other than main URL.
I found some forum post that also mentioned this tag so I assumed It's widely supported.
URL
but I added also WWW
and Website
as I found actual uses of them in web. This also isn't standarized in any way. EasyTag seems to tag URL input box into CONTACT
field on flac files which seems ambiguous to me. Standard defines CONTACT
field as
Contact information for the creators or distributors of the track. This could be a URL, an email address, the physical address of the producing label.
Which is not what I wanted to achieve by url
tag, it's more like authorurl
.
I know people who use Vorbis comments a lot don't love having duplicate data in their free-form tag data
Well.. They won't be happy about albumartists
from #47 then :X
I can remove WWW
and Website
then, I'm OK with that. Just wanted to maximize compatibility ;)
I'm still struggling with barcode as some standards are not-cool and are defining separate fields for other barcode standards and I honestly would like to have them in one tag, which is understood by most of software. What do you think, should I remove it or leave it here?
Sounds great! For barcode
: just to clarify, are you saying that there's a trend among software out there to define lots of different fields for different barcode classification standards, such as UPC and EAN?
I can see how that argues in favor of supporting all these miscellaneous tags as read-only mappings and "BARCODE" itself as the only writable version! Googling around reveals threads like this one where people are equally frustrated by the proliferation of different tag names for different kinds of barcodes.
Anyway, this sounds perfectly reasonable to me. Happy to do the "read-only for weird names" thing here—until someone out there wants to record both a UPC and an EAN for the same file. 😂
are you saying that there's a trend among software out there to define lots of different fields for different barcode classification standards, such as UPC and EAN?
That's what I want to say. Yes! :D Examples are here: dBPowerAMP and some wiki for Vorbis tags.
I'll fix my changes in few minutes then and.. It's ready to merge! :D
Done! :D
until someone out there wants to record both a UPC and an EAN for the same file.
UPC itself can be converted to EAN by adding 0 at the front, so I think they will not try to kill me for this change even if it overwrite one of them ;)
Aaand UPN is wrong name for UPC in our case as UPN is medical-only:
Drugs by National Drug Code (NDC) number. Pharmaceuticals in the U.S. use the middle 10 digits of the UPC as their NDC number. Though usually only over-the-counter drugs are scanned at point of sale, NDC-based UPCs are used on prescription drug packages and surgical products and, in this case, are commonly called UPN Codes.
Source: Wikipedia
Musicbrainz for example allows only one barcode per release, names it barcode
and forces you to use EAN, as UPC can be represented as subset of EAN (they advise how to convert UPC -> EAN).
Also adds additional barcode tags to read from
Sources for tag mapping: https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#artists https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping https://kodi.wiki/view/Music_tagging#albumartists