Tag: Preserve ID3v2 special items by default

lofty/Cargo.toml

@@ -18,7 +18,7 @@ byteorder     = { workspace = true }
 # ID3 compressed frames
 flate2        = { version = "1.0.28", optional = true }
 # Proc macros
-lofty_attr = "0.10.0"
+lofty_attr = { path = "../lofty_attr" }
 # Debug logging
 log           = "0.4.21"
 # OGG Vorbis/Opus

lofty/src/id3/v2/tag.rs

@@ -3,7 +3,7 @@ mod tests;
 
 use super::frame::{Frame, EMPTY_CONTENT_DESCRIPTOR, UNKNOWN_LANGUAGE};
 use super::header::{Id3v2TagFlags, Id3v2Version};
-use crate::config::WriteOptions;
+use crate::config::{global_options, WriteOptions};
 use crate::error::{LoftyError, Result};
 use crate::id3::v1::GENRES;
 use crate::id3::v2::frame::{FrameRef, MUSICBRAINZ_UFID_OWNER};
@@ -18,6 +18,7 @@ use crate::id3::v2::util::pairs::{
 use crate::id3::v2::{BinaryFrame, FrameHeader, FrameId, KeyValueFrame, TimestampFrame};
 use crate::mp4::AdvisoryRating;
 use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE};
+use crate::tag::companion_tag::CompanionTag;
 use crate::tag::items::Timestamp;
 use crate::tag::{
 	try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType,
@@ -28,6 +29,7 @@ use crate::util::text::{decode_text, TextDecodeOptions, TextEncoding};
 
 use std::borrow::Cow;
 use std::io::{Cursor, Write};
+use std::iter::Peekable;
 use std::ops::Deref;
 use std::str::FromStr;
 
@@ -906,7 +908,7 @@ impl TagExt for Id3v2Tag {
 	{
 		Id3v2TagRef {
 			flags: self.flags,
-			frames: self.frames.iter().filter_map(Frame::as_opt_ref),
+			frames: self.frames.iter().filter_map(Frame::as_opt_ref).peekable(),
 		}
 		.write_to(file, write_options)
 	}
@@ -924,7 +926,7 @@ impl TagExt for Id3v2Tag {
 	) -> std::result::Result<(), Self::Err> {
 		Id3v2TagRef {
 			flags: self.flags,
-			frames: self.frames.iter().filter_map(Frame::as_opt_ref),
+			frames: self.frames.iter().filter_map(Frame::as_opt_ref).peekable(),
 		}
 		.dump_to(writer, write_options)
 	}
@@ -1468,32 +1470,64 @@ impl MergeTag for SplitTagRemainder {
 
 impl From<Id3v2Tag> for Tag {
 	fn from(input: Id3v2Tag) -> Self {
-		input.split_tag().1
+		let (remainder, mut tag) = input.split_tag();
+
+		if unsafe { global_options().preserve_format_specific_items } && remainder.0.len() > 0 {
+			tag.companion_tag = Some(CompanionTag::Id3v2(remainder.0));
+		}
+
+		tag
 	}
 }
 
 impl From<Tag> for Id3v2Tag {
-	fn from(input: Tag) -> Self {
+	fn from(mut input: Tag) -> Self {
+		if unsafe { global_options().preserve_format_specific_items } {
+			if let Some(companion) = input.companion_tag.take().and_then(CompanionTag::id3v2) {
+				return SplitTagRemainder(companion).merge_tag(input);
+			}
+		}
+
 		SplitTagRemainder::default().merge_tag(input)
 	}
 }
 
 pub(crate) struct Id3v2TagRef<'a, I: Iterator<Item = FrameRef<'a>> + 'a> {
 	pub(crate) flags: Id3v2TagFlags,
-	pub(crate) frames: I,
+	pub(crate) frames: Peekable<I>,
 }
 
 impl<'a> Id3v2TagRef<'a, std::iter::Empty<FrameRef<'a>>> {
 	pub(crate) fn empty() -> Self {
 		Self {
 			flags: Id3v2TagFlags::default(),
-			frames: std::iter::empty(),
+			frames: std::iter::empty().peekable(),
 		}
 	}
 }
 
 // Create an iterator of FrameRef from a Tag's items for Id3v2TagRef::new
-pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> + Clone {
+pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> {
+	#[derive(Clone)]
+	enum CompanionTagIter<F, E> {
+		Filled(F),
+		Empty(E),
+	}
+
+	impl<'a, I> Iterator for CompanionTagIter<I, std::iter::Empty<Frame<'_>>>
+	where
+		I: Iterator<Item = FrameRef<'a>>,
+	{
+		type Item = FrameRef<'a>;
+
+		fn next(&mut self) -> Option<Self::Item> {
+			match self {
+				CompanionTagIter::Filled(iter) => iter.next(),
+				CompanionTagIter::Empty(_) => None,
+			}
+		}
+	}
+
 	fn create_frameref_for_number_pair<'a>(
 		number: Option<&str>,
 		total: Option<&str>,
@@ -1503,6 +1537,17 @@ pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> + Clon
 			.map(|value| FrameRef(Cow::Owned(Frame::text(Cow::Borrowed(id), value))))
 	}
 
+	fn create_framerefs_for_companion_tag(
+		companion: Option<&CompanionTag>,
+	) -> impl IntoIterator<Item = FrameRef<'_>> + Clone {
+		match companion {
+			Some(CompanionTag::Id3v2(companion)) => {
+				CompanionTagIter::Filled(companion.frames.iter().filter_map(Frame::as_opt_ref))
+			},
+			_ => CompanionTagIter::Empty(std::iter::empty()),
+		}
+	}
+
 	let items = tag
 		.items()
 		.filter(|item| !NUMBER_PAIR_KEYS.contains(item.key()))
@@ -1517,6 +1562,9 @@ pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> + Clon
 			tag.get_string(&ItemKey::DiscNumber),
 			tag.get_string(&ItemKey::DiscTotal),
 			"TPOS",
+		))
+		.chain(create_framerefs_for_companion_tag(
+			tag.companion_tag.as_ref(),
 		));
 
 	let pictures = tag.pictures().iter().map(|p| {
@@ -1529,7 +1577,7 @@ pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> + Clon
 	items.chain(pictures)
 }
 
-impl<'a, I: Iterator<Item = FrameRef<'a>> + Clone + 'a> Id3v2TagRef<'a, I> {
+impl<'a, I: Iterator<Item = FrameRef<'a>> + 'a> Id3v2TagRef<'a, I> {
 	pub(crate) fn write_to<F>(&mut self, file: &mut F, write_options: WriteOptions) -> Result<()>
 	where
 		F: FileLike,

lofty/src/id3/v2/tag/tests.rs

@@ -2,19 +2,26 @@ use crate::config::ParsingMode;
 use crate::id3::v2::header::Id3v2Header;
 use crate::id3::v2::items::PopularimeterFrame;
 use crate::id3::v2::util::pairs::DEFAULT_NUMBER_IN_PAIR;
-use crate::id3::v2::TimestampFrame;
+use crate::id3::v2::{
+	ChannelInformation, ChannelType, RelativeVolumeAdjustmentFrame, TimestampFrame,
+};
 use crate::picture::MimeType;
 use crate::tag::items::Timestamp;
 use crate::tag::utils::test_utils::read_path;
 
 use super::*;
 
+use std::collections::HashMap;
+
 const COMMENT_FRAME_ID: &str = "COMM";
 
 fn read_tag(path: &str) -> Id3v2Tag {
 	let tag_bytes = read_path(path);
+	read_tag_raw(&tag_bytes)
+}
 
-	let mut reader = Cursor::new(&tag_bytes[..]);
+fn read_tag_raw(bytes: &[u8]) -> Id3v2Tag {
+	let mut reader = Cursor::new(&bytes[..]);
 
 	let header = Id3v2Header::parse(&mut reader).unwrap();
 	crate::id3::v2::read::parse_id3v2(&mut reader, header, ParsingMode::Strict).unwrap()
@@ -1243,3 +1250,42 @@ fn timestamp_roundtrip() {
 		_ => panic!("Expected a TimestampFrame"),
 	}
 }
+
+#[test]
+fn special_items_roundtrip() {
+	let mut tag = Id3v2Tag::new();
+
+	let rva2 = Frame::RelativeVolumeAdjustment(RelativeVolumeAdjustmentFrame::new(
+		String::from("Foo RVA"),
+		HashMap::from([(
+			ChannelType::MasterVolume,
+			ChannelInformation {
+				channel_type: ChannelType::MasterVolume,
+				volume_adjustment: 30,
+				bits_representing_peak: 0,
+				peak_volume: None,
+			},
+		)]),
+	));
+
+	tag.insert(rva2.clone());
+	tag.set_artist(String::from("Foo Artist")); // Some value that we *can* represent generically
+
+	let tag: Tag = tag.into();
+
+	assert_eq!(tag.len(), 1);
+	assert_eq!(tag.artist().as_deref(), Some("Foo Artist"));
+
+	let tag: Id3v2Tag = tag.into();
+
+	assert_eq!(tag.frames.len(), 2);
+	assert_eq!(tag.artist().as_deref(), Some("Foo Artist"));
+	assert_eq!(tag.get(&FrameId::Valid(Cow::Borrowed("RVA2"))), Some(&rva2));
+
+	let mut tag_bytes = Vec::new();
+	tag.dump_to(&mut tag_bytes, WriteOptions::default())
+		.unwrap();
+
+	let tag_re_read = read_tag_raw(&tag_bytes[..]);
+	assert_eq!(tag, tag_re_read);
+}

lofty/src/id3/v2/write/frame.rs

@@ -36,7 +36,9 @@ fn verify_frame(frame: &FrameRef<'_>) -> Result<()> {
 		| ("POPM", Frame::Popularimeter(_))
 		| ("TIPL" | "TMCL", Frame::KeyValue { .. })
 		| ("WFED" | "GRP1" | "MVNM" | "MVIN", Frame::Text { .. })
-		| ("TDEN" | "TDOR" | "TDRC" | "TDRL" | "TDTG", Frame::Timestamp(_)) => Ok(()),
+		| ("TDEN" | "TDOR" | "TDRC" | "TDRL" | "TDTG", Frame::Timestamp(_))
+		| ("RVA2", Frame::RelativeVolumeAdjustment(_))
+		| ("PRIV", Frame::Private(_)) => Ok(()),
 		(id, Frame::Text { .. }) if id.starts_with('T') => Ok(()),
 		(id, Frame::Url(_)) if id.starts_with('W') => Ok(()),
 		(id, frame_value) => Err(Id3v2Error::new(Id3v2ErrorKind::BadFrame(

lofty/src/id3/v2/write/mod.rs

@@ -47,7 +47,7 @@ where
 	F: FileLike,
 	LoftyError: From<<F as Truncate>::Error>,
 	LoftyError: From<<F as Length>::Error>,
-	I: Iterator<Item = FrameRef<'a>> + Clone + 'a,
+	I: Iterator<Item = FrameRef<'a>> + 'a,
 {
 	let probe = Probe::new(file).guess_file_type()?;
 	let file_type = probe.file_type();
@@ -67,11 +67,8 @@ where
 
 	// Attempting to write a non-empty tag to a read only format
 	// An empty tag implies the tag should be stripped.
-	if Id3v2Tag::READ_ONLY_FORMATS.contains(&file_type) {
-		let mut peek = tag.frames.clone().peekable();
-		if peek.peek().is_some() {
-			err!(UnsupportedTag);
-		}
+	if Id3v2Tag::READ_ONLY_FORMATS.contains(&file_type) && tag.frames.peek().is_some() {
+		err!(UnsupportedTag);
 	}
 
 	let id3v2 = create_tag(tag, write_options)?;

lofty/src/tag/companion_tag.rs

@@ -0,0 +1,15 @@
+use crate::id3::v2::Id3v2Tag;
+
+#[derive(Debug, Clone)]
+pub(crate) enum CompanionTag {
+	Id3v2(Id3v2Tag),
+}
+
+impl CompanionTag {
+	pub(crate) fn id3v2(self) -> Option<Id3v2Tag> {
+		match self {
+			CompanionTag::Id3v2(tag) => Some(tag),
+			_ => None,
+		}
+	}
+}

lofty/src/tag/mod.rs

@@ -1,6 +1,7 @@
 //! Utilities for generic tag handling
 
 mod accessor;
+pub(crate) mod companion_tag;
 pub(crate) mod item;
 pub mod items;
 mod split_merge_tag;
@@ -111,6 +112,7 @@ pub struct Tag {
 	tag_type: TagType,
 	pub(crate) pictures: Vec<Picture>,
 	pub(crate) items: Vec<TagItem>,
+	pub(crate) companion_tag: Option<companion_tag::CompanionTag>,
 }
 
 #[must_use]
@@ -227,15 +229,55 @@ impl Tag {
 			tag_type,
 			pictures: Vec::new(),
 			items: Vec::new(),
+			companion_tag: None,
 		}
 	}
 
 	/// Change the [`TagType`], remapping all items
+	///
+	/// NOTE: If any format-specific items are present, they will be removed.
+	///       See [`GlobalOptions::preserve_format_specific_items`].
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::tag::{Tag, TagType, Accessor, TagExt};
+	///
+	/// let mut tag = Tag::new(TagType::Id3v2);
+	/// tag.set_album(String::from("Album"));
+	///
+	/// // ID3v2 supports the album tag
+	/// assert_eq!(tag.len(), 1);
+	///
+	/// // But AIFF text chunks do not, the item will be lost
+	/// tag.re_map(TagType::AiffText);
+	/// assert!(tag.is_empty());
 	pub fn re_map(&mut self, tag_type: TagType) {
+		self.companion_tag = None;
 		self.retain(|i| i.re_map(tag_type));
 		self.tag_type = tag_type
 	}
 
+	/// Check if the tag contains any format-specific items
+	///
+	/// See [`GlobalOptions::preserve_format_specific_items`].
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::tag::{Accessor, Tag, TagExt, TagType};
+	///
+	/// let mut tag = Tag::new(TagType::Id3v2);
+	/// tag.set_album(String::from("Album"));
+	///
+	/// // We cannot create a tag with format-specific items.
+	/// // This must come from a conversion, such as `Id3v2Tag` -> `Tag`
+	/// assert!(!tag.has_format_specific_items());
+	/// ```
+	pub fn has_format_specific_items(&self) -> bool {
+		self.companion_tag.is_some()
+	}
+
 	/// Returns the [`TagType`]
 	pub fn tag_type(&self) -> TagType {
 		self.tag_type

lofty/src/tag/utils.rs

@@ -65,7 +65,7 @@ pub(crate) fn dump_tag<W: Write>(
 		TagType::Id3v1 => Into::<Id3v1TagRef<'_>>::into(tag).dump_to(writer, write_options),
 		TagType::Id3v2 => Id3v2TagRef {
 			flags: Id3v2TagFlags::default(),
-			frames: v2::tag::tag_frames(tag),
+			frames: v2::tag::tag_frames(tag).peekable(),
 		}
 		.dump_to(writer, write_options),
 		TagType::Mp4Ilst => Into::<Ilst>::into(tag.clone())

lofty_attr/src/internal.rs

@@ -63,7 +63,7 @@ pub(crate) fn init_write_lookup(
 		insert!(map, Id3v2, {
 			lofty::id3::v2::tag::Id3v2TagRef {
 				flags: lofty::id3::v2::Id3v2TagFlags::default(),
-				frames: lofty::id3::v2::tag::tag_frames(tag),
+				frames: lofty::id3::v2::tag::tag_frames(tag).peekable(),
 			}
 			.write_to(file, write_options)
 		});