ID3v2: Support 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.

Author: Serial-ATA

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/assets/issue_87_duplicate_id3v2.mp3

@@ -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");