Merge pull request #2255 from beetbox/fix-2254-1545

Rewrite handling of images extensions and mimetype

beets/art.py

@@ -22,7 +22,6 @@ from __future__ import division, absolute_import, print_function
 import subprocess
 import platform
 from tempfile import NamedTemporaryFile
-import imghdr
 import os
 
 from beets.util import displayable_path, syspath, bytestring_path
@@ -194,7 +193,7 @@ def extract(log, outpath, item):
         return
 
     # Add an extension to the filename.
-    ext = imghdr.what(None, h=art)
+    ext = mediafile.image_extension(art)
     if not ext:
         log.warning(u'Unknown image type in {0}.',
                     displayable_path(item.path))

beets/mediafile.py

@@ -59,6 +59,7 @@ import enum
 
 from beets import logging
 from beets.util import displayable_path, syspath, as_string
+from beets.util.collections import IdentityFallbackDict
 import six
 
 
@@ -81,6 +82,8 @@ TYPES = {
     'aiff': 'AIFF',
 }
 
+PREFERRED_IMAGE_EXTENSIONS = IdentityFallbackDict({'jpeg': 'jpg'})
+
 
 # Exceptions.
 
@@ -308,6 +311,17 @@ def _sc_encode(gain, peak):
 
 
 # Cover art and other images.
+def _imghdr_what_wrapper(data):
+    """A wrapper around imghdr.what to account for jpeg files that can only be
+    identified as such using their magic bytes
+    See #1545
+    See https://github.com/file/file/blob/master/magic/Magdir/jpeg#L12
+    """
+    # imghdr.what returns none for jpegs with only the magic bytes, so
+    # _wider_test_jpeg is run in that case. It still returns None if it didn't
+    # match such a jpeg file.
+    return imghdr.what(None, h=data) or _wider_test_jpeg(data)
+
 
 def _wider_test_jpeg(data):
     """Test for a jpeg file following the UNIX file implementation which
@@ -318,14 +332,14 @@ def _wider_test_jpeg(data):
         return 'jpeg'
 
 
-def _image_mime_type(data):
+def image_mime_type(data):
     """Return the MIME type of the image data (a bytestring).
     """
     # This checks for a jpeg file with only the magic bytes (unrecognized by
     # imghdr.what). imghdr.what returns none for that type of file, so
     # _wider_test_jpeg is run in that case. It still returns None if it didn't
     # match such a jpeg file.
-    kind = imghdr.what(None, h=data) or _wider_test_jpeg(data)
+    kind = _imghdr_what_wrapper(data)
     if kind in ['gif', 'jpeg', 'png', 'tiff', 'bmp']:
         return 'image/{0}'.format(kind)
     elif kind == 'pgm':
@@ -340,6 +354,10 @@ def _image_mime_type(data):
         return 'image/x-{0}'.format(kind)
 
 
+def image_extension(data):
+    return PREFERRED_IMAGE_EXTENSIONS[_imghdr_what_wrapper(data)]
+
+
 class ImageType(enum.Enum):
     """Indicates the kind of an `Image` stored in a file's tag.
     """
@@ -394,7 +412,7 @@ class Image(object):
     @property
     def mime_type(self):
         if self.data:
-            return _image_mime_type(self.data)
+            return image_mime_type(self.data)
 
     @property
     def type_index(self):

beets/util/collections.py

@@ -0,0 +1,27 @@
+# -*- coding: utf-8 -*-
+# This file is part of beets.
+# Copyright 2016, Adrian Sampson.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+"""Custom collections classes
+"""
+
+
+class IdentityFallbackDict(dict):
+    """A dictionary which is "transparent" (maps keys to themselves) for all
+    keys not in it.
+    """
+    def __getitem__(self, key):
+        try:
+            return dict.__getitem__(self, key)
+        except KeyError:
+            return key

beetsplug/fetchart.py

@@ -29,7 +29,7 @@ from beets import importer
 from beets import ui
 from beets import util
 from beets import config
-from beets.mediafile import _image_mime_type
+from beets.mediafile import image_mime_type
 from beets.util.artresizer import ArtResizer
 from beets.util import confit
 from beets.util import syspath, bytestring_path, py3_path
@@ -250,7 +250,7 @@ class RemoteArtSource(ArtSource):
                     # server didn't return enough data, i.e. corrupt image
                     return
 
-                real_ct = _image_mime_type(header)
+                real_ct = image_mime_type(header)
                 if real_ct is None:
                     # detection by file magic failed, fall back to the
                     # server-supplied Content-Type

docs/changelog.rst

@@ -52,6 +52,12 @@ The are a couple of small new features:
 
 And there are a few bug fixes too:
 
+* :doc:`/plugins/embedart`: The plugin now uses ``jpg`` as an extension rather
+  than ``jpeg``, to ensure consistency with :doc:`plugins/fetchart`.
+  Thanks to :user:`tweitzel`. :bug:`2254` :bug:`2255`
+* :doc:`/plugins/embedart`: The plugin now works for all jpeg files, including
+  those that are only recognizable by their magic bytes.
+  :bug:`1545` :bug:`2255`
 * :doc:`/plugins/web`: The JSON output is no longer pretty-printed (for a
   space savings). :bug:`2050`
 * :doc:`/plugins/permissions`: Fix a regression in the previous release where
@@ -70,7 +76,7 @@ And there are a few bug fixes too:
   This is fixed. :bug:`2168`
 * :doc:`/plugins/embyupdate`: Fixes authentication header problem that caused
   a problem that it was not possible to get tokens from the Emby API.
-* :doc:`/plugins/lyrics`: Search for lyrics using the title part preceding the 
+* :doc:`/plugins/lyrics`: Search for lyrics using the title part preceding the
   colon character. :bug:`2206`
 * Fix a crash when a query contains a date field that is not set for all
   the items. :bug:`1938`

test/test_embedart.py

@@ -163,6 +163,17 @@ class EmbedartCliTest(_common.TestCase, TestHelper):
 
         self.assertExists(os.path.join(albumpath, b'extracted.png'))
 
+    def test_extracted_extension(self):
+        resource_path = os.path.join(_common.RSRC, b'image-jpeg.mp3')
+        album = self.add_album_fixture()
+        trackpath = album.items()[0].path
+        albumpath = album.path
+        shutil.copy(syspath(resource_path), syspath(trackpath))
+
+        self.run_command('extractart', '-n', 'extracted')
+
+        self.assertExists(os.path.join(albumpath, b'extracted.jpg'))
+
 
 @patch('beets.art.subprocess')
 @patch('beets.art.extract')

test/test_mediafile_edge.py

@@ -91,8 +91,7 @@ class EdgeTest(unittest.TestCase):
         with open(magic_bytes_file, 'rb') as f:
             jpg_data = f.read()
         self.assertEqual(
-            beets.mediafile._image_mime_type(jpg_data),
-            'image/jpeg')
+            beets.mediafile._imghdr_what_wrapper(jpg_data), 'jpeg')
 
     def test_soundcheck_non_ascii(self):
         # Make sure we don't crash when the iTunes SoundCheck field contains