library: add subtitle (id3 TIT3) field#5696
library: add subtitle (id3 TIT3) field#5696daniele-athome wants to merge 1 commit intobeetbox:masterfrom
Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
|
The musicbrainz plugin should also be updated to pull the field and populate it as well. Did this addition stall? Seems increasingly commonly used with Picard and now Navidrome’s metadata enhancements. |
I don't know... there are frequent commits on the main branch but some PRs are ignored. There must surely be a reason but the core devs would have to share that. Or they simply forgot about this :-) |
snejus
left a comment
There was a problem hiding this comment.
Added a comment. I reckon this change should be included in the changelog too
|
I added a changelog entry. |
|
Aw snap just realised this depends on an unmerged PR in mediafile repo :/ |
This PR adds support for a subtitle field for MP3 (ID3v2 "TIT3") and ASF. Reference for an MP4 tag missing. Required by beetbox/beets#5696
This PR adds support for a subtitle field for MP3 (ID3v2 "TIT3") and ASF. Reference for an MP4 tag missing. Required by beetbox/beets#5696
This PR adds support for a subtitle field for MP3 (ID3v2 "TIT3") and ASF. Reference for an MP4 tag missing. Required by beetbox/beets#5696
|
I just merged beetbox/mediafile#82 🙃. You up to rebase and revive this? |
493c0ae to
5152266
Compare
|
Rebased!! 🚀 |
|
Awesome! Let me see if I'm able to create a new mediafile version and how to do that 😨 |
|
I don't think I'm able to do a version bump for mediafile. I raised the issue in our internal chat tho. We will have to wait some more, sorry 🙈 |
|
Hi there! Any luck in releasing a new version of mediafile? We are like a meter from the finish line! 😄 I'm ready to rebase this whenever needed. |
|
We should be able to do that in the near future 🎉 We sill need to create a (maintainable) release workflow but other than that we should be able to create a release. |
|
@daniele-athome new $ uv tool run 'poetry<2' update mediafile |
|
On it! |
0e58122 to
50f6600
Compare
|
It took some time - sorry, life got in the way - but I'm finally there. Rebased on latest master and mediafile upgraded. |
docs/changelog.rst
Outdated
There was a problem hiding this comment.
I actually wanted to ask about the following:
- Add support for importing and writing
It doesn't seem like this PR adds support for importing this field? Currently, this field is ignored - it can only be populated manually using beet modify ... subtitle=....
What is your use case for it?
There was a problem hiding this comment.
It doesn't work when importing metadata from an external source (e.g. MusicBrainz), but it worked for importing the field from source files (e.g. an existing TIT3 field in an mp3 file).
My specific use case was to edit the field after import (I wanted to remove it).
There was a problem hiding this comment.
Have you considered removing this field from your music files before importing them?
For mp3 files you could use id3v2 --remove-frame TIT3 <files> and for flacs you may use metaflac --remove-tag SUBTITLE
There was a problem hiding this comment.
I did consider it, but since I use beets for anything related to my music collection, I thought it would be nice to implement it directly in beets and contribute it.
I know this is not 100% complete (because there is no way to actually import the field from sources, e.g. MusicBrainz), but it can pave the way for the next person to implement that (or I might even implement it myself if I find the time).
There was a problem hiding this comment.
I share that concern. Adding a field to the core database just to remove it feels wrong architecturally.
The scrub plugin is designed exactly for this—stripping unwanted metadata before writing. But it has a limitation: it removes all unknown tags, with no way to selectively target specific ones.
Your actual need is granular control: remove TIT3 without touching other tags. Neither scrub nor zero supports that today.[4][5]
Rather than adding subtitle to the schema, consider extending scrub with a blacklist option to remove specific frames by name. Something like:[3]
scrub:
auto: yes
remove_frames: [TIT3, COMM, USLT]This solves the problem cleanly for anyone managing legacy metadata, not just subtitle fields. It's also better than shelling out to id3v2 or metaflac externally.
What do you think?
There was a problem hiding this comment.
Rather than adding subtitle to the schema, consider extending scrub with a blacklist option to remove specific frames by name.
It's a nice idea and I might do that as well, another time.
I share that concern. Adding a field to the core database just to remove it feels wrong architecturally.
After your explanation about scrub, I totally agree. Does that mean that you are considering not adding support for the field in beets afterall? Despite its intended use by me, the new field can be used for importing (given someone - or myself - implement it in the MusicBrainz plugin for example).
There was a problem hiding this comment.
(I was about to rebase again but at this point I think I'll wait for an answer before doing it)
50f6600 to
723e94c
Compare
723e94c to
3f38b55
Compare
Description
Adds support for a subtitle field, corresponding to the
TIT3id3 tag. Requires beetbox/mediafile#82I don't think tests are required for this - I can't really see a comprehensive test suite for all available fields - but I might be wrong.
I'll add the changelog entry after your review.
To Do
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)