diff --git a/CHANGELOG.md b/CHANGELOG.md index ce54d1d2..6e964673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- **MP4**: Padding atoms (`free`) are used when writing + +### Fixed +- **MP4**: `meta` atoms are written correctly + ## [0.5.0] - 2022-02-20 ### Added diff --git a/src/mp4/ilst/mod.rs b/src/mp4/ilst/mod.rs index 8f48e697..0be8e776 100644 --- a/src/mp4/ilst/mod.rs +++ b/src/mp4/ilst/mod.rs @@ -439,8 +439,10 @@ fn item_key_to_ident(key: &ItemKey) -> Option> { #[cfg(test)] mod tests { - use crate::mp4::{AdvisoryRating, Atom, AtomData, AtomIdent, Ilst}; - use crate::{ItemKey, Tag, TagExt, TagType}; + use crate::mp4::{AdvisoryRating, Atom, AtomData, AtomIdent, Ilst, Mp4File}; + use crate::tag_utils::test_utils::read_path; + use crate::{Accessor, AudioFile, ItemKey, Tag, TagExt, TagType}; + use std::io::{Cursor, Read, Seek, SeekFrom, Write}; fn read_ilst(path: &str) -> Ilst { let tag = crate::tag_utils::test_utils::read_path(path); @@ -506,7 +508,7 @@ mod tests { AtomData::UTF8(String::from("Foo title")), )); - let tag = crate::tag_utils::test_utils::read_path("tests/tags/assets/test.ilst"); + let tag = crate::tag_utils::test_utils::read_path("tests/tags/assets/ilst/test.ilst"); let parsed_tag = super::read::parse_ilst(&mut &tag[..], tag.len() as u64).unwrap(); @@ -515,7 +517,7 @@ mod tests { #[test] fn ilst_re_read() { - let parsed_tag = read_ilst("tests/tags/assets/test.ilst"); + let parsed_tag = read_ilst("tests/tags/assets/ilst/test.ilst"); let mut writer = Vec::new(); parsed_tag.dump_to(&mut writer).unwrap(); @@ -529,7 +531,7 @@ mod tests { #[test] fn ilst_to_tag() { - let tag_bytes = crate::tag_utils::test_utils::read_path("tests/tags/assets/test.ilst"); + let tag_bytes = crate::tag_utils::test_utils::read_path("tests/tags/assets/ilst/test.ilst"); let ilst = super::read::parse_ilst(&mut &tag_bytes[..], tag_bytes.len() as u64).unwrap(); @@ -595,7 +597,7 @@ mod tests { #[test] fn issue_34() { - let ilst = read_ilst("tests/tags/assets/issue_34.ilst"); + let ilst = read_ilst("tests/tags/assets/ilst/issue_34.ilst"); verify_atom( &ilst, @@ -614,7 +616,7 @@ mod tests { #[test] fn advisory_rating() { - let ilst = read_ilst("tests/tags/assets/advisory_rating.ilst"); + let ilst = read_ilst("tests/tags/assets/ilst/advisory_rating.ilst"); verify_atom( &ilst, @@ -624,4 +626,53 @@ mod tests { assert_eq!(ilst.advisory_rating(), Some(AdvisoryRating::Explicit)); } + + #[test] + fn trailing_padding() { + const ILST_START: usize = 97; + const ILST_END: usize = 131; + const PADDING_SIZE: usize = 990; + + let file_bytes = read_path("tests/files/assets/ilst_trailing_padding.m4a"); + assert!(Mp4File::read_from(&mut Cursor::new(&file_bytes), false).is_ok()); + + let ilst_bytes = &file_bytes[ILST_START..ILST_END]; + + let old_free_size = + u32::from_be_bytes(file_bytes[ILST_END..ILST_END + 4].try_into().unwrap()); + assert_eq!(old_free_size, PADDING_SIZE as u32); + + let mut ilst = super::read::parse_ilst(&mut &*ilst_bytes, ilst_bytes.len() as u64).unwrap(); + + let mut file = tempfile::tempfile().unwrap(); + file.write_all(&file_bytes).unwrap(); + file.seek(SeekFrom::Start(0)).unwrap(); + + ilst.set_title(String::from("Exactly 21 Characters")); + ilst.save_to(&mut file).unwrap(); + + // Now verify the free atom + file.seek(SeekFrom::Start(0)).unwrap(); + + let mut file_bytes = Vec::new(); + file.read_to_end(&mut file_bytes).unwrap(); + + // 24 (atom + data) + title string (21) + let new_data_size = 24_u32 + 21; + let new_ilst_end = ILST_END + new_data_size as usize; + + let file_atom = &file_bytes[new_ilst_end..new_ilst_end + 8]; + + match file_atom { + [size @ .., b'f', b'r', b'e', b'e'] => assert_eq!( + old_free_size - new_data_size, + u32::from_be_bytes(size.try_into().unwrap()) + ), + _ => unreachable!(), + } + + // Verify we can re-read the file + file.seek(SeekFrom::Start(0)).unwrap(); + assert!(Mp4File::read_from(&mut file, false).is_ok()); + } } diff --git a/src/mp4/ilst/read.rs b/src/mp4/ilst/read.rs index 6a2d59b1..fab6be31 100644 --- a/src/mp4/ilst/read.rs +++ b/src/mp4/ilst/read.rs @@ -172,8 +172,8 @@ fn parse_int(bytes: &[u8]) -> Result { Ok(match bytes.len() { 1 => i32::from(bytes[0]), 2 => i32::from(i16::from_be_bytes([bytes[0], bytes[1]])), - 3 => i32::from_be_bytes([0, bytes[0], bytes[1], bytes[2]]) as i32, - 4 => i32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) as i32, + 3 => i32::from_be_bytes([0, bytes[0], bytes[1], bytes[2]]), + 4 => i32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]), _ => { return Err(LoftyError::new(ErrorKind::BadAtom( "Unexpected atom size for type \"BE signed integer\"", diff --git a/src/mp4/ilst/write.rs b/src/mp4/ilst/write.rs index b395dbc1..1a8b2edd 100644 --- a/src/mp4/ilst/write.rs +++ b/src/mp4/ilst/write.rs @@ -1,8 +1,10 @@ use super::{AtomDataRef, IlstRef}; -use crate::error::{FileEncodingError, Result}; +use crate::error::{ErrorKind, FileEncodingError, LoftyError, Result}; +use crate::macros::try_vec; +use crate::mp4::atom_info::{AtomIdent, AtomInfo}; use crate::mp4::ilst::{AtomIdentRef, AtomRef}; use crate::mp4::moov::Moov; -use crate::mp4::read::{nested_atom, verify_mp4}; +use crate::mp4::read::{atom_tree, nested_atom, verify_mp4}; use crate::types::file::FileType; use crate::types::picture::{MimeType, Picture}; @@ -36,117 +38,79 @@ pub(in crate) fn write_to(data: &mut File, tag: &mut IlstRef<'_>) -> Result<()> } // Total size of new atoms - let new_udta_size; + let mut new_udta_size; // Size of the existing udta atom let mut existing_udta_size = 0; // ilst is nested in udta.meta, so we need to check what atoms actually exist if let Some(udta) = udta { - if let Some(meta) = nested_atom(&mut cursor, udta.len, b"meta")? { - // Skip 4 bytes - // Version (1) - // Flags (3) - cursor.seek(SeekFrom::Current(4))?; + existing_udta_size = udta.len; + new_udta_size = existing_udta_size; - let replacement; - let range; - let existing_ilst_size; + let meta = nested_atom(&mut cursor, udta.len, b"meta")?; + match meta { + Some(meta) => { + // Skip 4 bytes + // Version (1) + // Flags (3) + cursor.seek(SeekFrom::Current(4))?; - let existing_ilst = nested_atom(&mut cursor, meta.len - 4, b"ilst")?; - - match existing_ilst { - Some(existing) => { - replacement = if remove_tag { Vec::new() } else { ilst }; - - range = existing.start as usize..(existing.start + existing.len) as usize; - existing_ilst_size = existing.len as u64; - }, - None => { - // Nothing to do - if remove_tag { - return Ok(()); - } - - let meta_end = (meta.start + meta.len) as usize; - - replacement = ilst; - range = meta_end..meta_end; - existing_ilst_size = 0; - }, - } - - existing_udta_size = udta.len; - - let new_meta_size = (meta.len - existing_ilst_size) + replacement.len() as u64; - new_udta_size = (udta.len - meta.len) + new_meta_size; - - cursor.get_mut().splice(range, replacement); - - cursor.seek(SeekFrom::Start(meta.start))?; - write_size(meta.start, new_meta_size, meta.extended, &mut cursor)?; - - cursor.seek(SeekFrom::Start(udta.start))?; - write_size(udta.start, new_udta_size, udta.extended, &mut cursor)?; - } else { + // We can use the existing `udta` and `meta` atoms + save_to_existing( + &mut cursor, + (meta, udta), + &mut new_udta_size, + ilst, + remove_tag, + )? + }, // Nothing to do - if remove_tag { - return Ok(()); - } + None if remove_tag => return Ok(()), + // We have to create the `meta` atom + None => { + existing_udta_size = udta.len; - existing_udta_size = udta.len; + // `meta` (12) + `ilst` + let capacity = 12 + ilst.len(); + let buf = Vec::with_capacity(capacity); - // `meta` + `ilst` - let capacity = 8 + ilst.len(); - let buf = Vec::with_capacity(capacity); + let mut bytes = Cursor::new(buf); + create_meta(&mut bytes, &ilst)?; - let mut bytes = Cursor::new(buf); - bytes.write_all(&[0, 0, 0, 0, b'm', b'e', b't', b'a'])?; + let bytes = bytes.into_inner(); - write_size(0, ilst.len() as u64 + 8, false, &mut bytes)?; + new_udta_size = udta.len + bytes.len() as u64; - bytes.write_all(&ilst)?; - let bytes = bytes.into_inner(); + cursor.seek(SeekFrom::Start(udta.start))?; + write_size(udta.start, new_udta_size, udta.extended, &mut cursor)?; - new_udta_size = udta.len + bytes.len() as u64; - - cursor.seek(SeekFrom::Start(udta.start))?; - write_size(udta.start, new_udta_size, udta.extended, &mut cursor)?; - - cursor - .get_mut() - .splice(udta.start as usize..udta.start as usize, bytes); + cursor + .get_mut() + .splice(udta.start as usize..udta.start as usize, bytes); + }, } } else { - // `udta` + `meta` + `ilst` - let capacity = 16 + ilst.len(); + // We have to create the `udta` atom + + // `udta` + `meta` (12) + `hdlr` (33) + `ilst` + let capacity = 53 + ilst.len(); let buf = Vec::with_capacity(capacity); let mut bytes = Cursor::new(buf); - bytes.write_all(&[ - 0, 0, 0, 0, b'u', b'd', b't', b'a', 0, 0, 0, 0, b'm', b'e', b't', b'a', - ])?; + bytes.write_all(&[0, 0, 0, 0, b'u', b'd', b't', b'a'])?; + + create_meta(&mut bytes, &ilst)?; // udta size + bytes.seek(SeekFrom::Start(0))?; write_size(0, ilst.len() as u64 + 8, false, &mut bytes)?; - // meta size - write_size( - bytes.seek(SeekFrom::Current(0))?, - ilst.len() as u64, - false, - &mut bytes, - )?; - - bytes.seek(SeekFrom::End(0))?; - bytes.write_all(&ilst)?; - let bytes = bytes.into_inner(); new_udta_size = bytes.len() as u64; - cursor - .get_mut() - .splice((moov.start + 8) as usize..(moov.start + 8) as usize, bytes); + let udta_pos = (moov.start + 8) as usize; + cursor.get_mut().splice(udta_pos..udta_pos, bytes); } cursor.seek(SeekFrom::Start(moov.start))?; @@ -166,6 +130,141 @@ pub(in crate) fn write_to(data: &mut File, tag: &mut IlstRef<'_>) -> Result<()> Ok(()) } +fn save_to_existing( + cursor: &mut Cursor>, + (meta, udta): (AtomInfo, AtomInfo), + new_udta_size: &mut u64, + ilst: Vec, + remove_tag: bool, +) -> Result<()> { + let replacement; + let range; + + let (ilst_idx, tree) = atom_tree(cursor, meta.len - 4, b"ilst")?; + + if tree.is_empty() { + // Nothing to do + if remove_tag { + return Ok(()); + } + + let meta_end = (meta.start + meta.len) as usize; + + replacement = ilst; + range = meta_end..meta_end; + } else { + let existing_ilst = &tree[ilst_idx]; + let existing_ilst_size = existing_ilst.len; + + let mut range_start = existing_ilst.start; + let range_end = existing_ilst.start + existing_ilst_size; + + if remove_tag { + // We just need to strip out the `ilst` atom + + replacement = Vec::new(); + range = range_start as usize..range_end as usize; + } else { + // Check for some padding atoms we can utilize + let mut available_space = existing_ilst_size; + + // Check for one directly before the `ilst` atom + if ilst_idx > 0 { + let previous_atom = &tree[ilst_idx - 1]; + + if previous_atom.ident == AtomIdent::Fourcc(*b"free") { + range_start = previous_atom.start; + available_space += previous_atom.len; + } + } + + // And after + if ilst_idx != tree.len() - 1 { + let next_atom = &tree[ilst_idx + 1]; + + if next_atom.ident == AtomIdent::Fourcc(*b"free") { + available_space += next_atom.len; + } + } + + let ilst_len = ilst.len() as u64; + + // Check if we have enough padding to fit the `ilst` atom and a new `free` atom + if available_space > ilst_len && available_space - ilst_len > 8 { + // We have enough space to make use of the padding + + let remaining_space = available_space - ilst_len; + if remaining_space > u64::from(u32::MAX) { + return Err(LoftyError::new(ErrorKind::TooMuchData)); + } + + let remaining_space = remaining_space as u32; + + cursor.seek(SeekFrom::Start(range_start))?; + cursor.write_all(&ilst)?; + + // Write the remaining padding + cursor.write_u32::(remaining_space)?; + cursor.write_all(b"free")?; + cursor.write_all(&try_vec![1; (remaining_space - 8) as usize])?; + + return Ok(()); + } + + replacement = ilst; + range = range_start as usize..range_end as usize; + } + } + + let new_meta_size = (meta.len - range.len() as u64) + replacement.len() as u64; + + // Replace the `ilst` atom + cursor.get_mut().splice(range, replacement); + + if new_meta_size != meta.len { + // We need to change the `meta` and `udta` atom sizes + + *new_udta_size = (udta.len - meta.len) + new_meta_size; + + cursor.seek(SeekFrom::Start(meta.start))?; + write_size(meta.start, new_meta_size, meta.extended, cursor)?; + + cursor.seek(SeekFrom::Start(udta.start))?; + write_size(udta.start, *new_udta_size, udta.extended, cursor)?; + } + + Ok(()) +} + +fn create_meta(cursor: &mut Cursor>, ilst: &[u8]) -> Result<()> { + const HDLR_SIZE: u64 = 33; + + let start = cursor.stream_position()?; + // meta atom + cursor.write_all(&[0, 0, 0, 0, b'm', b'e', b't', b'a', 0, 0, 0, 0])?; + + // hdlr atom + cursor.write_u32::(0)?; + cursor.write_all(b"hdlr")?; + cursor.write_u64::(0)?; + cursor.write_all(b"mdirappl")?; + cursor.write_all(&[0, 0, 0, 0, 0, 0, 0, 0, 0])?; + + cursor.seek(SeekFrom::Start(start))?; + + let meta_size = 4 + HDLR_SIZE + ilst.len() as u64; + write_size(start, meta_size, false, cursor)?; + + // Seek to `hdlr` size + let hdlr_size_pos = cursor.seek(SeekFrom::Current(4))?; + write_size(hdlr_size_pos, HDLR_SIZE, false, cursor)?; + + cursor.seek(SeekFrom::End(0))?; + cursor.write_all(ilst)?; + + Ok(()) +} + fn write_size(start: u64, size: u64, extended: bool, writer: &mut Cursor>) -> Result<()> { if size > u64::from(u32::MAX) { // 0001 (identifier) ???????? diff --git a/src/mp4/read.rs b/src/mp4/read.rs index a3717b7d..37c95596 100644 --- a/src/mp4/read.rs +++ b/src/mp4/read.rs @@ -98,3 +98,36 @@ where Ok(ret) } + +// Creates a tree of nested atoms +pub(crate) fn atom_tree(data: &mut R, len: u64, up_to: &[u8]) -> Result<(usize, Vec)> +where + R: Read + Seek, +{ + let mut read = 8; + let mut found_idx: usize = 0; + let mut buf = Vec::new(); + + let mut i = 0; + + while read < len { + let atom = AtomInfo::read(data)?; + + skip_unneeded(data, atom.extended, atom.len)?; + read += atom.len; + + if let AtomIdent::Fourcc(ref fourcc) = atom.ident { + i += 1; + + if fourcc == up_to { + found_idx = i; + } + + buf.push(atom); + } + } + + found_idx = found_idx.saturating_sub(1); + + Ok((found_idx, buf)) +} diff --git a/tests/files/assets/ilst_trailing_padding.m4a b/tests/files/assets/ilst_trailing_padding.m4a new file mode 100644 index 00000000..9170875b Binary files /dev/null and b/tests/files/assets/ilst_trailing_padding.m4a differ diff --git a/tests/files/mp4.rs b/tests/files/mp4.rs index 8dfc6fc7..df3bc406 100644 --- a/tests/files/mp4.rs +++ b/tests/files/mp4.rs @@ -27,6 +27,7 @@ fn write() { // Now reread the file file.seek(SeekFrom::Start(0)).unwrap(); + let mut tagged_file = lofty::read_from(&mut file, false).unwrap(); crate::set_artist!(tagged_file, tag_mut, TagType::Mp4Ilst, "Bar artist", 1 => file, "Foo artist"); diff --git a/tests/tags/assets/advisory_rating.ilst b/tests/tags/assets/ilst/advisory_rating.ilst similarity index 100% rename from tests/tags/assets/advisory_rating.ilst rename to tests/tags/assets/ilst/advisory_rating.ilst diff --git a/tests/tags/assets/issue_34.ilst b/tests/tags/assets/ilst/issue_34.ilst similarity index 100% rename from tests/tags/assets/issue_34.ilst rename to tests/tags/assets/ilst/issue_34.ilst diff --git a/tests/tags/assets/test.ilst b/tests/tags/assets/ilst/test.ilst similarity index 100% rename from tests/tags/assets/test.ilst rename to tests/tags/assets/ilst/test.ilst