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
Quickfix for upstream Mutagen compatibility #478
Conversation
There have been some changes in Mutagen upstream which makes it incompatible with Picard. For reference, SHAs of commits in Mutagen upstream (https://github.com/quodlibet/mutagen) that currently break Picard: 1. 2cd8cfdaec88bf896afd46cd03b209d688d3740a 2. e6e3fbe34f5946c92be8fb9266e9d27b46d8a700 This patch takes care of things for now, but is not a permanent solution. Also as Mutagen has dropped support for Python 2.6, but Picard is continuing it, there may be more conflicts in future.
A rather large change as both can include nested frames and we need to pass everything down to the specs and split out parsing/saving from the main ID3 class. Adds three new id3 specs: CTOCFlagsSpec for the flags value, also comes with a new flags type providing a nice repr. ID3FramesSpec for a list of id3 frames. And Latin1TextListSpec for the chapter id list. ID3 gets split up into ID3Tags and ID3, where the former only includes the non-IO parts and can be reused for CHAP/CTOC.sub_frames. update_do_v24() will recursively update sub frames and each ID3Tags instance will hold a unknown_frames attr which will be written back as is (given the version matches).
@@ -40,23 +40,23 @@ | |||
# Ugly, but... I need to save the text in ISO-8859-1 even if it contains | |||
# unsupported characters and this better than encoding, decoding and | |||
# again encoding. | |||
def patched_EncodedTextSpec_write(self, frame, value): |
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.
This would break Picard for everyone using mutagen < 1.34, wouldn't it?
It is not clear to me that this issues that this PR fixes are conflicts between Python versions (though I accept that if Picard is using 2.6 and Mutagen no longer supports it then issues could occur in future). Have the mutagen commits been tested with Picard on e.g. Python 2.7 to see? That said, Python 2.7 is now pretty mature, so I can see no real reason why Picard shouldn't choose 2.7+ as its version requirement. Personally I am guessing that these are more likely to be backward compatibility issues introduced by these mutagen commits - and that maintaining backwards compatibility is a mutagen responsibility (hence my comments against the mutagen commits). It seems to me that if new mutagen functionality can only be provided by breaking backwards compatibility, then mutagen should create a new major version for the new functionality and continue to support the current major version with fixes for some time.) |
I needed to rework the internal API to get the chapter extension working. Re Python2.6: I'm happy to continue supporting it (afaik the latest release is compatible, I've just disabled testing on travis). I've asked on the mailinglist some weeks ago and got no response: https://groups.google.com/forum/#!topic/quod-libet-development/1ErExqiDNqY I probably should have sent an extra mail. |
I can look into removing all this monkey patching if wanted. edit: I'm on it.. |
Opened #479 feedback welcome |
I'm closing this PR as it won't be required after #479. Follow that for related discussion. |
There have been some changes in Mutagen upstream which makes it
incompatible with Picard. For reference, commits in Mutagen
upstream that currently break Picard:
This patch takes care of things for now, but is not a permanent
solution. Also as Mutagen has dropped support for Python 2.6, but Picard
is continuing it, there may be more conflicts in future.