library: skip saving files with unchanged tags

beets/library/models.py

@@ -931,11 +931,42 @@ def read(self, read_path: util.PathLike | None = None) -> None:
 
         self.path = read_path
 
+    @staticmethod
+    def _read_tags(
+        mediafile: MediaFile, fields: Iterable[str]
+    ) -> dict[str, Any]:
+        """Read `fields` back from `mediafile`.
+
+        Reading the values back is what makes them comparable with the ones
+        about to be written, since a file only keeps what it can represent. It
+        rounds `rg_track_peak` to six decimal places, for instance.
+        """
+
+        def value(field: str) -> Any:
+            value = getattr(mediafile, field)
+            if isinstance(value, list):
+                # An absent list of tags reads back as an empty one from some
+                # formats. The values themselves are kept, empty ones
+                # included, so that dropping one shows.
+                return value or None
+
+            if isinstance(value, float):
+                # A gain of zero decibels is a value, not the lack of one.
+                return value
+
+            # Formats differ on whether they keep a tag holding the null of its
+            # type, an empty string or a zero. Report it as no tag at all.
+            return value or None
+
+        return {f: value(f) for f in fields}
+
     def write(
         self,
         path: bytes | None = None,
         tags: Mapping[str, Any] | None = None,
         id3v23: bool | None = None,
+        *,
+        force: bool = False,
     ) -> None:
         """Write the item's metadata to a media file.
 
@@ -951,6 +982,12 @@ def write(
         `id3v23` will override the global `id3v23` config option if it is
         set to something other than `None`.
 
+        Unless `force` is set, the item's own file is not saved when reading it
+        back already gives the tags to be written. Saving it would give it a new
+        mtime without changing any of its tags. A file whose tags still need
+        converting to another ID3 version is saved either way, since saving is
+        what converts them.
+
         Can raise either a `ReadError` or a `WriteError`.
         """
         if path is None:
@@ -970,14 +1007,50 @@ def write(
             item_tags.update(tags)
         plugins.send("write", item=self, path=path, tags=item_tags)
 
+        # The mtime of the file the tags are about to be read from. A save
+        # skipped below records this one, so a change landing once the file
+        # is read stays newer than the database and still shows.
+        mtime = 0
+        if path == self.path:
+            with suppress(OSError):
+                mtime = self.current_mtime()
+
         # Open the file.
         try:
             mediafile = MediaFile(syspath(path), id3v23=id3v23)
         except UnreadableFileError as exc:
             raise ReadError(path, exc)
 
+        # The tags the update below is going to write. Writing an image
+        # replaces the whole list of them, which none of the fields here would
+        # show, so a write that carries one is saved without comparing anything.
+        written = set(item_tags) & set(mediafile.fields())
+        compare = (
+            not force
+            # Saving is what converts the tags to ID3v2.3, and the conversion
+            # loses what the older version cannot hold, so a file written that
+            # way is saved every time. `MediaFile` sets this for MP3s only.
+            and not mediafile.id3v23
+            # Saving is also what converts older ID3 tags to the default
+            # v2.4. Reading translates them in memory, and `MediaFile` does
+            # not expose the version the file holds, hence mutagen's.
+            and getattr(mediafile.mgfile.tags, "version", (2, 4)) >= (2, 4)
+            and path == self.path
+            and written.isdisjoint(("art", "images"))
+        )
+        old_tags = self._read_tags(mediafile, written) if compare else None
+
         # Write the tags to the file.
         mediafile.update(item_tags)
+
+        if compare and self._read_tags(mediafile, written) == old_tags:
+            # The file already holds these tags. Saving it would only give it a
+            # new mtime, which makes tools that sync the library copy the file
+            # again for nothing.
+            log.debug("no tags to write to {.filepath}", self)
+            self.mtime = mtime
+            return
+
         try:
             mediafile.save()
         except UnreadableFileError as exc:
@@ -993,35 +1066,43 @@ def try_write(
         path: bytes | None = None,
         tags: Mapping[str, Any] | None = None,
         id3v23: bool | None = None,
+        *,
+        force: bool = False,
     ) -> bool:
         """Call `write()` but catch and log `FileOperationError`
         exceptions.
 
         Return `False` an exception was caught and `True` otherwise.
         """
         try:
-            self.write(path=path, tags=tags, id3v23=id3v23)
+            self.write(path=path, tags=tags, id3v23=id3v23, force=force)
             return True
         except FileOperationError as exc:
             log.error("{}", exc)
             return False
 
     def try_sync(
-        self, write: bool, move: bool, with_album: bool = True
+        self,
+        write: bool,
+        move: bool,
+        with_album: bool = True,
+        *,
+        force_write: bool = False,
     ) -> None:
         """Synchronize the item with the database and, possibly, update its
         tags on disk and its path (by moving the file).
 
-        `write` indicates whether to write new tags into the file. Similarly,
-        `move` controls whether the path should be updated. In the
-        latter case, files are *only* moved when they are inside their
+        `write` indicates whether to write new tags into the file, and
+        `force_write` whether to do so even when the file already contains
+        them. Similarly, `move` controls whether the path should be updated.
+        In the latter case, files are *only* moved when they are inside their
         library's directory (if any).
 
         Similar to calling :meth:`write`, :meth:`move`, and :meth:`store`
         (conditionally).
         """
         if write:
-            self.try_write()
+            self.try_write(force=force_write)
         if move:
             # Check whether this file is inside the library directory.
             if self._db and self._db.directory in util.ancestry(self.path):

beets/ui/commands/write.py

@@ -37,7 +37,7 @@ def write_items(lib, query, pretend, force):
         if (changed or force) and not pretend:
             # We use `try_sync` here to keep the mtime up to date in the
             # database.
-            item.try_sync(True, False)
+            item.try_sync(True, False, force_write=force)
 
 
 def write_func(lib, opts, args):

docs/changelog.rst

@@ -95,10 +95,21 @@ Bug fixes
   valid date/time string" error instead of crashing with an uncaught
   ``KeyError``. A ``|`` was being accepted as a relative-date unit due to a
   regular expression character-class typo.
+- ``beet modify --write`` no longer re-saves a file that already holds the tags
+  being written, so changing a field only the database keeps, such as
+  ``data_source``, no longer gives the file a new modification time. Files
+  written with the ``id3v23`` option are still saved every time, and so are
+  files whose tags have yet to be converted to the default ID3v2.4, since saving
+  is what converts them. :bug:`6529`
 
-..
-    For plugin developers
-    ~~~~~~~~~~~~~~~~~~~~~
+For plugin developers
+~~~~~~~~~~~~~~~~~~~~~
+
+- ``Item.write()`` no longer saves the item's own file, and no longer sends the
+  ``after_write`` event, when the file already holds the tags being written. A
+  file whose tags still need converting between ID3 versions is saved either
+  way. Pass ``force=True`` (or ``Item.try_sync(..., force_write=True)``) to save
+  it regardless.
 
 Other changes
 ~~~~~~~~~~~~~

docs/dev/library.rst

@@ -115,8 +115,9 @@ When it is possible that the metadata is out of sync, beets can then just set
 
 This leads to the following implementation policy:
 
-    - On every write of disk metadata (``Item.write()``), the DB mtime is
-      updated to match the post-write disk mtime.
+    - On every call to ``Item.write()`` for the item's own file, the DB mtime is
+      updated to match the disk mtime. This includes the case where the file
+      already holds the tags and is therefore left alone.
     - Same for metadata reads (``Item.read()``).
     - On every modification to DB metadata (``item.field = ...``), the DB mtime
       is reset to zero.

docs/dev/plugins/events.rst

@@ -111,13 +111,15 @@ registration process in this case:
 
 ``write``
     :Parameters: ``item`` (|Item|), ``path`` (path), ``tags`` (dict)
-    :Description: Called just before a file's metadata is written to disk.
-        Handlers may modify ``tags`` or raise ``library.FileOperationError`` to
-        abort.
+    :Description: Called before beets decides whether to write a file's metadata
+        to disk. Handlers may modify ``tags`` or raise
+        ``library.FileOperationError`` to abort.
 
 ``after_write``
-    :Parameters: ``item`` (|Item|)
-    :Description: Called after a file's metadata is written to disk.
+    :Parameters: ``item`` (|Item|), ``path`` (path)
+    :Description: Called after a file's metadata is written to disk. Not sent
+        when the write was skipped because the item's own file already held the
+        tags (see ``Item.write()``).
 
 ``import_task_created``
     :Parameters: ``task`` (|ImportTask|), ``session`` (|ImportSession|)

docs/plugins/importadded.rst

@@ -47,7 +47,9 @@ file. There are two options available:
 - **preserve_mtimes**: After importing files, re-set their mtimes to their
   original value. Default: ``no``.
 - **preserve_write_mtimes**: After writing files, re-set their mtimes to their
-  original value. Default: ``no``.
+  original value. A file that already holds the tags being written is not saved,
+  so its mtime is not re-set either; pass ``-f`` to ``beet write`` to force the
+  save. Default: ``no``.
 
 Reimport
 --------

docs/reference/cli.rst

@@ -309,6 +309,8 @@ Items will automatically be moved around when necessary if they're in your
 library directory, but you can disable that with ``-M``. Tags will be written to
 the files according to the settings you have for imports, but these can be
 overridden with ``-w`` (write tags, the default) and ``-W`` (don't write tags).
+A file that already holds the tags is left untouched, so changing a field that
+beets keeps only in its database does not give the file a new modification time.
 
 When you run the ``modify`` command, it prints a list of all affected items in
 the library and asks for your permission before making any changes. You can then

docs/reference/config.rst

@@ -420,6 +420,10 @@ By default, beets writes MP3 tags using the ID3v2.4 standard, the latest version
 of ID3. Enable this option to instead use the older ID3v2.3 standard, which is
 preferred by certain older software such as Windows Media Player.
 
+Saving a file is what converts its tags to ID3v2.3, so beets saves your MP3s
+every time it writes them while this option is enabled, even the ones that
+already hold the tags it is writing.
+
 .. _va_name:
 
 va_name