Skip to content

Commit e622a67

Browse files
committed
MP4: Merge existing atoms in Ilst::insert
Now, when inserting an atom that already exists in a tag, the data is taken from the provided atom and merged with the existing value in the tag. This should ensure that the value that already exists in the tag will remain the dominant one.
1 parent acc3c9f commit e622a67

File tree

2 files changed

+32
-2
lines changed

2 files changed

+32
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
- **MP4**:
2323
- `Ilst::remove` will now return all of the removed atoms
2424
- `Ilst::insert_picture` will now combine all pictures into a single `covr` atom
25+
- `Ilst::insert` will now merge atoms with the same identifier into a single atom
2526

2627
## [0.15.0] - 2023-07-11
2728

src/mp4/ilst/mod.rs

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ impl Ilst {
125125

126126
/// Inserts an [`Atom`]
127127
///
128+
/// NOTE: Do not use this to replace atoms. This will take the value from the provided atom and
129+
/// merge it into an atom of the same type, keeping any existing value(s). To ensure an atom
130+
/// is replaced, use [`Ilst::replace_atom`].
131+
///
128132
/// # Examples
129133
///
130134
/// ```rust
@@ -142,7 +146,8 @@ impl Ilst {
142146
/// let title = ilst.get(&TITLE_IDENTIFIER);
143147
/// assert!(title.is_some());
144148
/// ```
145-
pub fn insert(&mut self, atom: Atom<'_>) {
149+
#[allow(clippy::missing_panics_doc)] // Unwrap on an infallible
150+
pub fn insert(&mut self, atom: Atom<'static>) {
146151
if atom.ident == COVR && atom.data.is_pictures() {
147152
for data in atom.data {
148153
match data {
@@ -153,7 +158,14 @@ impl Ilst {
153158
return;
154159
}
155160

156-
self.atoms.push(atom.into_owned());
161+
if let Some(existing) = self.get_mut(atom.ident()) {
162+
existing.merge(atom).expect(
163+
"Somehow the atom merge condition failed, despite the validation beforehand.",
164+
);
165+
return;
166+
}
167+
168+
self.atoms.push(atom);
157169
}
158170

159171
/// Inserts an [`Atom`], replacing any atom with the same [`AtomIdent`]
@@ -752,6 +764,7 @@ impl From<Tag> for Ilst {
752764
#[cfg(test)]
753765
mod tests {
754766
use crate::mp4::ilst::atom::AtomDataStorage;
767+
use crate::mp4::ilst::TITLE;
755768
use crate::mp4::read::AtomReader;
756769
use crate::mp4::{AdvisoryRating, Atom, AtomData, AtomIdent, Ilst, Mp4File};
757770
use crate::tag::utils::test_utils;
@@ -1156,4 +1169,20 @@ mod tests {
11561169

11571170
assert_eq!(file.ilst(), Some(&Ilst::default()));
11581171
}
1172+
1173+
#[test]
1174+
fn merge_insert() {
1175+
let mut ilst = Ilst::new();
1176+
1177+
// Insert two titles
1178+
ilst.set_title(String::from("Foo"));
1179+
ilst.insert(Atom::new(TITLE, AtomData::UTF8(String::from("Bar"))));
1180+
1181+
// Title should still be the first value, but there should be two total values
1182+
assert_eq!(ilst.title().as_deref(), Some("Foo"));
1183+
assert_eq!(ilst.get(&TITLE).unwrap().data().count(), 2);
1184+
1185+
// Meaning we only have 1 atom
1186+
assert_eq!(ilst.len(), 1);
1187+
}
11591188
}

0 commit comments

Comments
 (0)