-
Notifications
You must be signed in to change notification settings - Fork 36
Added MP3DescStorageStyle for LABEL and MEDIA #95
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #95 +/- ##
=======================================
Coverage 93.26% 93.26%
=======================================
Files 16 16
Lines 817 817
Branches 118 118
=======================================
Hits 762 762
Misses 35 35
Partials 20 20 |
by foobar and Mp3Tag. For references see https://community.mp3tag.de/t/the-case-for-a-dedicated-label-tag/57647/17 and https://community.mp3tag.de/t/what-is-the-correct-tag-for-label/59445/2 closes #85
de20ac6 to
1bfc560
Compare
|
I dont know how to deal with it technically but TPUB is definitely the more important one. Did some (ai-assisted) digging around again, this time focusing on the usage in DJ software and all major ones use it for a field often called label in their UIs. rekordbox, mixxx, traktor, serato so RW support as we have it already should be kept (just saying, i'm sure that's your intention anyway) anyway if someone wants to use mediafile to tag this could be made possible of we invent a new field to access as really only thinking out loud here and need to dig deeper to find out if we don't have very similar situations already in mediafile. sorry not much help for now |
|
anyway answering your original question: In my opinion it is perfectly ok if beets writes the same info to both txxx and tpub. the enduser aka the tools that try to display a label will all be happy about it :-) |
|
and as a reminder where we are coming from, we would like imports to read the non standard txxx:label additionally (in case tpub is empty it will be used instead) that doesn't mean it hurts to also allow writing it (beets for example will then simply write both). it shouldn't be a readonly field in my opinion. hope that helps |
|
|
||
| label = MediaField( | ||
| MP3StorageStyle("TPUB"), | ||
| MP3DescStorageStyle("LABEL"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on read tpub is preferred, on write both are written, this is the correct order imho.
| ) | ||
| media = MediaField( | ||
| MP3StorageStyle("TMED"), | ||
| MP3DescStorageStyle("MEDIA"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
This PR adds the
TXXX:LABELandTXXX:MEDIAto the respective fields in the MediaFile class.These fields are read and written by foobar and Mp3Tag.
For references see
https://community.mp3tag.de/t/the-case-for-a-dedicated-label-tag/57647/17 and
https://community.mp3tag.de/t/what-is-the-correct-tag-for-label/59445/2
closes #85
Decision needed:
Im not entirely sure if we want to have both fields as read and write as this now duplicates the metadata if mediafile is used to write these fields:
This will now write both the
TXXX:LABELandTPUBframe (same for media).We could defined these fields as read_only, but this would than still make issues with tools relying on the
TXXXfields. @JOJ0 Do you know if there is a precedence for this and if we had such an issue beforehand?