Continued cleanup

src/id3/v2/frame/id.rs

@@ -23,11 +23,7 @@ impl FrameID {
 	/// * `id` contains invalid characters (must be 'A'..='Z' and '0'..='9')
 	/// * `id` is an invalid length (must be 3 or 4)
 	pub fn new(id: &str) -> Result<Self> {
-		for c in id.chars() {
-			if !('A'..='Z').contains(&c) && !('0'..='9').contains(&c) {
-				return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameID).into());
-			}
-		}
+		Self::verify_id(id)?;
 
 		match id.len() {
 			3 => Ok(FrameID::Outdated(id.to_string())),
@@ -42,6 +38,16 @@ impl FrameID {
 			FrameID::Valid(v) | FrameID::Outdated(v) => v.as_str(),
 		}
 	}
+
+	pub(crate) fn verify_id(id_str: &str) -> Result<()> {
+		for c in id_str.chars() {
+			if !('A'..='Z').contains(&c) && !('0'..='9').contains(&c) {
+				return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameID).into());
+			}
+		}
+
+		Ok(())
+	}
 }
 
 impl TryFrom<ItemKey> for FrameID {

src/id3/v2/frame/mod.rs

@@ -317,11 +317,8 @@ impl<'a> TryFrom<&'a TagItem> for FrameRef<'a> {
 
 	fn try_from(tag_item: &'a TagItem) -> std::result::Result<Self, Self::Error> {
 		let id = match tag_item.key() {
-			ItemKey::Unknown(unknown)
-				if unknown.len() == 4
-					&& unknown.is_ascii()
-					&& unknown.chars().all(|c| c.is_ascii_uppercase()) =>
-			{
+			ItemKey::Unknown(unknown) if unknown.len() == 4 => {
+				id::FrameID::verify_id(unknown)?;
 				Ok(unknown.as_str())
 			},
 			k => k

src/iff/wav/tag/mod.rs

@@ -73,7 +73,7 @@ impl RiffInfoList {
 	///
 	/// This will case-insensitively replace any item with the same key
 	pub fn insert(&mut self, key: String, value: String) {
-		if valid_key(key.as_str()) {
+		if read::verify_key(key.as_str()) {
 			self.items
 				.iter()
 				.position(|(k, _)| k.eq_ignore_ascii_case(key.as_str()))
@@ -110,11 +110,13 @@ impl TagIO for RiffInfoList {
 	}
 
 	fn save_to(&self, file: &mut File) -> std::result::Result<(), Self::Err> {
-		Into::<RiffInfoListRef<'_>>::into(self).write_to(file)
+		RiffInfoListRef::new(self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())))
+			.write_to(file)
 	}
 
 	fn dump_to<W: Write>(&self, writer: &mut W) -> std::result::Result<(), Self::Err> {
-		Into::<RiffInfoListRef<'_>>::into(self).dump_to(writer)
+		RiffInfoListRef::new(self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())))
+			.dump_to(writer)
 	}
 
 	fn remove_from_path<P: AsRef<Path>>(&self, path: P) -> std::result::Result<(), Self::Err> {
@@ -149,24 +151,18 @@ impl From<Tag> for RiffInfoList {
 
 		for item in input.items {
 			if let ItemValue::Text(val) | ItemValue::Locator(val) = item.item_value {
-				let item_key = match item.item_key {
+				match item.item_key {
 					ItemKey::Unknown(unknown) => {
-						if !(unknown.len() == 4 && unknown.is_ascii()) {
-							continue;
+						if read::verify_key(&unknown) {
+							riff_info.items.push((unknown, val))
 						}
-
-						unknown
 					},
 					k => {
 						if let Some(key) = k.map_key(TagType::RiffInfo, false) {
-							key.to_string()
-						} else {
-							continue;
+							riff_info.items.push((key.to_string(), val))
 						}
 					},
-				};
-
-				riff_info.items.push((item_key, val))
+				}
 			}
 		}
 
@@ -174,39 +170,21 @@ impl From<Tag> for RiffInfoList {
 	}
 }
 
-pub(crate) struct RiffInfoListRef<'a> {
-	items: Box<dyn Iterator<Item = (&'a str, &'a String)> + 'a>,
+pub(crate) struct RiffInfoListRef<'a, I>
+where
+	I: Iterator<Item = (&'a str, &'a str)>,
+{
+	items: I,
 }
 
-impl<'a> Into<RiffInfoListRef<'a>> for &'a RiffInfoList {
-	fn into(self) -> RiffInfoListRef<'a> {
-		RiffInfoListRef {
-			items: Box::new(self.items.iter().map(|(k, v)| (k.as_str(), v))),
+impl<'a, I> RiffInfoListRef<'a, I>
+where
+	I: Iterator<Item = (&'a str, &'a str)>,
+{
+	pub(crate) fn new(items: I) -> RiffInfoListRef<'a, I> {
+		RiffInfoListRef { items }
 	}
-	}
-}
 
-impl<'a> Into<RiffInfoListRef<'a>> for &'a Tag {
-	fn into(self) -> RiffInfoListRef<'a> {
-		RiffInfoListRef {
-			items: Box::new(self.items.iter().filter_map(|i| {
-				if let ItemValue::Text(val) | ItemValue::Locator(val) = &i.item_value {
-					let item_key = i.key().map_key(TagType::RiffInfo, true).unwrap();
-
-					if item_key.len() == 4 && item_key.is_ascii() {
-						Some((item_key, val))
-					} else {
-						None
-					}
-				} else {
-					None
-				}
-			})),
-		}
-	}
-}
-
-impl<'a> RiffInfoListRef<'a> {
 	pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> {
 		write::write_riff_info(file, self)
 	}
@@ -221,8 +199,19 @@ impl<'a> RiffInfoListRef<'a> {
 	}
 }
 
-fn valid_key(key: &str) -> bool {
-	key.len() == 4 && key.is_ascii()
+pub(crate) fn tagitems_into_riff(items: &[TagItem]) -> impl Iterator<Item = (&str, &str)> {
+	items.iter().filter_map(|i| {
+		let item_key = i.key().map_key(TagType::RiffInfo, true);
+
+		match (item_key, i.value()) {
+			(Some(key), ItemValue::Text(val) | ItemValue::Locator(val))
+				if read::verify_key(key) =>
+			{
+				Some((key, val.as_str()))
+			},
+			_ => None,
+		}
+	})
 }
 
 #[cfg(test)]

src/iff/wav/tag/read.rs

@@ -21,10 +21,7 @@ where
 			FileDecodingError::new(FileType::WAV, "Non UTF-8 item key found in RIFF INFO")
 		})?;
 
-		if key_str
-			.chars()
-			.any(|c| !('A'..='Z').contains(&c) && !('0'..='9').contains(&c))
-		{
+		if !verify_key(&key_str) {
 			return Err(FileDecodingError::new(
 				FileType::WAV,
 				"RIFF INFO item key contains invalid characters",
@@ -42,3 +39,10 @@ where
 
 	Ok(())
 }
+
+pub(super) fn verify_key(key: &str) -> bool {
+	key.len() == 4
+		&& key
+			.chars()
+			.all(|c| ('A'..='Z').contains(&c) || ('0'..='9').contains(&c))
+}

src/iff/wav/tag/write.rs

@@ -8,10 +8,13 @@ use std::io::{Read, Seek, SeekFrom, Write};
 
 use byteorder::{LittleEndian, WriteBytesExt};
 
-pub(in crate::iff::wav) fn write_riff_info(
+pub(in crate::iff::wav) fn write_riff_info<'a, I>(
 	data: &mut File,
-	tag: &mut RiffInfoListRef<'_>,
-) -> Result<()> {
+	tag: &mut RiffInfoListRef<'a, I>,
+) -> Result<()>
+where
+	I: Iterator<Item = (&'a str, &'a str)>,
+{
 	let file_size = verify_wav(data)?;
 
 	let mut riff_info_bytes = Vec::new();
@@ -76,7 +79,7 @@ where
 }
 
 pub(super) fn create_riff_info(
-	items: &mut dyn Iterator<Item = (&str, &String)>,
+	items: &mut dyn Iterator<Item = (&str, &str)>,
 	bytes: &mut Vec<u8>,
 ) -> Result<()> {
 	let mut items = items.peekable();

src/iff/wav/write.rs

@@ -10,7 +10,10 @@ use std::fs::File;
 pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 	match tag.tag_type() {
 		#[cfg(feature = "riff_info_list")]
-		TagType::RiffInfo => Into::<super::tag::RiffInfoListRef<'_>>::into(tag).write_to(data),
+		TagType::RiffInfo => {
+			super::tag::RiffInfoListRef::new(super::tag::tagitems_into_riff(tag.items()))
+				.write_to(data)
+		},
 		#[cfg(feature = "id3v2")]
 		TagType::Id3v2 => {
 			v2::tag::Id3v2TagRef::new(v2::Id3v2TagFlags::default(), v2::tag::tag_frames(tag))

src/ogg/flac/write.rs

@@ -12,7 +12,14 @@ use byteorder::{LittleEndian, WriteBytesExt};
 
 const MAX_BLOCK_SIZE: u32 = 16_777_215;
 
-pub(in crate) fn write_to(data: &mut File, tag: &mut VorbisCommentsRef<'_>) -> Result<()> {
+pub(in crate) fn write_to<'a, II, IP>(
+	data: &mut File,
+	tag: &mut VorbisCommentsRef<'a, II, IP>,
+) -> Result<()>
+where
+	II: Iterator<Item = (&'a str, &'a str)>,
+	IP: Iterator<Item = (&'a Picture, PictureInformation)>,
+{
 	let stream_info = verify_flac(data)?;
 	let stream_info_end = stream_info.end as usize;
 

src/ogg/tag.rs

@@ -159,7 +159,12 @@ impl TagIO for VorbisComments {
 	/// * [`PictureInformation::from_picture`]
 	/// * [`std::io::Error`]
 	fn save_to(&self, file: &mut File) -> std::result::Result<(), Self::Err> {
-		Into::<VorbisCommentsRef<'_>>::into(self).write_to(file)
+		VorbisCommentsRef {
+			vendor: self.vendor.as_str(),
+			items: self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())),
+			pictures: self.pictures.iter().map(|(p, i)| (p, *i)),
+		}
+		.write_to(file)
 	}
 
 	/// Dumps the tag to a writer
@@ -172,7 +177,12 @@ impl TagIO for VorbisComments {
 	/// * [`PictureInformation::from_picture`]
 	/// * [`std::io::Error`]
 	fn dump_to<W: Write>(&self, writer: &mut W) -> std::result::Result<(), Self::Err> {
-		Into::<VorbisCommentsRef<'_>>::into(self).dump_to(writer)
+		VorbisCommentsRef {
+			vendor: self.vendor.as_str(),
+			items: self.items.iter().map(|(k, v)| (k.as_str(), v.as_str())),
+			pictures: self.pictures.iter().map(|(p, i)| (p, *i)),
+		}
+		.dump_to(writer)
 	}
 
 	fn remove_from_path<P: AsRef<Path>>(&self, path: P) -> std::result::Result<(), Self::Err> {
@@ -254,13 +264,21 @@ impl From<Tag> for VorbisComments {
 	}
 }
 
-pub(crate) struct VorbisCommentsRef<'a> {
+pub(crate) struct VorbisCommentsRef<'a, II, IP>
+where
+	II: Iterator<Item = (&'a str, &'a str)>,
+	IP: Iterator<Item = (&'a Picture, PictureInformation)>,
+{
 	pub vendor: &'a str,
-	pub items: Box<dyn Iterator<Item = (&'a str, &'a str)> + 'a>,
-	pub pictures: Box<dyn Iterator<Item = (&'a Picture, PictureInformation)> + 'a>,
+	pub items: II,
+	pub pictures: IP,
 }
 
-impl<'a> VorbisCommentsRef<'a> {
+impl<'a, II, IP> VorbisCommentsRef<'a, II, IP>
+where
+	II: Iterator<Item = (&'a str, &'a str)>,
+	IP: Iterator<Item = (&'a Picture, PictureInformation)>,
+{
 	#[allow(clippy::shadow_unrelated)]
 	fn write_to(&mut self, file: &mut File) -> Result<()> {
 		let probe = Probe::new(file).guess_file_type()?;
@@ -286,47 +304,28 @@ impl<'a> VorbisCommentsRef<'a> {
 	}
 }
 
-impl<'a> Into<VorbisCommentsRef<'a>> for &'a VorbisComments {
-	fn into(self) -> VorbisCommentsRef<'a> {
-		VorbisCommentsRef {
-			vendor: self.vendor.as_str(),
-			items: Box::new(
-				self.items
-					.as_slice()
-					.iter()
-					.map(|(k, v)| (k.as_str(), v.as_str())),
-			),
-			pictures: Box::new(self.pictures.as_slice().iter().map(|(p, i)| (p, *i))),
-		}
-	}
-}
+pub(crate) fn create_vorbis_comments_ref(
+	tag: &Tag,
+) -> (
+	&str,
+	impl Iterator<Item = (&str, &str)>,
+	impl Iterator<Item = (&Picture, PictureInformation)>,
+) {
+	let vendor = tag.get_string(&ItemKey::EncoderSoftware).unwrap_or("");
 
-impl<'a> Into<VorbisCommentsRef<'a>> for &'a Tag {
-	fn into(self) -> VorbisCommentsRef<'a> {
-		let vendor = self.get_string(&ItemKey::EncoderSoftware).unwrap_or("");
-
-		let items = self.items.iter().filter_map(|i| match i.value() {
-			ItemValue::Text(val) | ItemValue::Locator(val)
-				// Already got the vendor above
-				if i.key() != &ItemKey::EncoderSoftware =>
-			{
-				i.key()
+	let items = tag.items.iter().filter_map(|i| match i.value() {
+		ItemValue::Text(val) | ItemValue::Locator(val) => i
+			.key()
 			.map_key(TagType::VorbisComments, true)
-					.map(|key| (key, val.as_str()))
-			},
+			.map(|key| (key, val.as_str())),
 		_ => None,
 	});
 
-		VorbisCommentsRef {
-			vendor,
-			items: Box::new(items),
-			pictures: Box::new(
-				self.pictures
+	let pictures = tag
+		.pictures
 		.iter()
-					.map(|p| (p, PictureInformation::default())),
-			),
-		}
-	}
+		.map(|p| (p, PictureInformation::default()));
+	(vendor, items, pictures)
 }
 
 #[cfg(test)]

src/ogg/write.rs

@@ -2,7 +2,7 @@ use super::verify_signature;
 use crate::error::{ErrorKind, FileEncodingError, LoftyError, Result};
 use crate::macros::try_vec;
 use crate::ogg::constants::{OPUSTAGS, VORBIS_COMMENT_HEAD};
-use crate::ogg::tag::VorbisCommentsRef;
+use crate::ogg::tag::{create_vorbis_comments_ref, VorbisCommentsRef};
 use crate::types::file::FileType;
 use crate::types::picture::PictureInformation;
 use crate::types::tag::{Tag, TagType};
@@ -35,7 +35,19 @@ impl OGGFormat {
 pub(in crate) fn write_to(data: &mut File, tag: &Tag, format: OGGFormat) -> Result<()> {
 	match tag.tag_type() {
 		#[cfg(feature = "vorbis_comments")]
-		TagType::VorbisComments => write(data, &mut Into::<VorbisCommentsRef<'_>>::into(tag), format),
+		TagType::VorbisComments => write(
+			data,
+			&mut {
+				let (vendor, items, pictures) = create_vorbis_comments_ref(tag);
+
+				VorbisCommentsRef {
+					vendor,
+					items,
+					pictures,
+				}
+			},
+			format,
+		),
 		_ => Err(LoftyError::new(ErrorKind::UnsupportedTag)),
 	}
 }
@@ -68,12 +80,16 @@ pub(crate) fn create_comments(
 }
 
 #[cfg(feature = "vorbis_comments")]
-pub(super) fn create_pages(
-	tag: &mut VorbisCommentsRef<'_>,
+pub(super) fn create_pages<'a, II, IP>(
+	tag: &mut VorbisCommentsRef<'a, II, IP>,
 	writer: &mut Cursor<Vec<u8>>,
 	stream_serial: u32,
 	add_framing_bit: bool,
-) -> Result<Vec<Page>> {
+) -> Result<Vec<Page>>
+where
+	II: Iterator<Item = (&'a str, &'a str)>,
+	IP: Iterator<Item = (&'a crate::types::picture::Picture, PictureInformation)>,
+{
 	const PICTURE_KEY: &str = "METADATA_BLOCK_PICTURE=";
 
 	let item_count_pos = writer.seek(SeekFrom::Current(0))?;
@@ -116,11 +132,15 @@ pub(super) fn create_pages(
 }
 
 #[cfg(feature = "vorbis_comments")]
-pub(super) fn write(
+pub(super) fn write<'a, II, IP>(
 	data: &mut File,
-	tag: &mut VorbisCommentsRef<'_>,
+	tag: &mut VorbisCommentsRef<'a, II, IP>,
 	format: OGGFormat,
-) -> Result<()> {
+) -> Result<()>
+where
+	II: Iterator<Item = (&'a str, &'a str)>,
+	IP: Iterator<Item = (&'a crate::types::picture::Picture, PictureInformation)>,
+{
 	let first_page = Page::read(data, false)?;
 
 	let ser = first_page.serial;

src/tag_utils.rs

@@ -15,7 +15,7 @@ use crate::iff::wav::tag::RiffInfoListRef;
 #[cfg(feature = "mp4_ilst")]
 use crate::mp4::ilst::IlstRef;
 #[cfg(feature = "vorbis_comments")]
-use crate::ogg::tag::VorbisCommentsRef;
+use crate::ogg::tag::{create_vorbis_comments_ref, VorbisCommentsRef};
 
 use crate::{ape, iff, mp3, mp4, ogg};
 
@@ -32,7 +32,15 @@ pub(crate) fn write_tag(tag: &Tag, file: &mut File, file_type: FileType) -> Resu
 		FileType::AIFF => iff::aiff::write::write_to(file, tag),
 		FileType::APE => ape::write::write_to(file, tag),
 		#[cfg(feature = "vorbis_comments")]
-		FileType::FLAC => ogg::flac::write::write_to(file, &mut Into::<VorbisCommentsRef<'_>>::into(tag)),
+		FileType::FLAC => ogg::flac::write::write_to(file, &mut {
+			let (vendor, items, pictures) = create_vorbis_comments_ref(tag);
+
+			VorbisCommentsRef {
+				vendor,
+				items,
+				pictures,
+			}
+		}),
 		FileType::MP3 => mp3::write::write_to(file, tag),
 		#[cfg(feature = "mp4_ilst")]
 		FileType::MP4 => mp4::ilst::write::write_to(file, &mut Into::<IlstRef<'_>>::into(tag)),
@@ -63,9 +71,20 @@ pub(crate) fn dump_tag<W: Write>(tag: &Tag, writer: &mut W) -> Result<()> {
 		#[cfg(feature = "mp4_ilst")]
 		TagType::Mp4Ilst => Into::<IlstRef<'_>>::into(tag).dump_to(writer),
 		#[cfg(feature = "vorbis_comments")]
-		TagType::VorbisComments => Into::<VorbisCommentsRef<'_>>::into(tag).dump_to(writer),
+		TagType::VorbisComments => {
+			let (vendor, items, pictures) = create_vorbis_comments_ref(tag);
+
+			VorbisCommentsRef {
+				vendor,
+				items,
+				pictures,
+			}
+			.dump_to(writer)
+		},
 		#[cfg(feature = "riff_info_list")]
-		TagType::RiffInfo => Into::<RiffInfoListRef<'_>>::into(tag).dump_to(writer),
+		TagType::RiffInfo => {
+			RiffInfoListRef::new(iff::wav::tag::tagitems_into_riff(tag.items())).dump_to(writer)
+		},
 		#[cfg(feature = "aiff_text_chunks")]
 		TagType::AiffText => AiffTextChunksRef::new(
 			tag.get_string(&ItemKey::TrackTitle),