Fix #879 (again): update ID3 string encodings

beets/mediafile.py

@@ -38,6 +38,7 @@ from __future__ import (division, absolute_import, print_function,
 
 import mutagen
 import mutagen.mp3
+import mutagen.id3
 import mutagen.oggopus
 import mutagen.oggvorbis
 import mutagen.mp4
@@ -747,19 +748,20 @@ class MP3DescStorageStyle(MP3StorageStyle):
         if self.key != 'USLT':
             value = [value]
 
-        # try modifying in place
+        # Try modifying in place.
         found = False
         for frame in frames:
             if frame.desc.lower() == self.description.lower():
                 frame.text = value
+                frame.encoding = mutagen.id3.Encoding.UTF8
                 found = True
 
-        # need to make a new frame?
+        # Try creating a new frame.
         if not found:
             frame = mutagen.id3.Frames[self.key](
                 desc=bytes(self.description),
                 text=value,
-                encoding=3
+                encoding=mutagen.id3.Encoding.UTF8,
             )
             if self.id3_lang:
                 frame.lang = self.id3_lang

docs/changelog.rst

@@ -71,6 +71,8 @@ Fixes:
 * Fix a crash when sorting by nonexistent fields on queries. :bug:`1734`
 * Probably fix some mysterious errors when dealing with images using
   ImageMagick on Windows. :bug:`1721`
+* Fix a crash when writing some Unicode comment strings to MP3s that used
+  older encodings. The encoding is now always updated to UTF-8. :bug:`879`
 
 .. _Emby Server: http://emby.media
 

test/test_mediafile_edge.py

@@ -199,7 +199,7 @@ class SideEffectsTest(unittest.TestCase):
         self.assertEqual(old_mtime, new_mtime)
 
 
-class EncodingTest(unittest.TestCase, TestHelper):
+class MP4EncodingTest(unittest.TestCase, TestHelper):
     def setUp(self):
         self.create_temp_dir()
         src = os.path.join(_common.RSRC, 'full.m4a')
@@ -218,6 +218,26 @@ class EncodingTest(unittest.TestCase, TestHelper):
         self.assertEqual(new_mf.label, u'foo\xe8bar')
 
 
+class MP3EncodingTest(unittest.TestCase, TestHelper):
+    def setUp(self):
+        self.create_temp_dir()
+        src = os.path.join(_common.RSRC, 'full.mp3')
+        self.path = os.path.join(self.temp_dir, 'test.mp3')
+        shutil.copy(src, self.path)
+
+        self.mf = beets.mediafile.MediaFile(self.path)
+
+    def test_comment_with_latin1_encoding(self):
+        # Set up the test file with a Latin1-encoded COMM frame. The encoding
+        # indices defined by MP3 are listed here:
+        # http://id3.org/id3v2.4.0-structure
+        self.mf.mgfile['COMM::eng'].encoding = 0
+
+        # Try to store non-Latin1 text.
+        self.mf.comments = u'\u2028'
+        self.mf.save()
+
+
 class ZeroLengthMediaFile(beets.mediafile.MediaFile):
     @property
     def length(self):