From 7de6259c1d445de7972469f88fba484a443cb090 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 15 Aug 2014 12:09:18 -0700 Subject: [PATCH] MediaFile: make id3v23 a constructor parameter For #899, we need to change MediaFile's behavior (pre-write) based on whether we're doing ID3v2.3 or not. So we need a field on the object, not a parameter to `save()`. --- beets/library.py | 5 +++-- beets/mediafile.py | 18 +++++++++++------- beetsplug/embedart.py | 5 +++-- beetsplug/scrub.py | 5 +++-- test/test_mediafile_edge.py | 16 ++++++++-------- 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/beets/library.py b/beets/library.py index 8496a844e..59d681043 100644 --- a/beets/library.py +++ b/beets/library.py @@ -393,7 +393,8 @@ class Item(LibModel): else: path = normpath(path) try: - mediafile = MediaFile(syspath(path)) + mediafile = MediaFile(syspath(path), + id3v23=beets.config['id3v23'].get(bool)) except (OSError, IOError) as exc: raise ReadError(self.path, exc) @@ -401,7 +402,7 @@ class Item(LibModel): mediafile.update(self) try: - mediafile.save(id3v23=beets.config['id3v23'].get(bool)) + mediafile.save() except (OSError, IOError, MutagenError) as exc: raise WriteError(self.path, exc) diff --git a/beets/mediafile.py b/beets/mediafile.py index 24152ec35..c1be35d49 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1222,9 +1222,12 @@ class MediaFile(object): """Represents a multimedia file on disk and provides access to its metadata. """ - def __init__(self, path): + def __init__(self, path, id3v23=False): """Constructs a new `MediaFile` reflecting the file at path. May throw `UnreadableFileError`. + + By default, MP3 files are saved with ID3v2.4 tags. You can use + the older ID3v2.3 standard by specifying the `id3v23` option. """ self.path = path @@ -1296,18 +1299,19 @@ class MediaFile(object): else: raise FileTypeError(path, type(self.mgfile).__name__) - # add a set of tags if it's missing + # Add a set of tags if it's missing. if self.mgfile.tags is None: self.mgfile.add_tags() - def save(self, id3v23=False): - """Write the object's tags back to the file. + # Set the ID3v2.3 flag only for MP3s. + self.id3v23 = id3v23 and self.type == 'mp3' - By default, MP3 files are saved with ID3v2.4 tags. You can use - the older ID3v2.3 standard by specifying the `id3v23` option. + def save(self): + """Write the object's tags back to the file. """ + # Possibly save the tags to ID3v2.3. kwargs = {} - if id3v23 and self.type == 'mp3': + if self.id3v23: id3 = self.mgfile if hasattr(id3, 'tags'): # In case this is an MP3 object, not an ID3 object. diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 0322af6c7..46e10a61d 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -183,11 +183,12 @@ def clear(lib, query): for item in lib.items(query): log.info(u'%s - %s' % (item.artist, item.title)) try: - mf = mediafile.MediaFile(syspath(item.path)) + mf = mediafile.MediaFile(syspath(item.path), + config['id3v23'].get(bool)) except mediafile.UnreadableFileError as exc: log.error(u'Could not clear art from {0}: {1}'.format( displayable_path(item.path), exc )) continue mf.art = None - mf.save(config['id3v23'].get(bool)) + mf.save() diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py index 0b0c69e2f..af1a77370 100644 --- a/beetsplug/scrub.py +++ b/beetsplug/scrub.py @@ -68,7 +68,8 @@ class ScrubPlugin(BeetsPlugin): # Get album art if we need to restore it. if opts.write: - mf = mediafile.MediaFile(item.path) + mf = mediafile.MediaFile(item.path, + config['id3v23'].get(bool)) art = mf.art # Remove all tags. @@ -82,7 +83,7 @@ class ScrubPlugin(BeetsPlugin): log.info('restoring art') mf = mediafile.MediaFile(item.path) mf.art = art - mf.save(config['id3v23'].get(bool)) + mf.save() scrubbing = False diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py index b0b45da0c..cb32b92ae 100644 --- a/test/test_mediafile_edge.py +++ b/test/test_mediafile_edge.py @@ -269,21 +269,21 @@ class SoundCheckTest(unittest.TestCase): class ID3v23Test(unittest.TestCase, TestHelper): - def _make_test(self, ext='mp3'): + def _make_test(self, ext='mp3', id3v23=False): self.create_temp_dir() src = os.path.join(_common.RSRC, 'full.{0}'.format(ext)) self.path = os.path.join(self.temp_dir, 'test.{0}'.format(ext)) shutil.copy(src, self.path) - return beets.mediafile.MediaFile(self.path) + return beets.mediafile.MediaFile(self.path, id3v23=id3v23) def _delete_test(self): self.remove_temp_dir() def test_v24_year_tag(self): - mf = self._make_test() + mf = self._make_test(id3v23=False) try: mf.year = 2013 - mf.save(id3v23=False) + mf.save() frame = mf.mgfile['TDRC'] self.assertTrue('2013' in str(frame)) self.assertTrue('TYER' not in mf.mgfile) @@ -291,10 +291,10 @@ class ID3v23Test(unittest.TestCase, TestHelper): self._delete_test() def test_v23_year_tag(self): - mf = self._make_test() + mf = self._make_test(id3v23=True) try: mf.year = 2013 - mf.save(id3v23=True) + mf.save() frame = mf.mgfile['TYER'] self.assertTrue('2013' in str(frame)) self.assertTrue('TDRC' not in mf.mgfile) @@ -302,10 +302,10 @@ class ID3v23Test(unittest.TestCase, TestHelper): self._delete_test() def test_v23_on_non_mp3_is_noop(self): - mf = self._make_test('m4a') + mf = self._make_test('m4a', id3v23=True) try: mf.year = 2013 - mf.save(id3v23=True) + mf.save() finally: self._delete_test()