diff --git a/beets/library.py b/beets/library.py index 7d02c32d1..f9285cabb 100644 --- a/beets/library.py +++ b/beets/library.py @@ -408,7 +408,7 @@ class Item(LibModel): except (OSError, IOError) as exc: raise ReadError(read_path, exc) - for key in list(self._media_fields): + for key in self._media_fields: value = getattr(mediafile, key) if isinstance(value, (int, long)): # Filter values wider than 64 bits (in signed representation). @@ -427,7 +427,8 @@ class Item(LibModel): def write(self, path=None): """Write the item's metadata to a media file. - Updates the mediafile with properties from itself. + All fields in `_media_fields` are written to disk according to + the values on this object. Can raise either a `ReadError` or a `WriteError`. """ @@ -442,8 +443,9 @@ class Item(LibModel): plugins.send('write', item=self, path=path) + mediafile.update(self) try: - mediafile.update(self, id3v23=beets.config['id3v23'].get(bool)) + mediafile.save(id3v23=beets.config['id3v23'].get(bool)) except (OSError, IOError, MutagenError) as exc: raise WriteError(self.path, exc) diff --git a/beets/mediafile.py b/beets/mediafile.py index c5e5efefa..c25fd3c86 100644 --- a/beets/mediafile.py +++ b/beets/mediafile.py @@ -1258,9 +1258,13 @@ class MediaFile(object): for tag in self.mgfile.keys(): del self.mgfile[tag] + + # Convenient access to the set of available fields. + @classmethod def fields(cls): - """Yield the names of all properties that are MediaFields. + """Get the names of all writable properties that reflect + metadata tags (i.e., those that are `MediaField`s). """ for property, descriptor in cls.__dict__.items(): if isinstance(descriptor, MediaField): @@ -1268,13 +1272,13 @@ class MediaFile(object): @classmethod def readable_fields(cls): - """Yield the elements of ``fields()`` and all additional - properties retrieved from the file + """Get all metadata fields: the writable ones from `fields` and + also other audio properties. """ for property in cls.fields(): yield property - for property in ['length', 'samplerate', 'bitdepth', 'bitrate', - 'channels', 'format']: + for property in ('length', 'samplerate', 'bitdepth', 'bitrate', + 'channels', 'format'): yield property @classmethod @@ -1282,9 +1286,7 @@ class MediaFile(object): """Add a field to store custom tags. ``name`` is the name of the property the field is accessed - through. It must not already exist for the class. If the name - coincides with the name of a property of ``Item`` it will be set - from the item in ``item.write()``. + through. It must not already exist on this class. ``descriptor`` must be an instance of ``MediaField``. """ @@ -1296,21 +1298,16 @@ class MediaFile(object): u'property "{0}" already exists on MediaField'.format(name)) setattr(cls, name, descriptor) - def update(self, dict, id3v23=False): - """Update tags from the dictionary and write them to the file. + def update(self, dict): + """Set all field values from a dictionary. For any key in ``dict`` that is also a field to store tags the method retrieves the corresponding value from ``dict`` and - updates the ``MediaFile``. The changes are then written to the - disk. - - By default, MP3 files are saved with ID3v2.4 tags. You can use - the older ID3v2.3 standard by specifying the `id3v23` option. + updates the ``MediaFile``. """ for field in self.fields(): if field in dict: setattr(self, field, dict[field]) - self.save(id3v23) # Field definitions. diff --git a/test/test_mediafile.py b/test/test_mediafile.py index 6f86f6c88..5548e364a 100644 --- a/test/test_mediafile.py +++ b/test/test_mediafile.py @@ -194,6 +194,7 @@ class LazySaveTestMixin(object): self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) mediafile.update({'title': mediafile.title}) + mediafile.save() self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) @unittest.skip('not yet implemented') @@ -213,6 +214,7 @@ class LazySaveTestMixin(object): self.assertEqual(os.stat(mediafile.path).st_mtime, mtime) mediafile.update({'title': mediafile.title, 'album': 'another'}) + mediafile.save() self.assertNotEqual(os.stat(mediafile.path).st_mtime, mtime) def _set_past_mtime(self, path): @@ -297,6 +299,7 @@ class ExtendedFieldTestMixin(object): mediafile = self._mediafile_fixture('empty') mediafile.update({'initialkey': 'F#'}) + mediafile.save() item = Item.from_path(mediafile.path) self.assertEqual(item['initialkey'], 'F#') @@ -441,6 +444,7 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, tags = self._generate_tags() mediafile.update(tags) + mediafile.save() mediafile = MediaFile(mediafile.path) self.assertTags(mediafile, tags) @@ -466,8 +470,10 @@ class ReadWriteTestBase(ArtTestMixin, GenreListTestMixin, tags = self._generate_tags() mediafile.update(tags) + mediafile.save() # Make sure the tags are already set when writing a second time mediafile.update(tags) + mediafile.save() mediafile = MediaFile(mediafile.path) self.assertTags(mediafile, tags)