id3v2: Do not replace non-empty frames with duplicate, empty frames

src/id3/v2/frame/mod.rs

@@ -202,6 +202,35 @@ pub enum FrameValue {
 	Binary(Vec<u8>),
 }
 
+impl FrameValue {
+	/// Check for empty content
+	///
+	/// Returns `None` if the frame type is not supported.
+	pub(super) fn is_empty(&self) -> Option<bool> {
+		let is_empty = match self {
+			FrameValue::Text(text) => text.value.is_empty(),
+			FrameValue::UserText(extended_text) => extended_text.content.is_empty(),
+			FrameValue::Url(link) => link.0.is_empty(),
+			FrameValue::UserUrl(extended_url) => extended_url.content.is_empty(),
+			FrameValue::Comment(comment) => comment.content.is_empty(),
+			FrameValue::UnsynchronizedText(unsync_text) => unsync_text.content.is_empty(),
+			FrameValue::Picture(picture) => picture.picture.data.is_empty(),
+			FrameValue::KeyValue(key_value) => key_value.key_value_pairs.is_empty(),
+			FrameValue::UniqueFileIdentifier(ufid) => ufid.identifier.is_empty(),
+			FrameValue::EventTimingCodes(event_timing) => event_timing.events.is_empty(),
+			FrameValue::Private(private) => private.private_data.is_empty(),
+			FrameValue::Binary(binary) => binary.is_empty(),
+			FrameValue::Popularimeter(_)
+			| FrameValue::RelativeVolumeAdjustment(_)
+			| FrameValue::Ownership(_) => {
+				// Undefined.
+				return None;
+			},
+		};
+		Some(is_empty)
+	}
+}
+
 impl TryFrom<ItemValue> for FrameValue {
 	type Error = LoftyError;
 

src/id3/v2/read.rs

@@ -67,7 +67,29 @@ where
 
 	loop {
 		match ParsedFrame::read(reader, header.version, parse_mode)? {
-			ParsedFrame::Next(frame) => drop(tag.insert(frame)),
+			ParsedFrame::Next(frame) => {
+				let frame_value_is_empty = frame.value.is_empty();
+				if let Some(replaced_frame) = tag.insert(frame) {
+					// Duplicate frames are not allowed. But if this occurs we try
+					// to keep the frame with the non-empty content. Superfluous,
+					// duplicate frames that follow the first frame are often empty.
+					if frame_value_is_empty == Some(true)
+						&& replaced_frame.value.is_empty() == Some(false)
+					{
+						log::warn!(
+							"Restoring non-empty frame with ID \"{id}\" that has been replaced by \
+							 an empty frame with the same ID",
+							id = replaced_frame.id
+						);
+						drop(tag.insert(replaced_frame));
+					} else {
+						log::warn!(
+							"Replaced frame with ID \"{id}\" by a frame with the same ID",
+							id = replaced_frame.id
+						);
+					}
+				}
+			},
 			// No frame content found or ignored due to errors, but we can expect more frames
 			ParsedFrame::Skip { size } => {
 				skip_frame(reader, size)?;