Repository: beetbox/beets

Issue:
MediaFile: use older text encodings in ID3v2.3 mode
I am trying to create an auto-tagging configuration in which my tags are saved in ID3v2.3 (as 2.4 lacks compatibility with some players I use) and I like the cover art to be embedded in each music file. However, the cover art of the output files is not recognised by OS X 10.9.4 (i.e. in Finder) or iTunes.

Here is a simple configuration with which the problem occurs:

``` yaml
directory: /mnt/data/home/Music

plugins: mbsync fetchart embedart
per_disc_numbering: yes
id3v23: yes

import:
    copy: yes
    write: yes

paths:
    default: $albumartist/$album%aunique{}/$disc-$track $title
```

When I comment the `id3v23: yes` option the covers of the output files are correctly recognised in Mac.

The cover art of the ID3v2.3 output files is recognised in Windows, so it seems a Mac-specific issue. Strangely enough, the input files I used already have ID3v2.3 tags and embedded cover art and are correctly recognised in Mac. Below you have a diff of the ID3v2.3 tags (sorted by name) between an input and an output file taken with `mid3v2`:

``` diff
1c1,2
< APIC= (image/jpeg, 32205 bytes)

---
> APIC= (image/jpeg, 111083 bytes)
> COMM=iTunNORM='eng'= 00001700 00001700 00003981 00003981 00000000 00000000 00008187 00008187 00000000 00000000
3a5,6
> TBPM=0
> TCMP=0
5a9
> TLAN=eng
12a17,19
> TXXX=Album Artist Credit=The Beatles
> TXXX=ALBUMARTISTSORT=Beatles, The
> TXXX=Artist Credit=The Beatles
14a22
> TXXX=MusicBrainz Album Comment=UK mono
17c25
< TXXX=MusicBrainz Album Status=official

---
> TXXX=MusicBrainz Album Status=Official
27a36
> USLT=[unrepresentable data]
```


Discussion:

Patch:
diff --git a/beets/mediafile.py b/beets/mediafile.py
index 87a9d10a6..7d1b07280 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 = mutagen.id3.Encoding.UTF16
+        else:
+            frame.encoding = mutagen.id3.Encoding.LATIN1
+
         frame.type = image.type_index
         return frame
 


Test patch:
diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index 0be177699..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
@@ -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():