diff --git a/CHANGELOG.md b/CHANGELOG.md index 208726ef6..18a083847 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/lofty/Cargo.toml b/lofty/Cargo.toml index e5135db59..c37cb7eb9 100644 --- a/lofty/Cargo.toml +++ b/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 diff --git a/lofty/src/config/global_options.rs b/lofty/src/config/global_options.rs index b3d7fc35f..9fcc4936d 100644 --- a/lofty/src/config/global_options.rs +++ b/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 { diff --git a/lofty/src/id3/v2/tag.rs b/lofty/src/id3/v2/tag.rs index 3c5cac013..56f5abd24 100644 --- a/lofty/src/id3/v2/tag.rs +++ b/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 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 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> + 'a> { pub(crate) flags: Id3v2TagFlags, - pub(crate) frames: I, + pub(crate) frames: Peekable, } impl<'a> Id3v2TagRef<'a, std::iter::Empty>> { 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> + Clone { +pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator> { + #[derive(Clone)] + enum CompanionTagIter { + Filled(F), + Empty(E), + } + + impl<'a, I> Iterator for CompanionTagIter>> + where + I: Iterator>, + { + type Item = FrameRef<'a>; + + fn next(&mut self) -> Option { + 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> + Clon .map(|value| FrameRef(Cow::Owned(Frame::text(Cow::Borrowed(id), value)))) } + fn create_framerefs_for_companion_tag( + companion: Option<&CompanionTag>, + ) -> impl IntoIterator> + 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> + 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> + Clon items.chain(pictures) } -impl<'a, I: Iterator> + Clone + 'a> Id3v2TagRef<'a, I> { +impl<'a, I: Iterator> + 'a> Id3v2TagRef<'a, I> { pub(crate) fn write_to(&mut self, file: &mut F, write_options: WriteOptions) -> Result<()> where F: FileLike, diff --git a/lofty/src/id3/v2/tag/tests.rs b/lofty/src/id3/v2/tag/tests.rs index 0a1eb6a62..c62115f3b 100644 --- a/lofty/src/id3/v2/tag/tests.rs +++ b/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); +} diff --git a/lofty/src/id3/v2/write/frame.rs b/lofty/src/id3/v2/write/frame.rs index 0212ce671..b627d3498 100644 --- a/lofty/src/id3/v2/write/frame.rs +++ b/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( diff --git a/lofty/src/id3/v2/write/mod.rs b/lofty/src/id3/v2/write/mod.rs index 99c89a905..26ac16ce4 100644 --- a/lofty/src/id3/v2/write/mod.rs +++ b/lofty/src/id3/v2/write/mod.rs @@ -47,7 +47,7 @@ where F: FileLike, LoftyError: From<::Error>, LoftyError: From<::Error>, - I: Iterator> + Clone + 'a, + I: Iterator> + '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)?; diff --git a/lofty/src/mp4/ilst/mod.rs b/lofty/src/mp4/ilst/mod.rs index 513ee0b8f..8c4a62249 100644 --- a/lofty/src/mp4/ilst/mod.rs +++ b/lofty/src/mp4/ilst/mod.rs @@ -5,10 +5,11 @@ mod r#ref; pub(crate) mod write; use super::AtomIdent; -use crate::config::WriteOptions; +use crate::config::{global_options, WriteOptions}; use crate::error::LoftyError; use crate::mp4::ilst::atom::AtomDataStorage; use crate::picture::{Picture, PictureType, TOMBSTONE_PICTURE}; +use crate::tag::companion_tag::CompanionTag; use crate::tag::{ try_parse_year, Accessor, ItemKey, ItemValue, MergeTag, SplitTag, Tag, TagExt, TagItem, TagType, }; @@ -771,12 +772,24 @@ impl MergeTag for SplitTagRemainder { impl From for Tag { fn from(input: Ilst) -> 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::Ilst(remainder.0)); + } + + tag } } impl From for Ilst { - 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::ilst) { + return SplitTagRemainder(companion).merge_tag(input); + } + } + SplitTagRemainder::default().merge_tag(input) } } @@ -797,9 +810,13 @@ mod tests { fn read_ilst(path: &str, parse_mode: ParsingMode) -> Ilst { let tag = std::fs::read(path).unwrap(); - let len = tag.len(); + read_ilst_raw(&tag, parse_mode) + } - let cursor = Cursor::new(tag); + fn read_ilst_raw(bytes: &[u8], parse_mode: ParsingMode) -> Ilst { + let len = bytes.len(); + + let cursor = Cursor::new(bytes); let mut reader = AtomReader::new(cursor, parse_mode).unwrap(); super::read::parse_ilst(&mut reader, parse_mode, len as u64).unwrap() @@ -1273,4 +1290,52 @@ mod tests { &AtomData::Bool(false) ); } + + #[test] + fn special_items_roundtrip() { + let mut tag = Ilst::new(); + + let atom = Atom::new( + AtomIdent::Fourcc(*b"SMTH"), + AtomData::Unknown { + code: 0, + data: b"Meaningless Data".to_vec(), + }, + ); + + tag.insert(atom.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: Ilst = tag.into(); + + assert_eq!(tag.atoms.len(), 2); + assert_eq!(tag.artist().as_deref(), Some("Foo Artist")); + assert_eq!(tag.get(&AtomIdent::Fourcc(*b"SMTH")), Some(&atom)); + + let mut tag_bytes = Vec::new(); + tag.dump_to(&mut tag_bytes, WriteOptions::default()) + .unwrap(); + + tag_bytes.drain(..8); // Remove the ilst identifier and size for `read_ilst` + + let tag_re_read = read_ilst_raw(&tag_bytes[..], ParsingMode::Strict); + 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(); + + tag_bytes.drain(..8); // Remove the ilst identifier and size for `read_ilst` + + let generic_tag_re_read = read_ilst_raw(&tag_bytes[..], ParsingMode::Strict); + assert_eq!(tag_re_read, generic_tag_re_read); + } } diff --git a/lofty/src/tag/companion_tag.rs b/lofty/src/tag/companion_tag.rs new file mode 100644 index 000000000..6c36bf35d --- /dev/null +++ b/lofty/src/tag/companion_tag.rs @@ -0,0 +1,24 @@ +use crate::id3::v2::Id3v2Tag; +use crate::mp4::Ilst; + +#[derive(Debug, Clone)] +pub(crate) enum CompanionTag { + Id3v2(Id3v2Tag), + Ilst(Ilst), +} + +impl CompanionTag { + pub(crate) fn id3v2(self) -> Option { + match self { + CompanionTag::Id3v2(tag) => Some(tag), + _ => None, + } + } + + pub(crate) fn ilst(self) -> Option { + match self { + CompanionTag::Ilst(tag) => Some(tag), + _ => None, + } + } +} diff --git a/lofty/src/tag/mod.rs b/lofty/src/tag/mod.rs index f2ef82899..ef99b66e6 100644 --- a/lofty/src/tag/mod.rs +++ b/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, pub(crate) items: Vec, + pub(crate) companion_tag: Option, } #[must_use] @@ -227,15 +229,64 @@ 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::{Accessor, Tag, TagExt, TagType}; + /// + /// 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()); + /// ``` + /// + /// [`GlobalOptions::preserve_format_specific_items`]: crate::config::GlobalOptions::preserve_format_specific_items pub fn re_map(&mut self, tag_type: TagType) { + if let Some(companion_tag) = self.companion_tag.take() { + log::warn!("Discarding format-specific items due to remap"); + drop(companion_tag); + } + 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()); + /// ``` + /// + /// [`GlobalOptions::preserve_format_specific_items`]: crate::config::GlobalOptions::preserve_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 diff --git a/lofty/src/tag/utils.rs b/lofty/src/tag/utils.rs index 40cd0fcff..fd9663941 100644 --- a/lofty/src/tag/utils.rs +++ b/lofty/src/tag/utils.rs @@ -65,7 +65,7 @@ pub(crate) fn dump_tag( TagType::Id3v1 => Into::>::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::::into(tag.clone()) diff --git a/lofty_attr/src/internal.rs b/lofty_attr/src/internal.rs index 1b8837186..5a1978440 100644 --- a/lofty_attr/src/internal.rs +++ b/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) });