Handle non-full meta atoms

CHANGELOG.md

@@ -6,9 +6,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- **MP4**: Non-full `meta` atoms are now properly handled.
+  - It is possible for these to be a regular atom (no version or flags).
+    This information was assumed to be present and would get skipped,
+    which would affect the reading of subsequent atoms.
+  
+    This behavior has been noticed by:
+    - https://leo-van-stee.github.io/
+    - https://github.com/axiomatic-systems/Bento4/blob/v1.6.0-639/Source/C%2B%2B/Core/Ap4ContainerAtom.cpp#L60
+    - https://github.com/taglib/taglib/issues/1041
+
 ## [0.5.3] - 2022-03-03
 
-## Fixed
+### Fixed
 - **OGG**: Segment tables are written correctly with data spanning multiple pages ([issue](https://github.com/Serial-ATA/lofty-rs/issues/37))
 
 ## [0.5.2] - 2022-02-26

src/mp4/ilst/mod.rs

@@ -691,4 +691,41 @@ mod tests {
 		file.seek(SeekFrom::Start(0)).unwrap();
 		assert!(Mp4File::read_from(&mut file, false).is_ok());
 	}
+
+	#[test]
+	fn read_non_full_meta_atom() {
+		let file_bytes = read_path("tests/files/assets/non_full_meta_atom.m4a");
+		let file = Mp4File::read_from(&mut Cursor::new(file_bytes), false).unwrap();
+
+		assert!(file.ilst.is_some());
+	}
+
+	#[test]
+	fn write_non_full_meta_atom() {
+		// This is testing writing to a file with a non-full meta atom
+		// We will *not* write a non-full meta atom
+
+		let file_bytes = read_path("tests/files/assets/non_full_meta_atom.m4a");
+		let mut file = tempfile::tempfile().unwrap();
+		file.write_all(&file_bytes).unwrap();
+		file.seek(SeekFrom::Start(0)).unwrap();
+
+		let mut tag = Ilst::default();
+		tag.insert_atom(Atom {
+			ident: AtomIdent::Fourcc(*b"\xa9ART"),
+			data: AtomData::UTF8(String::from("Foo artist")),
+		});
+
+		assert!(tag.save_to(&mut file).is_ok());
+		file.seek(SeekFrom::Start(0)).unwrap();
+
+		let mp4_file = Mp4File::read_from(&mut file, true).unwrap();
+		assert!(mp4_file.ilst.is_some());
+
+		verify_atom(
+			&mp4_file.ilst.unwrap(),
+			*b"\xa9ART",
+			&AtomData::UTF8(String::from("Foo artist")),
+		);
+	}
 }

src/mp4/ilst/write.rs

@@ -5,7 +5,7 @@ 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::{atom_tree, nested_atom, verify_mp4};
+use crate::mp4::read::{atom_tree, meta_is_full, nested_atom, verify_mp4};
 use crate::picture::{MimeType, Picture};
 
 use std::fs::File;
@@ -50,10 +50,8 @@ pub(in crate) fn write_to(data: &mut File, tag: &mut IlstRef<'_>) -> Result<()>
 		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))?;
+				// We may encounter a non-full `meta` atom
+				meta_is_full(&mut cursor)?;
 
 				// We can use the existing `udta` and `meta` atoms
 				save_to_existing(

src/mp4/moov.rs

@@ -5,12 +5,10 @@ use super::read::skip_unneeded;
 use super::trak::Trak;
 use crate::error::{FileDecodingError, Result};
 use crate::file::FileType;
+use crate::mp4::read::meta_is_full;
 
 use std::io::{Read, Seek};
 
-#[cfg(feature = "mp4_ilst")]
-use byteorder::{BigEndian, ReadBytesExt};
-
 pub(crate) struct Moov {
 	pub(crate) traks: Vec<Trak>,
 	#[cfg(feature = "mp4_ilst")]
@@ -98,12 +96,16 @@ where
 		return Ok(None);
 	}
 
-	// The meta atom has 4 bytes we don't care about
-	// Version (1)
-	// Flags (3)
-	let _version_flags = data.read_u32::<BigEndian>()?;
+	// It's possible for the `meta` atom to be non-full,
+	// so we have to check for that case
+	let full_meta_atom = meta_is_full(data)?;
+
+	if full_meta_atom {
+		read = 12;
+	} else {
+		read = 8;
+	}
 
-	read = 12;
 	let mut islt = (false, 0_u64);
 
 	while read < meta.1 {

src/mp4/read.rs

@@ -7,6 +7,8 @@ use crate::file::FileType;
 
 use std::io::{Read, Seek, SeekFrom};
 
+use byteorder::{BigEndian, ReadBytesExt};
+
 pub(in crate::mp4) fn verify_mp4<R>(data: &mut R) -> Result<String>
 where
 	R: Read + Seek,
@@ -55,7 +57,7 @@ where
 	})
 }
 
-pub(crate) fn skip_unneeded<R>(data: &mut R, ext: bool, len: u64) -> Result<()>
+pub(super) fn skip_unneeded<R>(data: &mut R, ext: bool, len: u64) -> Result<()>
 where
 	R: Read + Seek,
 {
@@ -74,7 +76,7 @@ where
 	Ok(())
 }
 
-pub(crate) fn nested_atom<R>(data: &mut R, len: u64, expected: &[u8]) -> Result<Option<AtomInfo>>
+pub(super) fn nested_atom<R>(data: &mut R, len: u64, expected: &[u8]) -> Result<Option<AtomInfo>>
 where
 	R: Read + Seek,
 {
@@ -100,7 +102,7 @@ where
 }
 
 // Creates a tree of nested atoms
-pub(crate) fn atom_tree<R>(data: &mut R, len: u64, up_to: &[u8]) -> Result<(usize, Vec<AtomInfo>)>
+pub(super) fn atom_tree<R>(data: &mut R, len: u64, up_to: &[u8]) -> Result<(usize, Vec<AtomInfo>)>
 where
 	R: Read + Seek,
 {
@@ -131,3 +133,34 @@ where
 
 	Ok((found_idx, buf))
 }
+
+pub(super) fn meta_is_full<R>(data: &mut R) -> Result<bool>
+where
+	R: Read + Seek,
+{
+	let meta_pos = data.stream_position()?;
+
+	// A full `meta` atom should have the following:
+	//
+	// Version (1)
+	// Flags (3)
+	//
+	// However, it's possible that it is written as a normal atom,
+	// meaning this would be the size of the next atom.
+	let _version_flags = data.read_u32::<BigEndian>()?;
+
+	// Check if the next four bytes is one of the nested `meta` atoms
+	let mut possible_ident = [0; 4];
+	data.read_exact(&mut possible_ident)?;
+
+	match &possible_ident {
+		b"hdlr" | b"ilst" | b"mhdr" | b"ctry" | b"lang" => {
+			data.seek(SeekFrom::Start(meta_pos))?;
+			Ok(false)
+		},
+		_ => {
+			data.seek(SeekFrom::Start(meta_pos + 4))?;
+			Ok(true)
+		},
+	}
+}