Skip to content

fix(mp4): preserve hdlr atom when writing new tags to metadata containers with missing ilst#660

Open
axel10 wants to merge 4 commits into
Serial-ATA:mainfrom
axel10:fix-mp4
Open

fix(mp4): preserve hdlr atom when writing new tags to metadata containers with missing ilst#660
axel10 wants to merge 4 commits into
Serial-ATA:mainfrom
axel10:fix-mp4

Conversation

@axel10
Copy link
Copy Markdown

@axel10 axel10 commented May 25, 2026

Description

This PR fixes a bug where writing new tags (Mp4Ilst) to MP4/M4A files that contain a meta box but no ilst box (e.g., after tag removal or clean metadata copy) would overwrite and destroy the hdlr (Handler Reference) atom. This resulted in malformed M4A files that standard-conforming media players (like FFmpeg, Windows Media Foundation/Groove, CoreMedia, etc.) could no longer read metadata from.

Root Cause

In lofty/src/mp4/read/mod.rs, atom_tree returned (usize, Vec<AtomInfo>) where the first element was the index of the matched atom. If the atom (e.g., ilst) was not found, it would return 0. If it was found at index 0 (which is valid), it would also return 0.
In lofty/src/mp4/ilst/write.rs, save_to_existing relied on this index. When ilst was deleted/absent, atom_tree returned 0 for the index, while tree still had child atoms (specifically, hdlr at index 0). The writer then incorrectly assumed tree[0] (the hdlr atom) was the existing ilst and replaced the hdlr atom with the new ilst atom.

Solution

  1. Modified atom_tree: The returned index is now an Option<usize>. It returns Some(index) if found, and None if not found, allowing callers to accurately distinguish whether ilst exists.
  2. Updated save_to_existing: It now checks if ilst_idx is None (via if let Some(ilst_idx) = ilst_idx). If it is None, it appends ilst to the end of meta rather than overwriting tree[0]. If it is Some(idx), it replaces the existing ilst atom in place as before. This also resolves a Clippy lint for unnecessary_unwrap.

Regression Test

An integration test hdlr_preservation_on_tag_recreation has been added to lofty/tests/files/mp4.rs. It simulates the exact tag recreation sequence:

  1. Copies a minimal test M4A (m4a_codec_aac.m4a).
  2. Deletes the tag container with remove_from_path.
  3. Inserts and saves a new tag with save_to_path.
  4. Asserts that the output file still contains the hdlr atom.
    All library unit and integration tests pass successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant