From 3984febf6cc1beba0c48a175c394b393eadab721 Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Mon, 27 Jun 2016 08:37:20 +0200 Subject: [PATCH 1/4] mediafile: Add note to ASF.delete that it's fixed in newer mutagen --- beets/mediafile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/mediafile.py b/beets/mediafile.py index 4e92419bb..b9fcae091 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1457,6 +1457,7 @@ class MediaFile(object): try: self.mgfile.delete() except NotImplementedError: + # FIXME: This is fixed in mutagen >=1.31 # For Mutagen types that don't support deletion (notably, # ASF), just delete each tag individually. for tag in self.mgfile.keys(): From 9f16cfd0786e0878fda93256eed8a729a7e3d2e0 Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Mon, 27 Jun 2016 08:39:27 +0200 Subject: [PATCH 2/4] mediafile: Remove alac detection workaround No longer needed since we depend on mutagen 1.27 --- beets/mediafile.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/beets/mediafile.py b/beets/mediafile.py index b9fcae091..1874b7d10 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1384,20 +1384,10 @@ class MediaFile(object): elif (type(self.mgfile).__name__ == 'M4A' or type(self.mgfile).__name__ == 'MP4'): info = self.mgfile.info - if hasattr(info, 'codec'): - if info.codec and info.codec.startswith('alac'): - self.type = 'alac' - else: - self.type = 'aac' + if info.codec and info.codec.startswith('alac'): + self.type = 'alac' else: - # This hack differentiates AAC and ALAC on versions of - # Mutagen < 1.26. Once Mutagen > 1.26 is out and - # required by beets, we can remove this. - if hasattr(self.mgfile.info, 'bitrate') and \ - self.mgfile.info.bitrate > 0: - self.type = 'aac' - else: - self.type = 'alac' + self.type = 'aac' elif (type(self.mgfile).__name__ == 'ID3' or type(self.mgfile).__name__ == 'MP3'): self.type = 'mp3' From 45404bce850487f185ad4c1c56981bb4a41f35b3 Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Mon, 27 Jun 2016 08:41:34 +0200 Subject: [PATCH 3/4] mediafile: Add note about MPEGInfo.channels being available in newer mutagen versions --- beets/mediafile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/mediafile.py b/beets/mediafile.py index 1874b7d10..0f51e5017 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1960,6 +1960,7 @@ class MediaFile(object): def channels(self): """The number of channels in the audio (an int).""" if isinstance(self.mgfile.info, mutagen.mp3.MPEGInfo): + # FIXME: MPEGInfo.channels was added in mutagen 1.30 return { mutagen.mp3.STEREO: 2, mutagen.mp3.JOINTSTEREO: 2, From 629241efd389bea7b4075f2591a06f2ef462dc82 Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Mon, 27 Jun 2016 09:43:48 +0200 Subject: [PATCH 4/4] mediafile: Cleanup mutagen error handling Instead of the individial mutagen format exceptions use the mutagen.MutagenError exception introduced in 1.25. Since 1.33 mutagen will only raise MutagenError for load/save/delete and no longer raise IOError. Translate both errors to UnreadableFileError to support older and newer mutagen versions. Unify error handling in __init__(), save() and delete(). Since it's no longer possible to get an IOError from MediaFile, adjust all callers and tests accordingly. This was tested with mutagen 1.27 and current mutagen master. --- beets/library.py | 8 ++--- beets/mediafile.py | 66 +++++++++++++++++-------------------- beetsplug/scrub.py | 13 +++++--- test/test_mediafile.py | 23 ++++++++++++- test/test_mediafile_edge.py | 2 +- 5 files changed, 65 insertions(+), 47 deletions(-) diff --git a/beets/library.py b/beets/library.py index 5859451d5..5ef809122 100644 --- a/beets/library.py +++ b/beets/library.py @@ -26,7 +26,7 @@ import six from unidecode import unidecode from beets import logging -from beets.mediafile import MediaFile, MutagenError, UnreadableFileError +from beets.mediafile import MediaFile, UnreadableFileError from beets import plugins from beets import util from beets.util import bytestring_path, syspath, normpath, samefile @@ -568,7 +568,7 @@ class Item(LibModel): read_path = normpath(read_path) try: mediafile = MediaFile(syspath(read_path)) - except (OSError, IOError, UnreadableFileError) as exc: + except UnreadableFileError as exc: raise ReadError(read_path, exc) for key in self._media_fields: @@ -615,14 +615,14 @@ class Item(LibModel): try: mediafile = MediaFile(syspath(path), id3v23=beets.config['id3v23'].get(bool)) - except (OSError, IOError, UnreadableFileError) as exc: + except UnreadableFileError as exc: raise ReadError(self.path, exc) # Write the tags to the file. mediafile.update(item_tags) try: mediafile.save() - except (OSError, IOError, MutagenError) as exc: + except UnreadableFileError as exc: raise WriteError(self.path, exc) # The file has a new mtime. diff --git a/beets/mediafile.py b/beets/mediafile.py index 0f51e5017..6cdea47b4 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1346,32 +1346,12 @@ class MediaFile(object): path = syspath(path) self.path = path - unreadable_exc = ( - mutagen.mp3.error, - mutagen.id3.error, - mutagen.flac.error, - mutagen.monkeysaudio.MonkeysAudioHeaderError, - mutagen.mp4.error, - mutagen.oggopus.error, - mutagen.oggvorbis.error, - mutagen.ogg.error, - mutagen.asf.error, - mutagen.apev2.error, - mutagen.aiff.error, - ) try: self.mgfile = mutagen.File(path) - except unreadable_exc as exc: - log.debug(u'header parsing failed: {0}', six.text_type(exc)) + except (mutagen.MutagenError, IOError) as exc: + # Mutagen <1.33 could raise IOError + log.debug(u'parsing failed: {0}', six.text_type(exc)) raise UnreadableFileError(path) - except IOError as exc: - if type(exc) == IOError: - # This is a base IOError, not a subclass from Mutagen or - # anywhere else. - raise - else: - log.debug(u'{}', traceback.format_exc()) - raise MutagenError(path, exc) except Exception as exc: # Isolate bugs in Mutagen. log.debug(u'{}', traceback.format_exc()) @@ -1418,7 +1398,8 @@ class MediaFile(object): self.id3v23 = id3v23 and self.type == 'mp3' def save(self): - """Write the object's tags back to the file. + """Write the object's tags back to the file. May + throw `UnreadableFileError`. """ # Possibly save the tags to ID3v2.3. kwargs = {} @@ -1430,28 +1411,41 @@ class MediaFile(object): id3.update_to_v23() kwargs['v2_version'] = 3 - # Isolate bugs in Mutagen. try: self.mgfile.save(**kwargs) - except (IOError, OSError): - # Propagate these through: they don't represent Mutagen bugs. - raise + except (mutagen.MutagenError, IOError) as exc: + # Mutagen <1.33 could raise IOError + log.debug(u'saving failed: {0}', six.text_type(exc)) + raise UnreadableFileError(self.path) except Exception as exc: + # Isolate bugs in Mutagen. log.debug(u'{}', traceback.format_exc()) log.error(u'uncaught Mutagen exception in save: {0}', exc) raise MutagenError(self.path, exc) def delete(self): - """Remove the current metadata tag from the file. + """Remove the current metadata tag from the file. May + throw `UnreadableFileError`. """ + try: - self.mgfile.delete() - except NotImplementedError: - # FIXME: This is fixed in mutagen >=1.31 - # For Mutagen types that don't support deletion (notably, - # ASF), just delete each tag individually. - for tag in self.mgfile.keys(): - del self.mgfile[tag] + try: + self.mgfile.delete() + except NotImplementedError: + # FIXME: This is fixed in mutagen >=1.31 + # For Mutagen types that don't support deletion (notably, + # ASF), just delete each tag individually. + for tag in self.mgfile.keys(): + del self.mgfile[tag] + except (mutagen.MutagenError, IOError) as exc: + # Mutagen <1.33 could raise IOError + log.debug(u'deleting failed: {0}', six.text_type(exc)) + raise UnreadableFileError(self.path) + except Exception as exc: + # Isolate bugs in Mutagen. + log.debug(u'{}', traceback.format_exc()) + log.error(u'uncaught Mutagen exception in save: {0}', exc) + raise MutagenError(self.path, exc) # Convenient access to the set of available fields. diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index ed4040d5b..4dcefe572 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -119,7 +119,7 @@ class ScrubPlugin(BeetsPlugin): try: mf = mediafile.MediaFile(util.syspath(item.path), config['id3v23'].get(bool)) - except IOError as exc: + except mediafile.UnreadableFileError as exc: self._log.error(u'could not open file to scrub: {0}', exc) art = mf.art @@ -133,10 +133,13 @@ class ScrubPlugin(BeetsPlugin): item.try_write() if art: self._log.debug(u'restoring art') - mf = mediafile.MediaFile(util.syspath(item.path), - config['id3v23'].get(bool)) - mf.art = art - mf.save() + try: + mf = mediafile.MediaFile(util.syspath(item.path), + config['id3v23'].get(bool)) + mf.art = art + mf.save() + except mediafile.UnreadableFileError as exc: + self._log.error(u'could not write tags: {0}', exc) def import_task_files(self, session, task): """Automatically scrub imported files.""" diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 258e03896..7a299a1d0 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -29,7 +29,7 @@ from test import _common from test._common import unittest from beets.mediafile import MediaFile, MediaField, Image, \ MP3DescStorageStyle, StorageStyle, MP4StorageStyle, \ - ASFStorageStyle, ImageType, CoverArtField + ASFStorageStyle, ImageType, CoverArtField, UnreadableFileError from beets.library import Item from beets.plugins import BeetsPlugin from beets.util import bytestring_path @@ -455,6 +455,27 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, if os.path.isdir(self.temp_dir): shutil.rmtree(self.temp_dir) + def test_read_nonexisting(self): + mediafile = self._mediafile_fixture('full') + os.remove(mediafile.path) + self.assertRaises(UnreadableFileError, MediaFile, mediafile.path) + + def test_save_nonexisting(self): + mediafile = self._mediafile_fixture('full') + os.remove(mediafile.path) + try: + mediafile.save() + except UnreadableFileError: + pass + + def test_delete_nonexisting(self): + mediafile = self._mediafile_fixture('full') + os.remove(mediafile.path) + try: + mediafile.delete() + except UnreadableFileError: + pass + def test_read_audio_properties(self): mediafile = self._mediafile_fixture('full') for key, value in self.audio_properties.items(): diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py index 9fbf4e2bb..c95556820 100644 --- a/test/test_mediafile_edge.py +++ b/test/test_mediafile_edge.py @@ -193,7 +193,7 @@ class SafetyTest(unittest.TestCase, TestHelper): fn = os.path.join(_common.RSRC, b'brokenlink') os.symlink('does_not_exist', fn) try: - self.assertRaises(IOError, + self.assertRaises(beets.mediafile.UnreadableFileError, beets.mediafile.MediaFile, fn) finally: os.unlink(fn)