Serial-ATA / lofty-rs

Audio metadata library
Apache License 2.0
187 stars 35 forks source link

Writing m4a tags seems to render the file unplayable in some cases #308

Closed milesegan closed 9 months ago

milesegan commented 9 months ago

Reproducer

use std::{
    fs::{copy, remove_file},
    path::Path,
    process::Command,
};

use lofty::{Accessor, Probe, Tag, TagExt, TagType, TaggedFileExt};

pub fn write_tag(dest: &Path) {
    let mut dest_file = Probe::open(&dest)
        .expect("can't open")
        .read()
        .expect("can't read");
    let tag = Tag::new(TagType::Mp4Ilst);
    let mut dest_tag = dest_file.insert_tag(tag).expect("no tag");
    dest_tag.set_artist("test artist".to_string());
    dest_tag.save_to_path(dest).expect("can't write");
}

fn main() {
    let _ = remove_file("encoded.m4a");
    let _ = remove_file("tagged.m4a");
    Command::new("afconvert")
        .args(["-d", "aac", "-f", "m4af", "test.flac", "encoded.m4a"])
        .output()
        .expect("couldn't encode file");
    println!("encoded to encoded.m4a");
    copy("encoded.m4a", "tagged.m4a").expect("copy failed");
    println!("copied to tagged.m4a");
    write_tag(Path::new("tagged.m4a"));
    println!("wrote tag to tagged.m4a");
}

Summary

Writing a new tag to an m4a file seems to corrupt the file somehow and make it unplayable.

Running afplay test.m4a on the file generated above reports this error:

Error: AudioQueueStart failed ('bada')

And the file is also unplayable in the finder.

Note that this only seems to happen with m4a files created with afconvert. If I use ffmpeg to transcode the flac file to m4a and then write the tag with lofty the resulting file is playable.

Expected behavior

The file should still be a playable m4a file after writing the tag.

Assets

https://github.com/milesegan/lofty-m4a-bug

Serial-ATA commented 9 months ago

Thanks for the report and all the information. Could you also provide the file created with afconvert prior to running it through Lofty? I do not have access to afconvert (I think?).

milesegan commented 9 months ago

Sorry about that. I've updated the repo to include the file before tagging and I'm also attaching it here. I believe afconvert is part of the OS on Macs but that's the only way to get access to it.

Thanks for all your work on lofty, by the way!

encoded.m4a.zip

Serial-ATA commented 9 months ago

Awesome, thanks!

I'm pretty sure this is just caused by the offset atoms, which need to be updated anytime the position of atoms change. I forgot they existed, since I don't have any files using them in my library. I'll try and get that fixed in the next couple days.

milesegan commented 9 months ago

Thanks for your quick attention on this!

This year I'm going to try to help you out with some of the good first issue issues.

Serial-ATA commented 9 months ago

Could you try out #318 and see if it fixes the issue for you?

milesegan commented 9 months ago

Yep it seems to be working now. Thanks for the quick fix!