Support multiple picture types

This will be used in the future, for now only the front cover is accessible. This doesn't fully for opus due to opus_headers only storing a single k/v pair.

Signed-off-by: Serial <69764315+Serial-ATA@users.noreply.github.com>

src/components/tags/id3_tag.rs

@@ -8,6 +8,7 @@ use crate::{
 
 pub use id3::Tag as Id3v2InnerTag;
 
+use crate::types::picture::PictureType;
 use filepath::FilePath;
 use std::convert::TryInto;
 use std::fs::File;
@@ -50,10 +51,17 @@ impl std::convert::TryFrom<id3::frame::Picture> for Picture {
 		let id3::frame::Picture {
 			ref mime_type,
 			data,
+			ref picture_type,
 			..
 		} = inp;
 		let mime_type: MimeType = mime_type.as_str().try_into()?;
-		Ok(Self { data, mime_type })
+		let pic_type: PictureType = picture_type.into();
+
+		Ok(Self {
+			pic_type,
+			data,
+			mime_type,
+		})
 	}
 }
 
@@ -126,6 +134,7 @@ impl AudioTagEdit for Id3v2Tag {
 			.find(|&pic| matches!(pic.picture_type, id3::frame::PictureType::CoverFront))
 			.and_then(|pic| {
 				Some(Picture {
+					pic_type: PictureType::CoverFront,
 					data: pic.data.clone(),
 					mime_type: (pic.mime_type.as_str()).try_into().ok()?,
 				})

src/components/tags/mp4_tag.rs

@@ -7,6 +7,7 @@ use crate::{
 
 pub use mp4ameta::{FourCC, Tag as Mp4InnerTag};
 
+use crate::types::picture::PictureType;
 use std::fs::File;
 use std::path::Path;
 #[cfg(feature = "duration")]
@@ -33,10 +34,12 @@ impl std::convert::TryFrom<mp4ameta::Data> for Picture {
 	fn try_from(inp: mp4ameta::Data) -> Result<Self> {
 		Ok(match inp {
 			mp4ameta::Data::Png(data) => Self {
+				pic_type: PictureType::CoverFront, // TODO
 				data,
 				mime_type: MimeType::Png,
 			},
 			mp4ameta::Data::Jpeg(data) => Self {
+				pic_type: PictureType::CoverFront, // TODO
 				data,
 				mime_type: MimeType::Jpeg,
 			},
@@ -104,10 +107,12 @@ impl AudioTagEdit for Mp4Tag {
 
 		self.inner.artwork().and_then(|data| match data {
 			Jpeg(d) => Some(Picture {
+				pic_type: PictureType::CoverFront, // TODO
 				data: d.clone(),
 				mime_type: MimeType::Jpeg,
 			}),
 			Png(d) => Some(Picture {
+				pic_type: PictureType::CoverFront, // TODO
 				data: d.clone(),
 				mime_type: MimeType::Png,
 			}),

src/components/tags/riff_tag.rs

@@ -8,7 +8,7 @@ use crate::{
 
 use std::borrow::BorrowMut;
 use std::collections::HashMap;
-use std::fs::{File, OpenOptions};
+use std::fs::File;
 use std::io::{Cursor, Seek, SeekFrom, Write};
 use std::path::Path;
 #[cfg(feature = "duration")]
@@ -141,17 +141,16 @@ impl AudioTagEdit for RiffTag {
 		self.remove_key("AlbumArtist")
 	}
 
+	/// This will always return `None`, as this is non-standard
 	fn album_cover(&self) -> Option<Picture> {
-		todo!()
+		None
 	}
 
-	fn set_album_cover(&mut self, _cover: Picture) {
-		todo!()
-	}
+	/// This will not do anything, as this is non-standard
+	fn set_album_cover(&mut self, _cover: Picture) {}
 
-	fn remove_album_cover(&mut self) {
-		todo!()
-	}
+	/// This will not do anything, as this is non-standard
+	fn remove_album_cover(&mut self) {}
 
 	fn track_number(&self) -> Option<u32> {
 		if let Some(Ok(y)) = self.get_value("TrackNumber").map(str::parse::<u32>) {
@@ -264,10 +263,4 @@ impl AudioTagWrite for RiffTag {
 
 		Ok(())
 	}
-
-	fn write_to_path(&self, path: &str) -> Result<()> {
-		self.write_to(&mut OpenOptions::new().read(true).write(true).open(path)?)?;
-
-		Ok(())
-	}
 }

src/components/tags/vorbis_tag.rs

@@ -3,13 +3,13 @@
 use crate::components::logic;
 use crate::tag::VorbisFormat;
 use crate::{
-	impl_tag, Album, AnyTag, AudioTag, AudioTagEdit, AudioTagWrite, MimeType, Picture, Result,
-	TagType, ToAny, ToAnyTag,
+	impl_tag, Album, AnyTag, AudioTag, AudioTagEdit, AudioTagWrite, Error, MimeType, Picture,
+	Result, TagType, ToAny, ToAnyTag, PictureType
 };
 
 use std::collections::HashMap;
 use std::convert::{TryFrom, TryInto};
-use std::fs::{File, OpenOptions};
+use std::fs::File;
 use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 use std::path::Path;
 #[cfg(feature = "duration")]
@@ -22,6 +22,7 @@ struct VorbisInnerTag {
 	format: Option<VorbisFormat>,
 	vendor: String,
 	comments: HashMap<String, String>,
+	pictures: Option<Vec<Picture>>,
 }
 
 impl Default for VorbisInnerTag {
@@ -30,6 +31,7 @@ impl Default for VorbisInnerTag {
 			format: None,
 			vendor: String::new(),
 			comments: HashMap::default(),
+			pictures: None,
 		}
 	}
 }
@@ -70,46 +72,111 @@ impl VorbisInnerTag {
 
 				let vendor = headers.comment_hdr.vendor;
 
-				let comments: HashMap<String, String> =
-					headers.comment_hdr.comment_list.into_iter().collect();
+				let mut comments = headers.comment_hdr.comment_list;
+
+				let mut pictures: Vec<Picture> = Vec::new();
+
+				if let Some(p) = comments
+					.iter()
+					.position(|(k, _)| *k == "METADATA_BLOCK_PICTURE")
+				{
+					let kv = comments.remove(p);
+					if let Some(pic) = picture_from_data(&*kv.1)? {
+						pictures.push(pic)
+					}
+				}
 
 				Ok(Self {
 					format: Some(format),
 					vendor,
-					comments,
+					comments: comments.into_iter().collect(),
+					pictures: if pictures.is_empty() {
+						None
+					} else {
+						Some(pictures)
+					},
 				})
 			},
 			VorbisFormat::Opus => {
 				let headers = opus_headers::parse_from_path(path)?;
 				let vendor = headers.comments.vendor;
 
+				let mut comments = headers.comments.user_comments;
+
+				// TODO: opus_headers doesn't store all keys
+				let pictures = if let Some(data) = comments.remove("METADATA_BLOCK_PICTURE") {
+					picture_from_data(&*data)?.map(|pic| vec![pic])
+				} else {
+					None
+				};
+
 				Ok(Self {
 					format: Some(format),
 					vendor,
-					comments: headers.comments.user_comments,
+					comments,
+					pictures,
 				})
 			},
 			VorbisFormat::Flac => {
 				let headers = metaflac::Tag::read_from_path(path)?;
-				let comments = headers.vorbis_comments().unwrap();
-				let mut comment_collection = Vec::new();
+				let as_vorbis: VorbisTag = headers.into();
 
-				for (k, v) in comments.comments.clone() {
-					for e in v {
-						comment_collection.push((k.clone(), e.clone()))
-					}
-				}
-
-				Ok(Self {
-					format: Some(format),
-					vendor: comments.vendor_string.clone(),
-					comments: comment_collection.into_iter().collect(),
-				})
+				Ok(as_vorbis.inner)
 			},
 		}
 	}
 }
 
+fn picture_from_data(data: &str) -> Result<Option<Picture>> {
+	let data = match base64::decode(data) {
+		Ok(o) => o,
+		Err(_) => data.as_bytes().to_vec(),
+	};
+
+	let mut i = 0;
+
+	let picture_type_b = u32::from_le_bytes(match (&data[i..i + 4]).try_into() {
+		Ok(o) => o,
+		Err(_) => return Err(Error::InvalidData),
+	});
+
+	let picture_type = match picture_type_b {
+		3 => PictureType::CoverFront,
+		4 => PictureType::CoverBack,
+		_ => PictureType::Other,
+	};
+
+	i += 4;
+
+	match data[i..i + 4].try_into() {
+		Ok(mime_len) => {
+			i += 4;
+			let mime_len = u32::from_le_bytes(mime_len);
+
+			match String::from_utf8(data[i..i + mime_len as usize].to_vec()) {
+				Ok(mime_type) => {
+					let mime_type = MimeType::try_from(&*mime_type);
+
+					match mime_type {
+						Ok(mime_type) => {
+							let content = data[(8 + mime_len) as usize..].to_vec();
+
+							Ok(Some(Picture {
+								pic_type: picture_type,
+								data: content,
+								mime_type,
+							}))
+						},
+						Err(_) => Ok(None),
+					}
+				},
+				Err(_) => Ok(None),
+			}
+		},
+		Err(_) => Ok(None),
+	}
+}
+
 impl_tag!(
 	VorbisTag,
 	VorbisInnerTag,
@@ -134,10 +201,23 @@ impl From<metaflac::Tag> for VorbisTag {
 	fn from(inp: metaflac::Tag) -> Self {
 		let mut tag = Self::default();
 
-		let (comments, vendor) = if let Some(comments) = inp.vorbis_comments() {
+		let (comments, vendor, pictures) = if let Some(comments) = inp.vorbis_comments() {
+			let comments = comments.clone();
+			let mut user_comments = comments.comments;
+
+			let mut pictures = Vec::new();
+
+			if let Some(data) = user_comments.remove("METADATA_BLOCK_PICTURE") {
+				for item in data {
+					if let Ok(Some(pic)) = picture_from_data(&*item) {
+						pictures.push(pic)
+					}
+				}
+			}
+
 			let mut comment_collection = Vec::new();
 
-			for (k, v) in comments.comments.clone() {
+			for (k, v) in user_comments.clone() {
 				for e in v {
 					comment_collection.push((k.clone(), e.clone()))
 				}
@@ -146,20 +226,21 @@ impl From<metaflac::Tag> for VorbisTag {
 			let comment_collection: HashMap<String, String> =
 				comment_collection.into_iter().collect();
 
-			let vendor = comments.vendor_string.clone();
+			let vendor = comments.vendor_string;
 
-			(comment_collection, vendor)
+			(comment_collection, vendor, Some(pictures))
 		} else {
 			let comments: HashMap<String, String> = HashMap::new();
 			let vendor = String::new();
 
-			(comments, vendor)
+			(comments, vendor, None)
 		};
 
 		tag.inner = VorbisInnerTag {
 			format: Some(VorbisFormat::Flac),
 			vendor,
 			comments,
+			pictures,
 		};
 
 		tag
@@ -264,64 +345,20 @@ impl AudioTagEdit for VorbisTag {
 	}
 
 	fn album_cover(&self) -> Option<Picture> {
-		let picture = self.inner.get_value("METADATA_BLOCK_PICTURE");
-
-		return match picture {
+		match &self.inner.pictures {
 			None => None,
-			Some(data) => {
-				let data_str = data.to_string();
-				let data = data_str.as_bytes();
-
-				if data.is_empty() {
-					return None;
+			Some(pictures) => {
+				for pic in pictures {
+					if pic.pic_type == PictureType::CoverFront {
+						return Some(pic.clone())
+					}
 				}
 
-				let data = match base64::decode(data) {
-					Ok(o) => o,
-					Err(_) => data.to_vec(),
-				};
-
-				let mut i = 0;
-
-				let picture_type = u32::from_le_bytes(match (&data[i..i + 4]).try_into() {
-					Ok(o) => o,
-					Err(_) => return None,
-				});
-
-				if picture_type != 3 {
-					return None;
-				}
-
-				i += 4;
-				match data[i..i + 4].try_into() {
-					Ok(mime_len) => {
-						i += 4;
-						let mime_len = u32::from_le_bytes(mime_len);
-
-						match String::from_utf8(data[i..i + mime_len as usize].to_vec()) {
-							Ok(mime_type) => {
-								let mime_type = MimeType::try_from(&*mime_type);
-
-								match mime_type {
-									Ok(mime_type) => {
-										let content = data[(8 + mime_len) as usize..].to_vec();
-
-										Some(Picture {
-											data: content,
-											mime_type,
-										})
-									},
-									Err(_) => return None,
-								}
-							},
-							Err(_) => return None,
-						}
-					},
-					Err(_) => return None,
-				}
+				None
 			},
-		};
+		}
 	}
+
 	fn set_album_cover(&mut self, cover: Picture) {
 		self.remove_album_cover();
 		let mime = String::from(cover.mime_type);
@@ -338,8 +375,17 @@ impl AudioTagEdit for VorbisTag {
 
 		let encoded = base64::encode(encoded);
 
-		self.inner.set_value("METADATA_BLOCK_PICTURE", encoded);
+		if let Ok(Some(pic))= picture_from_data(&*encoded) {
+			if let Some(mut pictures) = self.inner.pictures.clone() {
+				pictures.retain(|p| p.pic_type != PictureType::CoverFront);
+				pictures.push(pic);
+				self.inner.pictures = Some(pictures)
+			} else {
+				self.inner.pictures = Some(vec![pic])
+			}
+		}
 	}
+
 	fn remove_album_cover(&mut self) {
 		self.inner.remove_key("METADATA_BLOCK_PICTURE")
 	}
@@ -421,11 +467,6 @@ impl AudioTagWrite for VorbisTag {
 			}
 		}
 
-		Ok(())
-	}
-	fn write_to_path(&self, path: &str) -> Result<()> {
-		self.write_to(&mut OpenOptions::new().read(true).write(true).open(path)?)?;
-
 		Ok(())
 	}
 }

src/error.rs

@@ -9,6 +9,8 @@ pub enum Error {
 	UnknownFormat,
 	#[error("File contains no data")]
 	EmptyFile,
+	#[error("File has invalid data")]
+	InvalidData,
 
 	/// Unsupported file extension
 	#[error("Unsupported format: {0}")]

src/lib.rs

@@ -88,7 +88,7 @@ mod types;
 pub use crate::types::{
 	album::Album,
 	anytag::AnyTag,
-	picture::{MimeType, Picture},
+	picture::{MimeType, Picture, PictureType},
 };
 
 mod tag;

src/traits.rs

@@ -2,7 +2,7 @@
 use crate::components::tags::*;
 use crate::{Album, AnyTag, Picture, Result, TagType};
 
-use std::fs::File;
+use std::fs::{File, OpenOptions};
 
 pub trait AudioTag: AudioTagEdit + AudioTagWrite + ToAnyTag {}
 
@@ -100,7 +100,11 @@ pub trait AudioTagWrite {
 	/// # Errors
 	///
 	/// Will return `Err` if `path` doesn't exist
-	fn write_to_path(&self, path: &str) -> Result<()>;
+	fn write_to_path(&self, path: &str) -> Result<()> {
+		self.write_to(&mut OpenOptions::new().read(true).write(true).open(path)?)?;
+
+		Ok(())
+	}
 }
 
 pub trait ToAnyTag: ToAny {

src/types/picture.rs

@@ -1,4 +1,5 @@
 use crate::{Error, Result};
+use id3::frame::PictureType as id3PicType;
 use std::convert::TryFrom;
 
 /// Mime types for covers.
@@ -43,15 +44,38 @@ impl From<MimeType> for String {
 	}
 }
 
+/// The picture type
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum PictureType {
+	CoverFront,
+	CoverBack,
+	Other,
+}
+
+impl From<&id3PicType> for PictureType {
+	fn from(inp: &id3PicType) -> Self {
+		match inp {
+			id3PicType::CoverFront => PictureType::CoverFront,
+			id3PicType::CoverBack => PictureType::CoverBack,
+			_ => PictureType::Other,
+		}
+	}
+}
+
 /// Represents a picture, with its data and mime type.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Picture {
+	pub pic_type: PictureType,
 	pub data: Vec<u8>,
 	pub mime_type: MimeType,
 }
 
 impl Picture {
-	pub fn new(data: Vec<u8>, mime_type: MimeType) -> Self {
-		Self { data, mime_type }
+	pub fn new(pic_type: PictureType, data: Vec<u8>, mime_type: MimeType) -> Self {
+		Self {
+			pic_type,
+			data,
+			mime_type,
+		}
 	}
 }

tests/io.rs

@@ -1,5 +1,5 @@
 #![cfg(feature = "default")]
-use lofty::{MimeType, Picture, Tag};
+use lofty::{MimeType, Picture, PictureType, Tag};
 
 macro_rules! full_test {
 	($function:ident, $file:expr) => {
@@ -35,13 +35,18 @@ macro_rules! add_tags {
 		println!("Setting album artists");
 		tag.set_album_artist("foo album artist");
 
-		let _cover = Picture {
+		let cover = Picture {
+			pic_type: PictureType::CoverFront,
 			mime_type: MimeType::Jpeg,
 			data: vec![0; 10],
 		};
 
-		// tag.set_album_cover(cover.clone());
-		// assert_eq!(tag.album_cover(), Some(cover));
+		let file_name = stringify!($file);
+
+		if file_name != stringify!("tests/assets/a.wav") {
+			tag.set_album_cover(cover.clone());
+			assert_eq!(tag.album_cover(), Some(cover));
+		}
 
 		println!("Writing");
 		tag.write_to_path($file).unwrap();
@@ -108,9 +113,9 @@ macro_rules! remove_tags {
 		assert!(tag.album_artists_vec().is_none());
 		tag.remove_album_artists();
 
-		// tag.remove_album_cover();
-		// assert!(tag.album_cover().is_none());
-		// tag.remove_album_cover();
+		tag.remove_album_cover();
+		assert!(tag.album_cover().is_none());
+		tag.remove_album_cover();
 
 		println!("Writing");
 		tag.write_to_path($file).unwrap();