Skip to content
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

Add support for writing ID3v2.3 tags #85

Closed
lazka opened this issue Jul 4, 2014 · 18 comments
Closed

Add support for writing ID3v2.3 tags #85

lazka opened this issue Jul 4, 2014 · 18 comments
Labels

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

Originally reported by: Christoph Reiter (Bitbucket: lazka, GitHub: lazka)


From lalinsky on March 29, 2011 16:43:44

Mutagen always supported only saving of ID3v2.4 tags. When I considered using it in Picard, I implemented ID3v2.3 support by subclassing the ID3 class. I'd like to get this code included in Mutagen, so that we do not have to watch Mutagen for changes that could possibly make it incompatible. The code has been in use for a couple of years now (and since the majority of Picard uses are on Windows, they basically have to use ID3v2.3), so I'm pretty confident it's safe.

Attachment: id3v23.diff

Original issue: http://code.google.com/p/mutagen/issues/detail?id=85


@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From mur...@gmail.com on March 30, 2011 07:02:19

I don't have time for a full review right now, but it looks like calling save with v2=8 would write an ID3v2.8 tag rather than throwing an exception. While one could call this GIGO, I don't like allowing mutagen to write garbage.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From lalinsky on March 30, 2011 07:05:04

Sure, the code was used only internally, so I wasn't adding checks like that. I'll go through it and add them where necessary.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From e...@anentropic.com on May 18, 2011 07:56:37

@lalinsky
thanks! this seems exactly what I was looking for when I posted issue #82 anyone tagging files for Windows users needs to be able to output v2.3
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From lalinsky on March 30, 2011 07:13:16

To make it easier to review the code, here is the high-level overview of the changes:

 - 32bit non-sync-safe frame sizes for v2.3 tags (the only "structural" change)
 - only allow ISO-8859-1 or UTF-16 encoding in textual frames
 - don't allow multiple values in textual frames (join them by "/")
 - convert v2.4 frames TMCL and TIPL to v2.3 frame TIPL simply by taking all values from both frames
 - take the year part from v2.4 TDOR and save as TORY in v2.3
 - convert v2.4 TDRC by splitting it into frames TYER (year), TDAT (month+day), TIME (hour+minute) in v2.3
 - drop unsupported frames

I wasn't sure about enforcing the update_to_v23() call, but if you don't want to allow Mutagen to write garbage, then I guess save() should call it automatically.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From zco...@rhemasound.org on February 15, 2012 09:40:03

I'm considering using Mutagen in a personal project, but lack of support for v2.3 tags is indeed a deal breaker for me.  I liked the ability to tag multiple audio formats, but to support Windows users and DAPs I need to use 2.3
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From schla...@gmail.com on November 15, 2011 21:17:38

Are there any news to this issue. Are there plans to include the path into the trunk and any of the next releases.

There are still new media players which only supports ID3 v2.3 (e.g: Sansa Fuze+)
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From adr...@planetcoding.net on June 02, 2012 08:00:48

*bump* I'm sure the lack of v2.3 writing is not only a huge dealbreaker for me.
EVERYONE who wants to create tags that "just work everywhere" MUST write v2.3 as there are still tons of applications that do not support v2.4.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From sebastia...@gmail.com on March 25, 2013 05:48:33

@lalinsky: thx for the patch!

Right now I´m developing a piece of software for windows and would love to see native 2.3 support in mutagen.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From spamisg...@gmail.com on December 05, 2012 13:54:35

Windows Media Player, Windows Media Center and Windows Explorer in Windows 8 still don't support v2.4.... Would love to have the option to write tags in v2.3
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From reiter.christoph@gmail.com on May 29, 2013 09:38:51

Issue 153 has been merged into this issue.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From tush...@gmail.com on July 12, 2013 11:29:23

I wrote a programs using mutagen and realized for some reason Windows Media Player and my car music system are unable to read tags. Finally after debugging a lot I found that mutagen has written all tags in v2.4 while my Car music system, my home theater and Windows Media player 12 on Windows 7 can only read v2.3 tags. I would like to vote v2.3 support.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From reiter.christoph@gmail.com on May 29, 2013 10:23:56

Issue 153 has been merged into this issue.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From mats.pet...@gmail.com on July 12, 2013 15:29:09

Indeed.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From reiter.christoph@gmail.com on September 05, 2013 05:05:41

Based on the patch above with a slightly different approach:

load(self, filename, known_frames=None, translate=True, v2_version=4)
save(self, filename=None, v1=1, v2_version=4, v23_sep='/')
update_to_v23(self)

load:

 - calls update_to_v23 if v2_version == 3 and translate == True

save:

 - v2 renamed to v2_version so v1 isn't confused with a v1 version
 - instead of merging multivalues and changing encoding in update_to_v23, it gets done behind the scenes on save without changing the frames. I think that's better since it's harder to write off-spec tags this way and update_to_v23 can be called on load without losing multi values. It also allows to write v2.4 style null separators by passing v23_sep=None, which afaik isn't against the spec but might lead to loss of all values except the first (mutagen can still read them).

update_to_v23:

 - I went for removing all v2.4 frames by default. In the case of picard which keeps some of them around it has to load with translate=False, get the frames, update_to_v23, add the frames back. The rest is based on Lukas's patch.

Feedback welcome.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From reiter.christoph@gmail.com on September 05, 2013 04:53:51

This issue was closed by revision 5bb62316dd11 .

Status: Fixed

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From lalinsky on October 07, 2013 01:18:08

Hm, I'm probably looking at this the wrong way, or maybe I already forgot what the patch and Picard does, but I can't find the difference in update_to_v23.

I can see it only removing the new ID3v2.4 frames (except that you also have in the list those you have migrated in the code above).

What are the frames that we will need to manually add, if I was to port Picard to this?
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From adrian.sampson on September 05, 2013 11:02:34

Very nice, Christoph! (I saw your message on IRC belatedly and wanted to respond here.) This looks perfect. The way I imagine this working in beets is via configuration flag that enables ID3v2.3, in which case update_to_v23 is called before every save(). No need to have this on by default.

I was actually content to wait for the rest of the world to catch up to ID3v2.4, but this will be even nicer.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From reiter.christoph@gmail.com on October 07, 2013 03:01:29

In case of picard:

 - load(translate=False)
 - get v24 frames you want to write with v23 (TSOP, TSOA, TSOT..)
 - update_to_23()
 - add v24 frames back
 - save(v2_version=3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.