From b0898ad535f101c61675a870a3ab3608621e9c18 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Sun, 8 Jan 2023 21:12:40 -0500 Subject: [PATCH 1/2] lofty_attr: Make it possible to prevent auto impl of `Into` --- lofty_attr/src/lofty_file.rs | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/lofty_attr/src/lofty_file.rs b/lofty_attr/src/lofty_file.rs index b8635850d..d61f14a9a 100644 --- a/lofty_attr/src/lofty_file.rs +++ b/lofty_attr/src/lofty_file.rs @@ -12,6 +12,7 @@ pub(crate) fn parse( errors: &mut Vec, ) -> proc_macro2::TokenStream { let impl_audiofile = should_impl_audiofile(&input.attrs); + let impl_into_taggedfile = should_impl_into_taggedfile(&input.attrs); let struct_name = input.ident.clone(); let read_fn = match util::get_attr("read_fn", &input.attrs) { @@ -86,26 +87,32 @@ pub(crate) fn parse( } }); - let audiofile_impl = if impl_audiofile { + let mut audiofile_impl = proc_macro2::TokenStream::new(); + if impl_audiofile { let properties_field = if let Some(field) = properties_field { field } else { bail!(errors, input.ident.span(), "Struct has no properties field"); }; - generate_audiofile_impl( + audiofile_impl = generate_audiofile_impl( &struct_name, &tag_fields, properties_field, read_fn, write_fn, - ) - } else { - proc_macro2::TokenStream::new() - }; + ); + } - let from_taggedfile_impl = - generate_from_taggedfile_impl(&struct_name, &tag_fields, file_type, has_internal_file_type); + let mut from_taggedfile_impl = proc_macro2::TokenStream::new(); + if impl_into_taggedfile { + from_taggedfile_impl = generate_from_taggedfile_impl( + &struct_name, + &tag_fields, + file_type, + has_internal_file_type, + ); + } let getters = get_getters(&tag_fields, &struct_name); @@ -204,6 +211,16 @@ fn should_impl_audiofile(attrs: &[Attribute]) -> bool { true } +fn should_impl_into_taggedfile(attrs: &[Attribute]) -> bool { + for attr in attrs { + if util::has_path_attr(attr, "no_into_taggedfile_impl") { + return false; + } + } + + true +} + fn get_getters<'a>( tag_fields: &'a [FieldContents], struct_name: &'a Ident, From e86a99ec11a3cbe7cdbfd816c267c48226dadea1 Mon Sep 17 00:00:00 2001 From: Serial <69764315+Serial-ATA@users.noreply.github.com> Date: Sun, 8 Jan 2023 21:20:54 -0500 Subject: [PATCH 2/2] FLAC: Store pictures separately from the `VorbisComments` tag --- CHANGELOG.md | 4 + src/flac/mod.rs | 105 ++++++++++++++++++-- src/flac/read.rs | 3 +- src/ogg/mod.rs | 2 + src/ogg/picture_storage.rs | 193 +++++++++++++++++++++++++++++++++++++ src/ogg/tag.rs | 140 ++------------------------- 6 files changed, 306 insertions(+), 141 deletions(-) create mode 100644 src/ogg/picture_storage.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 384b7a9d5..6e1be60cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,11 +22,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **FLAC**: `FlacProperties` - Previously, FLAC files used `FileProperties`. `FlacProperties` was added to support getting the MD5 signature of the audio data. +- **OGG**: `OggPictureStorage` + - This was added to cover the overlap in functionality between `VorbisComments` and `FlacFile` in that they both + store `(Picture, PictureInformation)`. ### Changed - **MP4**: `AtomIdent` stores freeform identifiers as `Cow` opposed to `String` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/95)) - This allows freeform identifiers to be constructed in a const context. - **ID3v2**: `FrameID` now uses `Cow` opposed to `String` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/102)) +- **FLAC**: `FlacFile` now stores pictures separately from its `VorbisComments` tag ### Removed - **Metadata format features** ([PR](https://github.com/Serial-ATA/lofty-rs/pull/97)): diff --git a/src/flac/mod.rs b/src/flac/mod.rs index a31f258b6..a07098eca 100644 --- a/src/flac/mod.rs +++ b/src/flac/mod.rs @@ -9,8 +9,16 @@ pub(crate) mod properties; mod read; pub(crate) mod write; +use crate::error::Result; +use crate::file::{FileType, TaggedFile}; use crate::id3::v2::tag::ID3v2Tag; -use crate::ogg::VorbisComments; +use crate::ogg::tag::VorbisCommentsRef; +use crate::ogg::{OggPictureStorage, VorbisComments}; +use crate::picture::{Picture, PictureInformation}; +use crate::traits::TagExt; + +use std::fs::File; +use std::io::Seek; use lofty_attr::LoftyFile; @@ -23,21 +31,104 @@ pub use properties::FlacProperties; /// ## Notes /// /// * The ID3v2 tag is **read only**, and it's use is discouraged by spec -/// * Picture blocks will be stored in the `VorbisComments` tag, meaning a file could have no vorbis -/// comments block, but `FlacFile::vorbis_comments` will exist. -/// * When writing, the pictures will be stored in their own picture blocks -/// * This behavior will likely change in the future +/// * Pictures are stored in the `FlacFile` itself, rather than the tag. Any pictures inside the tag will +/// be extracted out and stored in their own picture blocks. +/// * It is possible to put pictures inside of the tag, that will not be accessible using the available +/// methods on `FlacFile` ([`FlacFile::pictures`], [`FlacFile::remove_picture_type`], etc.) +/// * When converting to [`TaggedFile`], all pictures will be put inside of a [`VorbisComments`] tag, even if the +/// file did not originally contain one. #[derive(LoftyFile)] #[lofty(read_fn = "read::read_from")] +#[lofty(write_fn = "Self::write_to")] +#[lofty(no_into_taggedfile_impl)] pub struct FlacFile { /// An ID3v2 tag #[lofty(tag_type = "ID3v2")] pub(crate) id3v2_tag: Option, /// The vorbis comments contained in the file - /// - /// NOTE: This field being `Some` does not mean the file has vorbis comments, as Picture blocks exist. #[lofty(tag_type = "VorbisComments")] pub(crate) vorbis_comments_tag: Option, + pub(crate) pictures: Vec<(Picture, PictureInformation)>, /// The file's audio properties pub(crate) properties: FlacProperties, } + +impl FlacFile { + // We need a special write fn to append our pictures into a `VorbisComments` tag + fn write_to(&self, file: &mut File) -> Result<()> { + if let Some(ref id3v2) = self.id3v2_tag { + id3v2.save_to(file)?; + file.rewind()?; + } + + // We have an existing vorbis comments tag, we can just append our pictures to it + if let Some(ref vorbis_comments) = self.vorbis_comments_tag { + return VorbisCommentsRef { + vendor: vorbis_comments.vendor.as_str(), + items: vorbis_comments + .items + .iter() + .map(|(k, v)| (k.as_str(), v.as_str())), + pictures: vorbis_comments + .pictures + .iter() + .map(|(p, i)| (p, *i)) + .chain(self.pictures.iter().map(|(p, i)| (p, *i))), + } + .write_to(file); + } + + // We have pictures, but no vorbis comments tag, we'll need to create a dummy one + if !self.pictures.is_empty() { + return VorbisCommentsRef { + vendor: "", + items: std::iter::empty(), + pictures: self.pictures.iter().map(|(p, i)| (p, *i)), + } + .write_to(file); + } + + Ok(()) + } +} + +impl OggPictureStorage for FlacFile { + fn pictures(&self) -> &[(Picture, PictureInformation)] { + &self.pictures + } +} + +impl From for TaggedFile { + fn from(mut value: FlacFile) -> Self { + TaggedFile { + ty: FileType::FLAC, + properties: value.properties.into(), + tags: { + let mut tags = Vec::with_capacity(2); + + if let Some(id3v2) = value.id3v2_tag { + tags.push(id3v2.into()); + } + + // Move our pictures into a `VorbisComments` tag, creating one if necessary + match value.vorbis_comments_tag { + Some(mut vorbis_comments) => { + vorbis_comments.pictures.append(&mut value.pictures); + tags.push(vorbis_comments.into()); + }, + None if !value.pictures.is_empty() => tags.push( + VorbisComments { + vendor: String::new(), + items: Vec::new(), + pictures: value.pictures, + } + .into(), + ), + _ => {}, + } + + tags + }, + } + } +} diff --git a/src/flac/read.rs b/src/flac/read.rs index 5c56e632c..5cd94c48c 100644 --- a/src/flac/read.rs +++ b/src/flac/read.rs @@ -39,6 +39,7 @@ where let mut flac_file = FlacFile { id3v2_tag: None, vorbis_comments_tag: None, + pictures: Vec::new(), properties: FlacProperties::default(), }; @@ -75,7 +76,7 @@ where match block.ty { 4 => read_comments(&mut &*block.content, block.content.len() as u64, &mut tag)?, - 6 => tag + 6 => flac_file .pictures .push(Picture::from_flac_bytes(&block.content, false)?), _ => {}, diff --git a/src/ogg/mod.rs b/src/ogg/mod.rs index a7b9c23f9..e96c5c2a4 100644 --- a/src/ogg/mod.rs +++ b/src/ogg/mod.rs @@ -5,6 +5,7 @@ //! The only supported tag format is [`VorbisComments`] pub(crate) mod constants; pub(crate) mod opus; +mod picture_storage; pub(crate) mod read; pub(crate) mod speex; pub(crate) mod tag; @@ -22,6 +23,7 @@ use ogg_pager::Page; pub use opus::properties::OpusProperties; pub use opus::OpusFile; +pub use picture_storage::OggPictureStorage; pub use speex::properties::SpeexProperties; pub use speex::SpeexFile; pub use tag::VorbisComments; diff --git a/src/ogg/picture_storage.rs b/src/ogg/picture_storage.rs new file mode 100644 index 000000000..bfd9e86fc --- /dev/null +++ b/src/ogg/picture_storage.rs @@ -0,0 +1,193 @@ +use crate::error::Result; +use crate::picture::{Picture, PictureInformation, PictureType}; + +/// Defines methods for interacting with an item storing OGG pictures +/// +/// This exists due to *both* [`VorbisComments`](crate::ogg::VorbisComments) and [`FlacFile`](crate::flac::FlacFile) needing to store +/// pictures in their own ways. +/// +/// It cannot be implemented downstream. +pub trait OggPictureStorage: private::Sealed { + /// Inserts a [`Picture`] + /// + /// NOTES: + /// + /// * If `information` is `None`, the [`PictureInformation`] will be inferred using [`PictureInformation::from_picture`]. + /// * According to spec, there can only be one picture of type [`PictureType::Icon`] and [`PictureType::OtherIcon`]. + /// When attempting to insert these types, if another is found it will be removed and returned. + /// + /// # Errors + /// + /// * See [`PictureInformation::from_picture`] + fn insert_picture( + &mut self, + picture: Picture, + information: Option, + ) -> Result> { + let ret = match picture.pic_type { + PictureType::Icon | PictureType::OtherIcon => self + .pictures() + .iter() + .position(|(p, _)| p.pic_type == picture.pic_type) + .map(|pos| self.remove_picture(pos)), + _ => None, + }; + + let info = match information { + Some(pic_info) => pic_info, + None => PictureInformation::from_picture(&picture)?, + }; + + self.pictures_mut().push((picture, info)); + + Ok(ret) + } + + /// Removes a certain [`PictureType`] + fn remove_picture_type(&mut self, picture_type: PictureType) { + self.pictures_mut() + .retain(|(pic, _)| pic.pic_type != picture_type); + } + + /// Returns the stored [`Picture`]s as a slice + /// + /// # Examples + /// + /// ```rust + /// use lofty::ogg::{OggPictureStorage, VorbisComments}; + /// + /// let mut tag = VorbisComments::default(); + /// + /// assert!(tag.pictures().is_empty()); + /// ``` + fn pictures(&self) -> &[(Picture, PictureInformation)]; + + /// Replaces the picture at the given `index` + /// + /// NOTE: If `index` is out of bounds, the `picture` will be appended + /// to the list. + /// + /// # Examples + /// + /// ```rust + /// use lofty::ogg::{VorbisComments, OggPictureStorage}; + /// # use lofty::{Picture, PictureInformation, PictureType, MimeType}; + /// + /// # fn main() -> lofty::Result<()> { + /// # let front_cover = Picture::new_unchecked(PictureType::CoverFront, MimeType::Png, None, Vec::new()); + /// # let front_cover_info = PictureInformation::default(); + /// # let back_cover = Picture::new_unchecked(PictureType::CoverBack, MimeType::Png, None, Vec::new()); + /// # let back_cover_info = PictureInformation::default(); + /// # let another_picture = Picture::new_unchecked(PictureType::Band, MimeType::Png, None, Vec::new()); + /// let mut tag = VorbisComments::default(); + /// + /// // Add a front cover + /// tag.insert_picture(front_cover, Some(front_cover_info))?; + /// + /// assert_eq!(tag.pictures().len(), 1); + /// assert_eq!(tag.pictures()[0].0.pic_type(), PictureType::CoverFront); + /// + /// // Replace the front cover with a back cover + /// tag.set_picture(0, back_cover, back_cover_info); + /// + /// assert_eq!(tag.pictures().len(), 1); + /// assert_eq!(tag.pictures()[0].0.pic_type(), PictureType::CoverBack); + /// + /// // Use an out of bounds index + /// tag.set_picture(100, another_picture, PictureInformation::default()); + /// + /// assert_eq!(tag.pictures().len(), 2); + /// # Ok(()) } + /// ``` + #[allow(clippy::missing_panics_doc)] + fn set_picture(&mut self, index: usize, picture: Picture, info: PictureInformation) { + if index >= self.pictures().len() { + // Safe to unwrap, since `info` is guaranteed to exist + self.insert_picture(picture, Some(info)).unwrap(); + } else { + self.pictures_mut()[index] = (picture, info); + } + } + + /// Removes and returns the picture at the given `index` + /// + /// # Panics + /// + /// Panics if `index` is out of bounds. + /// + /// # Examples + /// + /// ```rust + /// use lofty::ogg::{VorbisComments, OggPictureStorage}; + /// # use lofty::{Picture, PictureType, MimeType, PictureInformation}; + /// + /// # fn main() -> lofty::Result<()> { + /// # let front_cover = Picture::new_unchecked(PictureType::CoverFront, MimeType::Png, None, Vec::new()); + /// # let front_cover_info = PictureInformation::default(); + /// let mut tag = VorbisComments::default(); + /// + /// // Add a front cover + /// tag.insert_picture(front_cover, Some(front_cover_info))?; + /// + /// assert_eq!(tag.pictures().len(), 1); + /// + /// tag.remove_picture(0); + /// + /// assert_eq!(tag.pictures().len(), 0); + /// # Ok(()) } + /// ``` + fn remove_picture(&mut self, index: usize) -> (Picture, PictureInformation) { + self.pictures_mut().remove(index) + } + + /// Removes all pictures and returns them + /// + /// # Examples + /// + /// ```rust + /// use lofty::ogg::{VorbisComments, OggPictureStorage}; + /// # use lofty::{Picture, PictureType, MimeType, PictureInformation}; + /// + /// # fn main() -> lofty::Result<()> { + /// # let front_cover = Picture::new_unchecked(PictureType::CoverFront, MimeType::Png, None, Vec::new()); + /// # let front_cover_info = PictureInformation::default(); + /// # let back_cover = Picture::new_unchecked(PictureType::CoverBack, MimeType::Png, None, Vec::new()); + /// # let back_cover_info = PictureInformation::default(); + /// let mut tag = VorbisComments::default(); + /// + /// // Add front and back covers + /// tag.insert_picture(front_cover, Some(front_cover_info))?; + /// tag.insert_picture(back_cover, Some(front_cover_info))?; + /// + /// assert_eq!(tag.pictures().len(), 2); + /// + /// let pictures = tag.remove_pictures(); + /// assert_eq!(pictures.len(), 2); + /// + /// // The tag no longer contains any pictures + /// assert_eq!(tag.pictures().len(), 0); + /// # Ok(()) } + /// ``` + fn remove_pictures(&mut self) -> Vec<(Picture, PictureInformation)> { + core::mem::take(self.pictures_mut()) + } +} + +mod private { + use crate::{Picture, PictureInformation}; + + pub trait Sealed { + fn pictures_mut(&mut self) -> &mut Vec<(Picture, PictureInformation)>; + } + + impl Sealed for crate::ogg::tag::VorbisComments { + fn pictures_mut(&mut self) -> &mut Vec<(Picture, PictureInformation)> { + &mut self.pictures + } + } + impl Sealed for crate::flac::FlacFile { + fn pictures_mut(&mut self) -> &mut Vec<(Picture, PictureInformation)> { + &mut self.pictures + } + } +} diff --git a/src/ogg/tag.rs b/src/ogg/tag.rs index 9bf3d787e..047616224 100644 --- a/src/ogg/tag.rs +++ b/src/ogg/tag.rs @@ -1,8 +1,9 @@ use crate::error::{LoftyError, Result}; use crate::file::FileType; use crate::macros::err; +use crate::ogg::picture_storage::OggPictureStorage; use crate::ogg::write::OGGFormat; -use crate::picture::{Picture, PictureInformation, PictureType}; +use crate::picture::{Picture, PictureInformation}; use crate::probe::Probe; use crate::tag::item::{ItemKey, ItemValue, TagItem}; use crate::tag::{Tag, TagType}; @@ -127,139 +128,12 @@ impl VorbisComments { self.items.drain(..split_idx).map(|(_, v)| v) } +} - /// Inserts a [`Picture`] - /// - /// NOTES: - /// - /// * If `information` is `None`, the [`PictureInformation`] will be inferred using [`PictureInformation::from_picture`]. - /// * According to spec, there can only be one picture of type [`PictureType::Icon`] and [`PictureType::OtherIcon`]. - /// When attempting to insert these types, if another is found it will be removed and returned. - /// - /// # Errors - /// - /// * See [`PictureInformation::from_picture`] - pub fn insert_picture( - &mut self, - picture: Picture, - information: Option, - ) -> Result> { - let ret = match picture.pic_type { - PictureType::Icon | PictureType::OtherIcon => self - .pictures - .iter() - .position(|(p, _)| p.pic_type == picture.pic_type) - .map(|pos| self.pictures.remove(pos)), - _ => None, - }; - - let info = match information { - Some(pic_info) => pic_info, - None => PictureInformation::from_picture(&picture)?, - }; - - self.pictures.push((picture, info)); - - Ok(ret) - } - - /// Removes a certain [`PictureType`] - pub fn remove_picture_type(&mut self, picture_type: PictureType) { - self.pictures.retain(|(p, _)| p.pic_type != picture_type) - } - - /// Returns the stored [`Picture`]s as a slice - /// - /// # Examples - /// - /// ```rust - /// use lofty::ogg::VorbisComments; - /// - /// let mut tag = VorbisComments::default(); - /// - /// assert!(tag.pictures().is_empty()); - /// ``` - pub fn pictures(&self) -> &[(Picture, PictureInformation)] { +impl OggPictureStorage for VorbisComments { + fn pictures(&self) -> &[(Picture, PictureInformation)] { &self.pictures } - - /// Replaces the picture at the given `index` - /// - /// NOTE: If `index` is out of bounds, the `picture` will be appended - /// to the list. - /// - /// # Examples - /// - /// ```rust - /// use lofty::ogg::VorbisComments; - /// # use lofty::{Picture, PictureInformation, PictureType, MimeType}; - /// - /// # fn main() -> lofty::Result<()> { - /// # let front_cover = Picture::new_unchecked(PictureType::CoverFront, MimeType::Png, None, Vec::new()); - /// # let front_cover_info = PictureInformation::default(); - /// # let back_cover = Picture::new_unchecked(PictureType::CoverBack, MimeType::Png, None, Vec::new()); - /// # let back_cover_info = PictureInformation::default(); - /// # let another_picture = Picture::new_unchecked(PictureType::Band, MimeType::Png, None, Vec::new()); - /// let mut tag = VorbisComments::default(); - /// - /// // Add a front cover - /// tag.insert_picture(front_cover, Some(front_cover_info))?; - /// - /// assert_eq!(tag.pictures().len(), 1); - /// assert_eq!(tag.pictures()[0].0.pic_type(), PictureType::CoverFront); - /// - /// // Replace the front cover with a back cover - /// tag.set_picture(0, back_cover, back_cover_info); - /// - /// assert_eq!(tag.pictures().len(), 1); - /// assert_eq!(tag.pictures()[0].0.pic_type(), PictureType::CoverBack); - /// - /// // Use an out of bounds index - /// tag.set_picture(100, another_picture, PictureInformation::default()); - /// - /// assert_eq!(tag.pictures().len(), 2); - /// # Ok(()) } - /// ``` - #[allow(clippy::missing_panics_doc)] - pub fn set_picture(&mut self, index: usize, picture: Picture, info: PictureInformation) { - if index >= self.pictures.len() { - // Safe to unwrap, since `info` is guaranteed to exist - self.insert_picture(picture, Some(info)).unwrap(); - } else { - self.pictures[index] = (picture, info); - } - } - - /// Removes and returns the picture at the given `index` - /// - /// # Panics - /// - /// Panics if `index` is out of bounds. - /// - /// # Examples - /// - /// ```rust - /// use lofty::ogg::VorbisComments; - /// # use lofty::{Picture, PictureType, MimeType, PictureInformation}; - /// - /// # fn main() -> lofty::Result<()> { - /// # let front_cover = Picture::new_unchecked(PictureType::CoverFront, MimeType::Png, None, Vec::new()); - /// # let front_cover_info = PictureInformation::default(); - /// let mut tag = VorbisComments::default(); - /// - /// // Add a front cover - /// tag.insert_picture(front_cover, Some(front_cover_info))?; - /// - /// assert_eq!(tag.pictures().len(), 1); - /// - /// tag.remove_picture(0); - /// - /// assert_eq!(tag.pictures().len(), 0); - /// # Ok(()) } - /// ``` - pub fn remove_picture(&mut self, index: usize) -> (Picture, PictureInformation) { - self.pictures.remove(index) - } } impl Accessor for VorbisComments { @@ -527,7 +401,7 @@ where IP: Iterator, { #[allow(clippy::shadow_unrelated)] - fn write_to(&mut self, file: &mut File) -> Result<()> { + pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> { let probe = Probe::new(file).guess_file_type()?; let f_ty = probe.file_type(); @@ -586,7 +460,7 @@ pub(crate) fn create_vorbis_comments_ref( #[cfg(test)] mod tests { - use crate::ogg::VorbisComments; + use crate::ogg::{OggPictureStorage, VorbisComments}; use crate::{Tag, TagExt, TagType}; fn read_tag(tag: &[u8]) -> VorbisComments {