FLAC: Store pictures separately from the VorbisComments tag

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<str>` 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<str>` 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)):

lofty_attr/src/lofty_file.rs

@@ -12,6 +12,7 @@ pub(crate) fn parse(
 	errors: &mut Vec<syn::Error>,
 ) -> 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,

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<ID3v2Tag>,
 	/// 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<VorbisComments>,
+	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<FlacFile> 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
+			},
+		}
+	}
+}

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)?),
 			_ => {},

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;