Merge pull request #2270 from lazka/apic-encoding-compat

mediafile: prefer latin-1 encoding for ID3 APIC descriptions. Fixes #899

beets/mediafile.py

@@ -920,7 +920,16 @@ class MP3ImageStorageStyle(ListStorageStyle, MP3StorageStyle):
         frame.data = image.data
         frame.mime = image.mime_type
         frame.desc = image.desc or u''
-        frame.encoding = 3  # UTF-8 encoding of desc
+
+        # For compatibility with OS X/iTunes prefer latin-1 if possible.
+        # See issue #899
+        try:
+            frame.desc.encode("latin-1")
+        except UnicodeEncodeError:
+            frame.encoding = mutagen.id3.Encoding.UTF16
+        else:
+            frame.encoding = mutagen.id3.Encoding.LATIN1
+
         frame.type = image.type_index
         return frame
 

test/test_mediafile_edge.py

@@ -19,6 +19,7 @@ from __future__ import division, absolute_import, print_function
 
 import os
 import shutil
+import mutagen.id3
 
 from test import _common
 from test._common import unittest
@@ -375,30 +376,30 @@ class ID3v23Test(unittest.TestCase, TestHelper):
         finally:
             self._delete_test()
 
-    def test_v24_image_encoding(self):
-        mf = self._make_test(id3v23=False)
-        try:
-            mf.images = [beets.mediafile.Image(b'test data')]
-            mf.save()
-            frame = mf.mgfile.tags.getall('APIC')[0]
-            self.assertEqual(frame.encoding, 3)
-        finally:
-            self._delete_test()
+    def test_image_encoding(self):
+        """For compatibility with OS X/iTunes.
 
-    @unittest.skip("a bug, see #899")
-    def test_v23_image_encoding(self):
-        """For compatibility with OS X/iTunes (and strict adherence to
-        the standard), ID3v2.3 tags need to use an inferior text
-        encoding: UTF-8 is not supported.
+        See https://github.com/beetbox/beets/issues/899#issuecomment-62437773
         """
-        mf = self._make_test(id3v23=True)
-        try:
-            mf.images = [beets.mediafile.Image(b'test data')]
-            mf.save()
-            frame = mf.mgfile.tags.getall('APIC')[0]
-            self.assertEqual(frame.encoding, 1)
-        finally:
-            self._delete_test()
+
+        for v23 in [True, False]:
+            mf = self._make_test(id3v23=v23)
+            try:
+                mf.images = [
+                    beets.mediafile.Image(b'data', desc=u""),
+                    beets.mediafile.Image(b'data', desc=u"foo"),
+                    beets.mediafile.Image(b'data', desc=u"\u0185"),
+                ]
+                mf.save()
+                apic_frames = mf.mgfile.tags.getall('APIC')
+                encodings = dict([(f.desc, f.encoding) for f in apic_frames])
+                self.assertEqual(encodings, {
+                    u"": mutagen.id3.Encoding.LATIN1,
+                    u"foo": mutagen.id3.Encoding.LATIN1,
+                    u"\u0185": mutagen.id3.Encoding.UTF16,
+                })
+            finally:
+                self._delete_test()
 
 
 def suite():