MP4: Correctness fixes

CHANGELOG.md

@@ -9,13 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Added
 - **ID3v2**: Support for "RVA2", "OWNE", "ETCO", and "PRIV" frames through
              `id3::v2::{RelativeVolumeAdjustmentFrame, OwnershipFrame, EventTimingCodesFrame, PrivateFrame}`
+- **MP4**:
+  - `Atom::into_data`
+  - `Atom::merge`
 
 ## Changed
 - **ID3v2**:
   - For spec compliance, `Id3v2Tag::insert` will now check for frames that are only meant to appear
     in a tag once and remove them. Those frames are: "MCDI", "ETCO", "MLLT", "SYTC", "RVRB", "PCNT", "RBUF", "POSS", "OWNE", "SEEK", and "ASPI".
   - `Id3v2Tag::remove` will now take a `FrameId` rather than `&str`
   - `FrameId` now implements `Into<Cow<'_, str>>`, making it possible to use it in `Frame::new`
+- **MP4**:
+  - `Ilst::remove` will now return all of the removed atoms
+  - `Ilst::insert_picture` will now combine all pictures into a single `covr` atom
+  - `Ilst::insert` will now merge atoms with the same identifier into a single atom
 
 ## [0.15.0] - 2023-07-11
 

src/error.rs

@@ -53,6 +53,8 @@ pub enum ErrorKind {
 
 	/// Arises when an atom contains invalid data
 	BadAtom(&'static str),
+	/// Arises when attempting to use [`Atom::merge`](crate::mp4::Atom::merge) with mismatching identifiers
+	AtomMismatch,
 
 	// Conversions for external errors
 	/// Errors that arise while parsing OGG pages
@@ -520,6 +522,10 @@ impl Display for LoftyError {
 			ErrorKind::TextDecode(message) => write!(f, "Text decoding: {message}"),
 			ErrorKind::Id3v2(ref id3v2_err) => write!(f, "{id3v2_err}"),
 			ErrorKind::BadAtom(message) => write!(f, "MP4 Atom: {message}"),
+			ErrorKind::AtomMismatch => write!(
+				f,
+				"MP4 Atom: Attempted to use `Atom::merge()` with mismatching identifiers"
+			),
 
 			// Files
 			ErrorKind::TooMuchData => write!(

src/mp4/ilst/atom.rs

@@ -1,3 +1,5 @@
+use crate::error::Result;
+use crate::macros::err;
 use crate::mp4::AtomIdent;
 use crate::picture::Picture;
 
@@ -18,6 +20,13 @@ impl AtomDataStorage {
 			AtomDataStorage::Multiple(data) => data.first_mut().expect("not empty"),
 		}
 	}
+
+	pub(super) fn is_pictures(&self) -> bool {
+		match self {
+			AtomDataStorage::Single(v) => matches!(v, AtomData::Picture(_)),
+			AtomDataStorage::Multiple(v) => v.iter().all(|p| matches!(p, AtomData::Picture(_))),
+		}
+	}
 }
 
 impl Debug for AtomDataStorage {
@@ -29,6 +38,18 @@ impl Debug for AtomDataStorage {
 	}
 }
 
+impl IntoIterator for AtomDataStorage {
+	type Item = AtomData;
+	type IntoIter = std::vec::IntoIter<Self::Item>;
+
+	fn into_iter(self) -> Self::IntoIter {
+		match self {
+			AtomDataStorage::Single(s) => vec![s].into_iter(),
+			AtomDataStorage::Multiple(v) => v.into_iter(),
+		}
+	}
+}
+
 impl<'a> IntoIterator for &'a AtomDataStorage {
 	type Item = &'a AtomData;
 	type IntoIter = AtomDataStorageIter<'a>;
@@ -118,6 +139,23 @@ impl<'a> Atom<'a> {
 		(&self.data).into_iter()
 	}
 
+	/// Consumes the atom, returning its [`AtomData`]
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::mp4::{Atom, AtomData, AtomIdent};
+	///
+	/// let atom = Atom::new(
+	/// 	AtomIdent::Fourcc(*b"\x49ART"),
+	/// 	AtomData::UTF8(String::from("Foo")),
+	/// );
+	/// assert_eq!(atom.into_data().count(), 1);
+	/// ```
+	pub fn into_data(self) -> impl Iterator<Item = AtomData> {
+		self.data.into_iter()
+	}
+
 	/// Append a value to the atom
 	pub fn push_data(&mut self, data: AtomData) {
 		match self.data {
@@ -128,6 +166,49 @@ impl<'a> Atom<'a> {
 		}
 	}
 
+	/// Merge the data of another atom into this one
+	///
+	/// NOTE: Both atoms must have the same identifier
+	///
+	/// # Errors
+	///
+	/// * `self.ident()` != `other.ident()`
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::mp4::{Atom, AtomData, AtomIdent};
+	///
+	/// # fn main() -> lofty::Result<()> {
+	/// // Create an artist atom
+	/// let mut atom = Atom::new(
+	/// 	AtomIdent::Fourcc(*b"\x49ART"),
+	/// 	AtomData::UTF8(String::from("foo")),
+	/// );
+	///
+	/// // Create a second and merge it into the first
+	/// let atom2 = Atom::new(
+	/// 	AtomIdent::Fourcc(*b"\x49ART"),
+	/// 	AtomData::UTF8(String::from("bar")),
+	/// );
+	/// atom.merge(atom2)?;
+	///
+	/// // Our first atom now contains the second atom's content
+	/// assert_eq!(atom.data().count(), 2);
+	/// # Ok(()) }
+	/// ```
+	pub fn merge(&mut self, other: Atom<'_>) -> Result<()> {
+		if self.ident != other.ident {
+			err!(AtomMismatch);
+		}
+
+		for data in other.data {
+			self.push_data(data)
+		}
+
+		Ok(())
+	}
+
 	// Used internally, has no correctness checks
 	pub(crate) fn unknown_implicit(ident: AtomIdent<'_>, data: Vec<u8>) -> Self {
 		Self {
@@ -248,7 +329,7 @@ impl AdvisoryRating {
 impl TryFrom<u8> for AdvisoryRating {
 	type Error = u8;
 
-	fn try_from(input: u8) -> Result<Self, Self::Error> {
+	fn try_from(input: u8) -> std::result::Result<Self, Self::Error> {
 		match input {
 			0 => Ok(Self::Inoffensive),
 			1 | 4 => Ok(Self::Explicit),