From 31898111ed89bfe079ccc5db16e55b50f0518d05 Mon Sep 17 00:00:00 2001
From: Christoph Reiter <reiter.christoph@gmail.com>
Date: Fri, 18 Nov 2016 01:07:02 +0100
Subject: [PATCH 1/2] mediafile: prefer latin-1 encoding for ID3 APIC
 descriptions. Fixes #899

iTunes has problems with everything but latin-1
Try to use latin-1 if possible and fall back to utf-16.
---
 beets/mediafile.py          | 11 +++++++++-
 test/test_mediafile_edge.py | 40 +++++++++++++++++--------------------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/beets/mediafile.py b/beets/mediafile.py
index 87a9d10a6..1ee778406 100644
--- a/beets/mediafile.py
+++ b/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 = 1  # utf-16
+        else:
+            frame.encoding = 0  # latin-1 if possible
+
         frame.type = image.type_index
         return frame
 
diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index 0be177699..4e61b5336 100644
--- a/test/test_mediafile_edge.py
+++ b/test/test_mediafile_edge.py
@@ -375,30 +375,26 @@ 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"": 0, u"foo": 0, u"\u0185": 1})
+            finally:
+                self._delete_test()
 
 
 def suite():

From 31f91129f318404dd905d52ef8e1b04e539b66c5 Mon Sep 17 00:00:00 2001
From: Christoph Reiter <reiter.christoph@gmail.com>
Date: Sat, 19 Nov 2016 10:36:31 +0100
Subject: [PATCH 2/2] Use mutagen id3 encoding constants

---
 beets/mediafile.py          | 4 ++--
 test/test_mediafile_edge.py | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/beets/mediafile.py b/beets/mediafile.py
index 1ee778406..7d1b07280 100644
--- a/beets/mediafile.py
+++ b/beets/mediafile.py
@@ -926,9 +926,9 @@ class MP3ImageStorageStyle(ListStorageStyle, MP3StorageStyle):
         try:
             frame.desc.encode("latin-1")
         except UnicodeEncodeError:
-            frame.encoding = 1  # utf-16
+            frame.encoding = mutagen.id3.Encoding.UTF16
         else:
-            frame.encoding = 0  # latin-1 if possible
+            frame.encoding = mutagen.id3.Encoding.LATIN1
 
         frame.type = image.type_index
         return frame
diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index 4e61b5336..ae758f142 100644
--- a/test/test_mediafile_edge.py
+++ b/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
@@ -392,7 +393,11 @@ class ID3v23Test(unittest.TestCase, TestHelper):
                 mf.save()
                 apic_frames = mf.mgfile.tags.getall('APIC')
                 encodings = dict([(f.desc, f.encoding) for f in apic_frames])
-                self.assertEqual(encodings, {u"": 0, u"foo": 0, u"\u0185": 1})
+                self.assertEqual(encodings, {
+                    u"": mutagen.id3.Encoding.LATIN1,
+                    u"foo": mutagen.id3.Encoding.LATIN1,
+                    u"\u0185": mutagen.id3.Encoding.UTF16,
+                })
             finally:
                 self._delete_test()