arcnmx / MPD

MPD fork adding a youtube-dl plugin
https://github.com/MusicPlayerDaemon/MPD/pull/223
GNU General Public License v2.0
7 stars 2 forks source link

Metadata/tag mapping #14

Closed arcnmx closed 2 years ago

arcnmx commented 4 years ago

Need to think about whether to tone down certain metadata mappings and tag assignments. Some discussion in the original PR mentioned being more conservative about not assigning items that may not always be correct (think artist vs uploader for example):

I don't think filling as many metadata fields as possible is a good goal, not when the tags do not match the contents. Only fill those which match, and if you feel others are important, we can discuss later how to integrate them. For now, to get this merged, let's get to a minimal commit; one that is stable and robust.

While I kinda personally disagree, a few thoughts:

Rio6 commented 4 years ago

Was reading https://github.com/MusicPlayerDaemon/MPD/pull/223#issuecomment-366427063

I think using uploader and playlist title as fall back to artist and album is reasonable, as that's often the case on Youtube.

Not sure how long TAG_COMMENT supposed to be, but I know in many videos the description is very long. Video url is probably nicer as comment.

Also, what's about TAG_DATE's format that needs to be fixed?

arcnmx commented 4 years ago

I think using uploader and playlist title as fall back to artist and album is reasonable, as that's often the case on Youtube.

It seems pretty reasonable since you're unlikely to have anything else to go by, but I believe the comment just above where you linked said "Why assign TITLE to ALBUM? This sounds very wrong, doesn't it?" I can't find a comment mentioning uploader but I think there was a problem with that too?

Not sure how long TAG_COMMENT supposed to be, but I know in many videos the description is very long. Video url is probably nicer as comment.

Eh I've seen plenty of mp3s with like a page of comments included, dunno that it's all that uncommon! But again, the original review said "The COMMENT is an URL? Why that?" and "More strange COMMENT tags" and "comment==url?" so... I think my "Be conservative" note generally means to be conservative and literal about the assignments? At least by default, because I do personally want to tweak it quite a bit.

Also, what's about TAG_DATE's format that needs to be fixed?

From what I could find, it was only ever used to store a year (id3 has a TDAT tag for example, but mpd ignores it and only sets TAG_DATE with TYER). So I wasn't sure if YYYYMMDD (or whatever we're even currently filling it with? I hope that's it) is valid. I'm not sure whether any documentation exists on the format and meaning of mpd's tags and how they correlate with id3 or anything else. Do clients make any assumptions about their contents? Should they be able to?

Rio6 commented 4 years ago

Perhaps we could bring it up in the original PR again? I think what we have currently have is pretty good.

I think having the URL in one of the tags is important, since it can be used to get thumbnail. Is the video description important? I don't use a lot of other platforms so I'm not really sure how they use it.

For date, MPD's document says "This is usually a 4-digit year". We can always truncate it if we want to follow that.

Rio6 commented 4 years ago

nvm I can read url from file name tag

arcnmx commented 3 years ago

Another consideration here: though I like having the video description in the comment, libmpdclient will bail out if a protocol line exceeds its buffer size, which is fixed at 4KB apparently. Some videos have excessively long descriptions, and any mpd client using that library (so you know, basically all of them) will fail and error out trying to read the tags.

This also happens if you just add a file to your mpd library that was downloaded with youtube-dl --add-metadata https://youtube.etc so if anything I'd consider it an mpd bug, but is a lot more likely when using this plugin for obvious reasons.