Mp4: Chunk offsets are not updated when the `udta`/`meta` atoms have to be created, corrupting faststart files

[jfietz]

Disclosure, per the note in CONTRIBUTING.md: I ran into this on my own media library and worked out the symptom myself, but I used AI for narrowing it down to these specific branches. PR (coming soon) and the tests were done with AI assistance, and I have reviewed and reproduced all of the results above. Happy to adjust the approach or trim the tests if you would prefer something smaller.

---

Reproducer

I tried this code (examples created with Claude). This creates the mp4 in memory, but I'm also attaching a test file that breaks when adding ilst tags.

//! Standalone reproducer: MP4 chunk offsets are not updated when lofty has to
//! create the `udta`/`meta` atoms.
//!
//! Builds a minimal MP4 in memory with `moov` placed *before* `mdat` (the
//! layout `-movflags +faststart` produces) and a single `stco` entry pointing
//! at the start of the `mdat` payload. Writing a tag grows `moov`, which pushes
//! `mdat` later in the file -- but the chunk offset is left pointing at the old
//! location.
//!
//! Cargo.toml needs only `lofty = "0.24.0"`. No external files or tools
//! required; run with `cargo run`.

use lofty::config::{ParseOptions, WriteOptions};
use lofty::file::{AudioFile, TaggedFileExt};
use lofty::probe::Probe;
use lofty::tag::{Accessor, Tag};

/// Wraps `payload` in an MP4 box.
fn atom(fourcc: &[u8; 4], payload: &[u8]) -> Vec<u8> {
    let mut out = ((payload.len() + 8) as u32).to_be_bytes().to_vec();
    out.extend_from_slice(fourcc);
    out.extend_from_slice(payload);
    out
}

/// A minimal MP4: ftyp, then moov (holding one stco), then mdat.
///
/// `include_udta` selects which of the two affected branches is taken:
/// `false` -> "No `udta` atom found, creating one"
/// `true`  -> "No `meta` atom found, creating one"
fn build_faststart_mp4(include_udta: bool) -> Vec<u8> {
    const MDAT_PAYLOAD: &[u8] = b"pretend this is a video sample";

    let mut stco_payload = vec![0u8; 4]; // version + flags
    stco_payload.extend_from_slice(&1u32.to_be_bytes()); // entry count
    stco_payload.extend_from_slice(&0u32.to_be_bytes()); // offset, patched below

    let stbl = atom(b"stbl", &atom(b"stco", &stco_payload));
    let trak = atom(b"trak", &atom(b"mdia", &atom(b"minf", &stbl)));

    let mut moov_payload = trak;
    if include_udta {
        moov_payload.extend_from_slice(&atom(b"udta", &[]));
    }

    let mut file = atom(b"ftyp", b"isom\x00\x00\x02\x00isomiso2");
    file.extend_from_slice(&atom(b"moov", &moov_payload));

    let mdat_payload_start = file.len() + 8;
    file.extend_from_slice(&atom(b"mdat", MDAT_PAYLOAD));

    let entry = find(&file, b"stco") + 12;
    file[entry..entry + 4].copy_from_slice(&(mdat_payload_start as u32).to_be_bytes());

    file
}

fn find(bytes: &[u8], fourcc: &[u8; 4]) -> usize {
    bytes
        .windows(4)
        .position(|w| w == fourcc)
        .unwrap_or_else(|| panic!("`{}` not found", String::from_utf8_lossy(fourcc)))
}

/// Returns (single stco entry, actual offset of the mdat payload).
fn probe_layout(bytes: &[u8]) -> (u32, u32) {
    let entry = find(bytes, b"stco") + 12;
    let stco = u32::from_be_bytes(bytes[entry..entry + 4].try_into().unwrap());
    let mdat_payload_start = (find(bytes, b"mdat") + 4) as u32;
    (stco, mdat_payload_start)
}

fn run_case(label: &str, include_udta: bool) -> bool {
    let original = build_faststart_mp4(include_udta);
    let (before_stco, before_mdat) = probe_layout(&original);

    let path = std::env::temp_dir().join(format!("lofty-repro-{}.mp4", include_udta));
    std::fs::write(&path, &original).unwrap();

    let mut tagged_file = Probe::open(&path)
        .unwrap()
        .options(ParseOptions::new().read_properties(false))
        .read()
        .unwrap();

    let tag = match tagged_file.primary_tag_mut() {
        Some(tag) => tag,
        None => {
            let tag_type = tagged_file.file_type().primary_tag_type();
            tagged_file.insert_tag(Tag::new(tag_type));
            tagged_file.primary_tag_mut().unwrap()
        }
    };

    tag.set_genre(String::from("documentary"));
    tagged_file
        .save_to_path(&path, WriteOptions::default())
        .unwrap();

    let written = std::fs::read(&path).unwrap();
    let (after_stco, after_mdat) = probe_layout(&written);

    let media_shift = after_mdat as i64 - before_mdat as i64;
    let offset_shift = after_stco as i64 - before_stco as i64;
    let ok = after_stco == after_mdat;

    println!("{label}");
    println!("  mdat payload : {before_mdat} -> {after_mdat}  ({media_shift:+})");
    println!("  stco entry   : {before_stco} -> {after_stco}  ({offset_shift:+})");
    println!(
        "  result       : {}\n",
        if ok {
            "OK - chunk offset still points at the media data".to_string()
        } else {
            format!(
                "BROKEN - chunk offset is stale by {} bytes",
                media_shift - offset_shift
            )
        }
    );

    let _ = std::fs::remove_file(&path);
    ok
}

fn main() {
    println!("minimal faststart MP4, one stco entry\n");

    let a = run_case("case A: moov has no `udta` (create udta path)", false);
    let b = run_case("case B: moov has an empty `udta` (create meta path)", true);

    if a && b {
        println!("both cases OK");
    } else {
        println!("at least one case is broken -- media data moved, chunk offsets did not");
        std::process::exit(1);
    }
}

which prints, on 0.24.0 and on bcea39d:

minimal faststart MP4, one stco entry

case A: moov has no `udta` (create udta path)
  mdat payload : 92 -> 188  (+96)
  stco entry   : 92 -> 92  (+0)
  result       : BROKEN - chunk offset is stale by 96 bytes

case B: moov has an empty `udta` (create meta path)
  mdat payload : 100 -> 188  (+88)
  stco entry   : 100 -> 100  (+0)
  result       : BROKEN - chunk offset is stale by 88 bytes

at least one case is broken -- media data moved, chunk offsets did not

The same thing happens on real files. Taking any -movflags +faststart MP4, stripping moov/udta, and writing a tag leaves the file undecodable:

$ ffmpeg -v error -i out.mp4 -f null -
[h264 @ 0x…] Invalid NAL unit size (18863 > 3845).
[h264 @ 0x…] missing picture in access unit with size 3849
…

### Summary

When writing an `Ilst` to an MP4 whose `moov` atom precedes `mdat` (anything muxed with `-movflags +faststart`), and whose `moov` does **not** already contain a `udta`/`meta` pair, lofty inserts the new tag atoms into `moov` without updating the `stco`/`co64` chunk offset tables. The inserted bytes push `mdat` later in the file while every chunk offset keeps pointing at the old location. The tags read back correctly, but the media stream no longer decodes.

`update_offsets` is only called from `save_to_existing`. The two branches in `write_to` that *create* the tag container return without calling it:

```rust
// "No `udta` atom found, creating one"
let bytes = create_udta(&ilst, write_options)?;
new_udta_size = bytes.len() as u64;

// We'll put the new `udta` atom right at the start of `moov`
let udta_pos = (moov_start + ATOM_HEADER_LEN) as usize;
write_handle.splice(udta_pos..udta_pos, bytes);   // <- offsets never updated

drop(write_handle);

and likewise the None => ("We have to create the meta atom") arm above it.

Trigger conditions, all confirmed against bcea39d:

moov layout	result
no moov/udta	corrupted
udta present but empty	corrupted
udta present with only non-meta children	corrupted
udta/meta/hdlr/ilst already present	fine (save_to_existing updates offsets)
mdat before moov	fine (nothing after moov to shift)

So the condition is "whenever lofty has to create the meta box". Files that already carry an iTunes-style tag take the working path, which is probably why this has gone unnoticed. Reproduces identically on 0.22.4 and 0.24.0.

Calling update_offsets from both creation branches, before the splice (while the offset atoms are still at their pre-splice positions), fixes every case above with the existing test suite still passing:

 		// We'll put the new `udta` atom right at the start of `moov`
 		let udta_pos = (moov_start + ATOM_HEADER_LEN) as usize;
+
+		drop(write_handle);
+		update_offsets(&atom_writer, moov, bytes.len() as i64, udta_pos as u64)?;
+		let mut write_handle = atom_writer.start_write();
+
 		write_handle.splice(udta_pos..udta_pos, bytes);

With that applied, the program above prints both cases OK. I have a branch with this change plus regression tests and can open a PR referencing this issue.

While testing the above I also ran into this, which may deserve a separate issue: for a file with a QuickTime-style non-full meta atom (no version/flags), save_to_path succeeds and reports no error, but no tags are written at all — reading the file back afterwards shows none of the atoms that were set. The file is left structurally intact, so this one is a silent no-op rather than corruption.

Expected behavior

The chunk offsets should be shifted by the same amount as the media data, exactly as save_to_existing already does through update_offsets, leaving the file decodable.

Actual behavior: mdat moves, stco/co64 do not, and every sample offset is stale by the size of the inserted atoms. The sample data itself is never touched, so affected files are losslessly repairable by adding the shift back onto each chunk offset — but only if you can work out what the shift was.

Assets

The reproducer generates its own asset in memory, so there is no copyrighted content involved and nothing that needs emailing, but I did create this sample file that is broken by the tag edit: https://github.com/user-attachments/assets/903cf411-f0cc-4962-8716-04c28dc9227b

The regression tests on my branch use a real H.264/AAC MP4 (42 KB) checked in as a test asset, since the current implementation demonstrably corrupts it, plus two minimal hand-built MP4s covering both creation paths. That asset was produced from ffmpeg's synthetic testsrc/sine sources:

ffmpeg -f lavfi -i testsrc=size=320x240:rate=10:duration=3 \
       -f lavfi -i sine=frequency=440:duration=3 \
       -c:v libx264 -c:a aac -map_metadata -1 -movflags +faststart out.mp4

with moov/udta then removed and every stco entry decremented by its size, so the file starts out consistent.