Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite handling of images extensions and mimetype #2255

Merged
merged 5 commits into from
Nov 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions beets/art.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import subprocess
import platform
from tempfile import NamedTemporaryFile
import imghdr
import os

from beets.util import displayable_path, syspath, bytestring_path
Expand Down Expand Up @@ -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))
Expand Down
24 changes: 21 additions & 3 deletions beets/mediafile.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

from beets import logging
from beets.util import displayable_path, syspath, as_string
from beets.util.collections import IdentityFallbackDict
import six


Expand All @@ -81,6 +82,8 @@
'aiff': 'AIFF',
}

PREFERRED_IMAGE_EXTENSIONS = IdentityFallbackDict({'jpeg': 'jpg'})


# Exceptions.

Expand Down Expand Up @@ -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
Expand All @@ -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':
Expand All @@ -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.
"""
Expand Down Expand Up @@ -394,7 +412,7 @@ def __init__(self, data, desc=None, type=None):
@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):
Expand Down
27 changes: 27 additions & 0 deletions beets/util/collections.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions beetsplug/fetchart.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
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
Expand Down Expand Up @@ -250,7 +250,7 @@ def fetch_image(self, candidate, extra):
# 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
Expand Down
8 changes: 7 additions & 1 deletion docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
Expand Down
Binary file added test/rsrc/image-jpeg.mp3
Binary file not shown.
11 changes: 11 additions & 0 deletions test/test_embedart.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,17 @@ def test_non_ascii_album_path(self):

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')
Expand Down
3 changes: 1 addition & 2 deletions test/test_mediafile_edge.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ def test_only_magic_bytes_jpeg(self):
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
Expand Down