ID3v2: Support duplicate tags

CHANGELOG.md

@@ -27,6 +27,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Accessor**: All methods returning string values now return `Cow<str>`.
   - This is an unfortunate change that needed to be made in order to accommodate the handling of the different
     possible text separators between ID3v2 versions.
+- **ID3v2**: Support reading of duplicate tags
+  - Previously, if we were reading a file and encountered an ID3v2 tag after having already read one,
+    we would overwrite the last one, losing all of its information. Now we preserve all of the information,
+    overwriting frames as necessary.
 
 ### Removed
 - **ogg_pager**: Removed `Page::new`, now pages can only be created through `ogg_pager::paginate` or

src/aac/read.rs

@@ -51,6 +51,14 @@ where
 				#[cfg(feature = "id3v2")]
 				{
 					let id3v2 = parse_id3v2(reader, header)?;
+					if let Some(existing_tag) = &mut file.id3v2_tag {
+						// https://github.com/Serial-ATA/lofty-rs/issues/87
+						// Duplicate tags should have their frames appended to the previous
+						for frame in id3v2.frames {
+							existing_tag.insert(frame);
+						}
+						continue;
+					}
 					file.id3v2_tag = Some(id3v2);
 				}
 

src/id3/v2/tag.rs

@@ -89,7 +89,7 @@ macro_rules! impl_accessor {
 pub struct ID3v2Tag {
 	flags: ID3v2TagFlags,
 	pub(super) original_version: ID3v2Version,
-	frames: Vec<Frame>,
+	pub(crate) frames: Vec<Frame>,
 }
 
 impl IntoIterator for ID3v2Tag {

src/iff/aiff/read.rs

@@ -60,7 +60,18 @@ where
 	while chunks.next(data).is_ok() {
 		match &chunks.fourcc {
 			#[cfg(feature = "id3v2")]
-			b"ID3 " | b"id3 " => id3v2_tag = Some(chunks.id3_chunk(data)?),
+			b"ID3 " | b"id3 " => {
+				let tag = chunks.id3_chunk(data)?;
+				if let Some(existing_tag) = id3v2_tag.as_mut() {
+					// https://github.com/Serial-ATA/lofty-rs/issues/87
+					// Duplicate tags should have their frames appended to the previous
+					for frame in tag.frames {
+						existing_tag.insert(frame);
+					}
+					continue;
+				}
+				id3v2_tag = Some(tag);
+			},
 			b"COMM" if parse_options.read_properties && comm.is_none() => {
 				if chunks.size < 18 {
 					decode_err!(@BAIL AIFF, "File has an invalid \"COMM\" chunk size (< 18)");

src/iff/wav/read.rs

@@ -93,7 +93,18 @@ where
 				}
 			},
 			#[cfg(feature = "id3v2")]
-			b"ID3 " | b"id3 " => id3v2_tag = Some(chunks.id3_chunk(data)?),
+			b"ID3 " | b"id3 " => {
+				let tag = chunks.id3_chunk(data)?;
+				if let Some(existing_tag) = id3v2_tag.as_mut() {
+					// https://github.com/Serial-ATA/lofty-rs/issues/87
+					// Duplicate tags should have their frames appended to the previous
+					for frame in tag.frames {
+						existing_tag.insert(frame);
+					}
+					continue;
+				}
+				id3v2_tag = Some(tag);
+			},
 			_ => chunks.skip(data)?,
 		}
 	}

src/mpeg/read.rs

@@ -46,6 +46,14 @@ where
 				#[cfg(feature = "id3v2")]
 				{
 					let id3v2 = parse_id3v2(reader, header)?;
+					if let Some(existing_tag) = &mut file.id3v2_tag {
+						// https://github.com/Serial-ATA/lofty-rs/issues/87
+						// Duplicate tags should have their frames appended to the previous
+						for frame in id3v2.frames {
+							existing_tag.insert(frame);
+						}
+						continue;
+					}
 					file.id3v2_tag = Some(id3v2);
 				}
 

tests/files/mpeg.rs

@@ -65,6 +65,29 @@ fn issue_82_solidus_in_tag() {
 	assert_eq!(id3v2_tag.title().as_deref(), Some("Foo / title"));
 }
 
+#[test]
+fn issue_87_duplicate_id3v2() {
+	// The first tag has a bunch of information: An album, artist, encoder, and a title.
+	// This tag is immediately followed by another the contains an artist.
+	// We expect that the title from the first tag has been replaced by this second tag, while
+	// retaining the rest of the information from the first tag.
+	let file = Probe::open("tests/files/assets/issue_87_duplicate_id3v2.mp3")
+		.unwrap()
+		.read()
+		.unwrap();
+
+	assert_eq!(file.file_type(), FileType::MPEG);
+
+	let id3v2_tag = &file.tags()[0];
+	assert_eq!(id3v2_tag.album().as_deref(), Some("album test"));
+	assert_eq!(id3v2_tag.artist().as_deref(), Some("Foo artist")); // Original tag has "artist test"
+	assert_eq!(
+		id3v2_tag.get_string(&ItemKey::EncoderSettings),
+		Some("Lavf58.62.100")
+	);
+	assert_eq!(id3v2_tag.title().as_deref(), Some("title test"));
+}
+
 #[test]
 fn write() {
 	let mut file = temp_file!("tests/files/assets/minimal/full_test.mp3");