GlobalOptions: Add preserve_format_specific_items()

CHANGELOG.md

@@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - New `tag::items` module for generic representations of complex tag items
   - New `Timestamp` item for ISO 8601 timestamps ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
 - **ID3v2**: Special handling for frames with timestamps with `Frame::Timestamp` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
+- **GlobalOptions**: `preserve_format_specific_items()` ([issue](https://github.com/Serial-ATA/lofty-rs/issues/302)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/391))
+  - This will allow for the preservation of format-specific items when converting between tag types.
+  - Previously, these items would be discarded when converting to the generic `Tag`. Now they are stored
+    in an immutable container, and silently rejoined with the tag when converting back to the original format
+    or when writing.
 
 ### Changed
 - **VorbisComments**/**ApeTag**: Verify contents of `ItemKey::FlagCompilation` during `Tag` merge ([PR](https://github.com/Serial-ATA/lofty-rs/pull/387))

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/config/global_options.rs

@@ -24,6 +24,7 @@ pub(crate) unsafe fn global_options() -> &'static GlobalOptions {
 pub struct GlobalOptions {
 	pub(crate) use_custom_resolvers: bool,
 	pub(crate) allocation_limit: usize,
+	pub(crate) preserve_format_specific_items: bool,
 }
 
 impl GlobalOptions {
@@ -46,6 +47,7 @@ impl GlobalOptions {
 		Self {
 			use_custom_resolvers: true,
 			allocation_limit: Self::DEFAULT_ALLOCATION_LIMIT,
+			preserve_format_specific_items: true,
 		}
 	}
 
@@ -85,6 +87,31 @@ impl GlobalOptions {
 		self.allocation_limit = allocation_limit;
 		*self
 	}
+
+	/// Whether or not to preserve format-specific items
+	///
+	/// When converting a tag from its concrete format (ex. [`Id3v2Tag`](crate::id3::v2::Id3v2Tag)) to
+	/// a [`Tag`], this options controls whether to preserve any special items that
+	/// are unique to the concrete tag.
+	///
+	/// This will store an extra immutable tag alongside the generic [`Tag`], which will be merged
+	/// back into the concrete tag when converting back.
+	///
+	/// # Examples
+	///
+	/// ```rust
+	/// use lofty::config::{apply_global_options, GlobalOptions};
+	///
+	/// // I'm just reading tags, I don't need to preserve format-specific items
+	/// let global_options = GlobalOptions::new().preserve_format_specific_items(false);
+	/// apply_global_options(global_options);
+	/// ```
+	///
+	/// [`Tag`]: crate::tag::Tag
+	pub fn preserve_format_specific_items(&mut self, preserve_format_specific_items: bool) -> Self {
+		self.preserve_format_specific_items = preserve_format_specific_items;
+		*self
+	}
 }
 
 impl Default for GlobalOptions {
@@ -96,6 +123,7 @@ impl Default for GlobalOptions {
 	/// GlobalOptions {
 	/// 	use_custom_resolvers: true,
 	/// 	allocation_limit: Self::DEFAULT_ALLOCATION_LIMIT,
+	/// 	preserve_format_specific_items: true,
 	/// }
 	/// ```
 	fn default() -> Self {

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,62 @@ 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 mut 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 mut tag_re_read = read_tag_raw(&tag_bytes[..]);
+
+	// Ensure ordered comparison
+	tag.frames.sort_by_key(|frame| frame.id().to_string());
+	tag_re_read
+		.frames
+		.sort_by_key(|frame| frame.id().to_string());
+	assert_eq!(tag, tag_re_read);
+
+	// Now write from `Tag`
+	let tag: Tag = tag.into();
+
+	let mut tag_bytes = Vec::new();
+	tag.dump_to(&mut tag_bytes, WriteOptions::default())
+		.unwrap();
+
+	let mut generic_tag_re_read = read_tag_raw(&tag_bytes[..]);
+
+	generic_tag_re_read
+		.frames
+		.sort_by_key(|frame| frame.id().to_string());
+	assert_eq!(tag_re_read, generic_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)?;