feat: expose attachment size on MKVAttachment

pymkv/MKVAttachment.py

@@ -56,6 +56,9 @@ class MKVAttachment:
     attach_once : bool
         Determines if the attachment should be added to all split files or only the first. Default is False,
         which will attach to all files.
+    size : int | None
+        The size of the attachment in bytes, as reported by the file it was read from. It is ``None`` for an
+        attachment built from a local path, since that attachment is not part of an MKV yet.
     """
 
     def __init__(
@@ -73,6 +76,7 @@ def __init__(
         self._attach_once = attach_once
         self._source_id: int | None = None
         self._source_file: str | None = None
+        self._size: int | None = None
 
     def __repr__(self) -> str:
         """
@@ -90,10 +94,16 @@ def __repr__(self) -> str:
     def file_path(self) -> str:
         """str: The path to the attachment file.
 
+        For an attachment read out of an MKV this is the containing file, and the path cannot be changed.
+        Replace such an attachment with :meth:`~pymkv.MKVFile.remove_attachment` followed by
+        :meth:`~pymkv.MKVFile.add_attachment`.
+
         Raises
         ------
         FileNotFoundError
             Raised if `file_path` does not exist.
+        ValueError
+            Raised when changing the path of an attachment that was read from a file.
         """
         return self._file_path
 
@@ -109,17 +119,38 @@ def file_path(self, file_path: str) -> None:
         ------
         FileNotFoundError
             If the specified file does not exist.
+        ValueError
+            If this attachment was read from a file and `file_path` names a different one.
 
         Returns
         -------
         None
         """
+        current = getattr(self, "_file_path", None)
+        if current is not None and str(file_path) == current:
+            # Byte-identical path. Nothing to validate, and nothing learned about the file has gone stale,
+            # so this stays a no-op even if the file has since been deleted.
+            return
         fp = Path(file_path).expanduser()
         if not fp.is_file():
             msg = f'"{fp}" does not exist'
             raise FileNotFoundError(msg)
+        if current is not None and Path(current).is_file() and fp.samefile(current):
+            # Same file spelled differently, e.g. after the caller normalized the path.
+            return
+        if getattr(self, "_source_id", None) is not None:
+            # This attachment is a reference into an MKV, not a local file. Repointing it used to look like
+            # it worked while AttachmentOptions skipped it, so the old embedded payload was muxed instead.
+            msg = (
+                f"attachment {self._name!r} (id {self._source_id}) was read from "
+                f'"{current}", so its path cannot be changed. '
+                "Remove this attachment and add a new MKVAttachment for the replacement instead."
+            )
+            raise ValueError(msg)
         self._mime_type = guess_type(fp)[0]
         self._name = None
+        # The name and size described the attachment this object used to point at.
+        self._size = None
         self._file_path = str(fp)
 
     @property
@@ -219,11 +250,43 @@ def source_id(self, source_id: int | None) -> None:
         """
         Set the ID of the attachment from the source file.
 
+        Pointing at a different attachment invalidates the size, which described the previous one.
+
         Parameters:
             source_id (int | None): The ID to set for the attachment in the source file.
         """
+        if source_id != self._source_id:
+            self._size = None
         self._source_id = source_id
 
+    @property
+    def size(self) -> int | None:
+        """
+        Get the size of the attachment in bytes.
+
+        The value comes from the file the attachment was read from, not from
+        :attr:`~pymkv.MKVAttachment.file_path`, which points at the containing MKV for an attachment read
+        out of one.
+
+        It stays None for an attachment built from a local path, including after
+        :meth:`~pymkv.MKVFile.mux`: this object is never refreshed from the output. Load the muxed file into
+        a new :class:`~pymkv.MKVFile` to read the sizes it ended up with.
+
+        Returns:
+            int | None: The size in bytes, or None for an attachment that was not read from a file.
+        """
+        return self._size
+
+    @size.setter
+    def size(self, size: int | None) -> None:
+        """
+        Set the size of the attachment in bytes.
+
+        Parameters:
+            size (int | None): The size in bytes.
+        """
+        self._size = size
+
     @property
     def source_file(self) -> str | None:
         """
@@ -239,7 +302,11 @@ def source_file(self, source_file: str | None) -> None:
         """
         Set the path to the source file containing the attachment.
 
+        Pointing at a different file invalidates the size, which described the previous one.
+
         Parameters:
             source_file (str | None): The path to set for the source file.
         """
+        if source_file != self._source_file:
+            self._size = None
         self._source_file = source_file

pymkv/MKVFile.py

@@ -246,6 +246,7 @@ def __init__(
                     new_attachment.mime_type = mime_type
                     new_attachment.source_id = attachment_id
                     new_attachment.source_file = file_path
+                    new_attachment.size = attachment.size
                     self.attachments.append(new_attachment)
 
         # split options

pymkv/models.py

@@ -158,13 +158,16 @@ class AttachmentInfo(msgspec.Struct):
         The ID of the attachment.
     properties : AttachmentProperties
         The properties of the attachment.
+    size : int | None
+        The size of the attachment in bytes. None when mkvmerge did not report one — mkvmerge refuses to
+        attach an empty file, so a real attachment is never 0 bytes and a missing value must not be read as one.
     """
 
     id: int
     file_name: str | None = None
     content_type: str | None = None
     description: str | None = None
-    size: int = 0
+    size: int | None = None
     properties: AttachmentProperties = msgspec.field(default_factory=AttachmentProperties)
 
 

tests/test_mkv_file_attachments.py

@@ -1,3 +1,4 @@
+import subprocess
 from pathlib import Path
 
 import msgspec
@@ -9,6 +10,9 @@
 ATTACHMENT_COUNT_1 = 1
 ATTACHMENT_COUNT_2 = 2
 ATTACHMENT_COUNT_3 = 3
+SMALL_ATTACHMENT_SIZE = 24
+LARGE_ATTACHMENT_SIZE = 4096
+REPLACEMENT_SIZE = 1234
 
 
 def test_get_attachment(temp_file: str) -> None:
@@ -174,3 +178,211 @@ def test_attachments_preserved_after_mux(temp_file: str, tmp_path: Path, get_pat
     assert len(output_mkv.attachments) >= ATTACHMENT_COUNT_2, (
         f"Expected at least 2 attachments, found {output_mkv.attachments}"
     )
+
+
+def _mkv_with_two_attachments(source: Path, tmp_path: Path) -> tuple[str, int, int]:
+    """Mux two attachments of deliberately different sizes and return the path plus both sizes."""
+    small = tmp_path / "small.txt"
+    small.write_bytes(b"x" * SMALL_ATTACHMENT_SIZE)
+    large = tmp_path / "large.txt"
+    large.write_bytes(b"y" * LARGE_ATTACHMENT_SIZE)
+
+    mkv = MKVFile(str(source))
+    mkv.add_attachment(MKVAttachment(str(small)))
+    mkv.add_attachment(MKVAttachment(str(large)))
+    output = str(tmp_path / "with_attachments.mkv")
+    mkv.mux(output, silent=True)
+    return output, SMALL_ATTACHMENT_SIZE, LARGE_ATTACHMENT_SIZE
+
+
+def test_attachment_size_read_from_source(get_path_test_file: Path, tmp_path: Path) -> None:
+    """Each attachment reports its own payload size, not the size of the MKV holding it."""
+    output, small_size, large_size = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    container_size = Path(output).stat().st_size
+
+    attachments = MKVFile(output).attachments
+
+    assert [a.size for a in attachments] == [small_size, large_size]
+    # file_path points at the container, so a size taken from it would be this instead.
+    assert container_size not in [a.size for a in attachments]
+
+
+def test_attachment_size_is_none_before_muxing(temp_file: str) -> None:
+    """An attachment built from a local path is not in an MKV yet, so it has no reported size."""
+    assert MKVAttachment(temp_file).size is None
+
+
+def test_attachment_size_survives_mux(get_path_test_file: Path, tmp_path: Path) -> None:
+    output, small_size, large_size = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+
+    mkv = MKVFile(output)
+    remuxed = str(tmp_path / "remuxed.mkv")
+    mkv.mux(remuxed, silent=True)
+
+    assert [a.size for a in MKVFile(remuxed).attachments] == [small_size, large_size]
+
+
+def test_attachment_read_from_file_rejects_a_new_path(get_path_test_file: Path, tmp_path: Path) -> None:
+    """Repointing an embedded attachment used to mux the old payload silently, so it is refused."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    attachment = MKVFile(output).attachments[0]
+    other = tmp_path / "other.txt"
+    other.write_bytes(b"z" * LARGE_ATTACHMENT_SIZE)
+
+    with pytest.raises(ValueError, match="cannot be changed"):
+        attachment.file_path = str(other)
+
+    assert attachment.size == SMALL_ATTACHMENT_SIZE
+
+
+def test_replacing_an_attachment_by_removing_and_adding(get_path_test_file: Path, tmp_path: Path) -> None:
+    """The supported way to swap an attachment: remove it, add a new one, and the bytes follow."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    replacement = tmp_path / "replacement.bin"
+    replacement.write_bytes(b"r" * REPLACEMENT_SIZE)
+
+    mkv = MKVFile(output)
+    mkv.remove_attachment(0)
+    mkv.add_attachment(MKVAttachment(str(replacement)))
+    repointed = str(tmp_path / "replaced.mkv")
+    mkv.mux(repointed, silent=True)
+
+    result = MKVFile(repointed)
+    by_name = {a.name: a.size for a in result.attachments}
+    assert by_name.get("replacement.bin") == REPLACEMENT_SIZE
+    assert "small.txt" not in by_name
+
+    extracted = tmp_path / "extracted.bin"
+    attachment_id = next(a.source_id for a in result.attachments if a.name == "replacement.bin")
+    subprocess.run(  # noqa: S603
+        ["mkvextract", repointed, "attachments", f"{attachment_id}:{extracted}"],  # noqa: S607
+        check=True,
+        capture_output=True,
+    )
+    assert extracted.read_bytes() == replacement.read_bytes()
+
+
+def test_local_attachment_path_can_still_change(temp_file: str, tmp_path: Path) -> None:
+    """An attachment that is not tied to an MKV is still free to point somewhere else."""
+    attachment = MKVAttachment(temp_file)
+    other = tmp_path / "other.bin"
+    other.write_bytes(b"o" * REPLACEMENT_SIZE)
+
+    attachment.file_path = str(other)
+
+    assert attachment.file_path == str(other)
+    assert attachment.size is None
+
+
+def test_attachment_size_invalidated_when_source_id_changes(get_path_test_file: Path, tmp_path: Path) -> None:
+    """source_id selects which embedded attachment is used, so the old size must not survive it."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    attachment = MKVFile(output).attachments[0]
+    assert attachment.size == SMALL_ATTACHMENT_SIZE
+
+    attachment.source_id = 2
+
+    assert attachment.size is None
+
+
+def test_attachment_metadata_kept_when_file_path_reassigned_to_itself(
+    get_path_test_file: Path,
+    tmp_path: Path,
+) -> None:
+    """Assigning the same path is not a mutation and must not discard what was read from the source."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    attachment = MKVFile(output).attachments[0]
+
+    attachment.file_path = attachment.file_path
+
+    assert attachment.size == SMALL_ATTACHMENT_SIZE
+    assert attachment.source_id is not None
+    assert attachment.name == "small.txt"
+
+
+def test_attachment_sizes_with_duplicate_names_and_removal(get_path_test_file: Path, tmp_path: Path) -> None:
+    """Sizes must follow the attachment, not its name or list index, and survive mkvmerge reindexing."""
+    first = tmp_path / "dup" / "шрифт.ttf"
+    first.parent.mkdir()
+    first.write_bytes(b"a" * SMALL_ATTACHMENT_SIZE)
+    second = tmp_path / "шрифт.ttf"
+    second.write_bytes(b"b" * LARGE_ATTACHMENT_SIZE)
+
+    mkv = MKVFile(str(get_path_test_file))
+    mkv.add_attachment(MKVAttachment(str(first)))
+    mkv.add_attachment(MKVAttachment(str(second)))
+    output = str(tmp_path / "dupes.mkv")
+    mkv.mux(output, silent=True)
+
+    loaded = MKVFile(output)
+    assert [a.name for a in loaded.attachments] == ["шрифт.ttf", "шрифт.ttf"]
+    assert [a.size for a in loaded.attachments] == [SMALL_ATTACHMENT_SIZE, LARGE_ATTACHMENT_SIZE]
+
+    loaded.remove_attachment(0)
+    remuxed = str(tmp_path / "dupes_trimmed.mkv")
+    loaded.mux(remuxed, silent=True)
+
+    remaining = MKVFile(remuxed).attachments
+    assert [a.size for a in remaining] == [LARGE_ATTACHMENT_SIZE]
+
+
+def test_attachment_size_after_dropping_source_attachments(get_path_test_file: Path, tmp_path: Path) -> None:
+    """Dropping the inherited attachments and adding a new one leaves only the new size."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    fresh = tmp_path / "fresh.bin"
+    fresh.write_bytes(b"f" * 777)
+
+    mkv = MKVFile(output)
+    mkv.remove_all_attachments()
+    mkv.no_attachments()
+    mkv.add_attachment(MKVAttachment(str(fresh)))
+    trimmed = str(tmp_path / "only_fresh.mkv")
+    mkv.mux(trimmed, silent=True)
+
+    attachments = MKVFile(trimmed).attachments
+    assert [(a.name, a.size) for a in attachments] == [("fresh.bin", 777)]
+
+
+def test_attachment_path_may_be_reassigned_in_another_spelling(get_path_test_file: Path, tmp_path: Path) -> None:
+    """A different spelling of the same file is not a change, so it must not be refused."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    attachment = MKVFile(output).attachments[0]
+    # Path keeps ".." segments verbatim, so this is a different string naming the same file.
+    detour = tmp_path / "detour"
+    detour.mkdir()
+    spelled_differently = str(detour / ".." / Path(output).name)
+    assert spelled_differently != output
+
+    attachment.file_path = spelled_differently
+
+    assert attachment.size == SMALL_ATTACHMENT_SIZE
+    assert attachment.name == "small.txt"
+    assert attachment.source_id is not None
+
+
+def test_attachment_path_reassignment_survives_a_deleted_source(get_path_test_file: Path, tmp_path: Path) -> None:
+    """An identical path is a no-op even when the container is gone, rather than FileNotFoundError."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    attachment = MKVFile(output).attachments[0]
+    Path(output).unlink()
+
+    attachment.file_path = attachment.file_path
+
+    assert attachment.size == SMALL_ATTACHMENT_SIZE
+
+
+def test_attachment_repoint_error_identifies_the_attachment(get_path_test_file: Path, tmp_path: Path) -> None:
+    """Iterating attachments and catching the error must show which one refused."""
+    output, _, _ = _mkv_with_two_attachments(get_path_test_file, tmp_path)
+    other = tmp_path / "other.bin"
+    other.write_bytes(b"o" * REPLACEMENT_SIZE)
+
+    messages = []
+    for attachment in MKVFile(output).attachments:
+        with pytest.raises(ValueError, match="cannot be changed") as excinfo:
+            attachment.file_path = str(other)
+        messages.append(str(excinfo.value))
+
+    assert "small.txt" in messages[0]
+    assert "large.txt" in messages[1]
+    assert messages[0] != messages[1]

tests/test_models.py

@@ -60,7 +60,8 @@ def test_attachment_info_defaults() -> None:
     assert ai.file_name is None
     assert ai.content_type is None
     assert ai.description is None
-    assert ai.size == 0
+    # A missing size stays unknown. mkvmerge refuses to attach an empty file, so 0 would be a fabricated value.
+    assert ai.size is None
     assert isinstance(ai.properties, AttachmentProperties)