Cleanup id3::v2

src/id3/v2/frame/id.rs

@@ -49,8 +49,13 @@ impl TryFrom<ItemKey> for FrameID {
 
 	fn try_from(value: ItemKey) -> std::prelude::rust_2015::Result<Self, Self::Error> {
 		match value {
-			ItemKey::Unknown(unknown) if unknown.len() == 4 && unknown.is_ascii() => {
-				Ok(Self::Valid(unknown.to_ascii_uppercase()))
+			ItemKey::Unknown(unknown)
+				if unknown.len() == 4
+					&& unknown
+						.chars()
+						.all(|c| ('A'..='Z').contains(&c) || ('0'..='9').contains(&c)) =>
+			{
+				Ok(Self::Valid(unknown))
 			},
 			k => k.map_key(TagType::Id3v2, false).map_or(
 				Err(LoftyError::Id3v2(

src/id3/v2/tag.rs

@@ -283,7 +283,8 @@ impl Id3v2Tag {
 	/// * Attempting to write the tag to a format that does not support it
 	/// * Attempting to write an encrypted frame without a valid method symbol or data length indicator
 	pub fn write_to(&self, file: &mut File) -> Result<()> {
-		Into::<Id3v2TagRef>::into(self).write_to(file)
+		Id3v2TagRef::new(self.flags, self.frames.iter().filter_map(Frame::as_opt_ref))
+			.write_to(file)
 	}
 
 	/// Dumps the tag to a writer
@@ -292,7 +293,8 @@ impl Id3v2Tag {
 	///
 	/// * [`std::io::Error`]
 	pub fn dump_to<W: Write>(&self, writer: &mut W) -> Result<()> {
-		Into::<Id3v2TagRef>::into(self).dump_to(writer)
+		Id3v2TagRef::new(self.flags, self.frames.iter().filter_map(Frame::as_opt_ref))
+			.dump_to(writer)
 	}
 }
 
@@ -384,7 +386,7 @@ impl From<Tag> for Id3v2Tag {
 				Err(_) => continue,
 			};
 
-			id3v2_tag.frames.push(frame);
+			id3v2_tag.insert(frame);
 		}
 
 		for picture in input.pictures {
@@ -402,12 +404,26 @@ impl From<Tag> for Id3v2Tag {
 	}
 }
 
-pub(crate) struct Id3v2TagRef<'a> {
+pub(crate) struct Id3v2TagRef<'a, I: Iterator<Item = FrameRef<'a>> + 'a> {
 	pub(crate) flags: Id3v2TagFlags,
-	pub(crate) frames: Box<dyn Iterator<Item = FrameRef<'a>> + 'a>,
+	pub(crate) frames: I,
 }
 
-impl<'a> Id3v2TagRef<'a> {
+// Create an iterator of FrameRef from a Tag's items for Id3v2TagRef::new
+pub(crate) fn tag_frames(tag: &Tag) -> impl Iterator<Item = FrameRef<'_>> + '_ {
+	tag.items()
+		.iter()
+		.map(TryInto::<FrameRef>::try_into)
+		.filter_map(Result::ok)
+}
+
+impl<'a, I: Iterator<Item = FrameRef<'a>> + 'a> Id3v2TagRef<'a, I> {
+	pub(crate) fn new(flags: Id3v2TagFlags, frames: I) -> Self {
+		Self { flags, frames }
+	}
+}
+
+impl<'a, I: Iterator<Item = FrameRef<'a>> + 'a> Id3v2TagRef<'a, I> {
 	pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> {
 		super::write::write_id3v2(file, self)
 	}
@@ -420,29 +436,6 @@ impl<'a> Id3v2TagRef<'a> {
 	}
 }
 
-impl<'a> Into<Id3v2TagRef<'a>> for &'a Tag {
-	fn into(self) -> Id3v2TagRef<'a> {
-		Id3v2TagRef {
-			flags: Id3v2TagFlags::default(),
-			frames: Box::new(
-				self.items()
-					.iter()
-					.map(TryInto::<FrameRef>::try_into)
-					.filter_map(Result::ok),
-			),
-		}
-	}
-}
-
-impl<'a> Into<Id3v2TagRef<'a>> for &'a Id3v2Tag {
-	fn into(self) -> Id3v2TagRef<'a> {
-		Id3v2TagRef {
-			flags: self.flags,
-			frames: Box::new(self.frames.iter().filter_map(Frame::as_opt_ref)),
-		}
-	}
-}
-
 #[cfg(test)]
 mod tests {
 	use crate::id3::v2::{

src/id3/v2/write/mod.rs

@@ -4,6 +4,7 @@ mod frame;
 use super::Id3v2TagFlags;
 use crate::error::{LoftyError, Result};
 use crate::id3::find_id3v2;
+use crate::id3::v2::frame::FrameRef;
 use crate::id3::v2::synch_u32;
 use crate::id3::v2::tag::Id3v2TagRef;
 use crate::probe::Probe;
@@ -12,25 +13,30 @@ use crate::types::file::FileType;
 use std::fs::File;
 use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 
-use byteorder::{BigEndian, ByteOrder, LittleEndian, WriteBytesExt};
+use byteorder::{BigEndian, LittleEndian, WriteBytesExt};
 
 #[allow(clippy::shadow_unrelated)]
-pub(crate) fn write_id3v2(data: &mut File, tag: &mut Id3v2TagRef) -> Result<()> {
+pub(crate) fn write_id3v2<'a, I: Iterator<Item = FrameRef<'a>> + 'a>(
+	data: &mut File,
+	tag: &mut Id3v2TagRef<'a, I>,
+) -> Result<()> {
 	let probe = Probe::new(data).guess_file_type()?;
+	let file_type = probe.file_type();
 
-	match probe.file_type() {
+	let data = probe.into_inner();
+
+	match file_type {
 		Some(FileType::APE | FileType::MP3) => {},
+		// Formats such as WAV and AIFF store the ID3v2 tag in an 'ID3 ' chunk rather than at the beginning of the file
 		Some(FileType::WAV) => {
-			return write_id3v2_to_chunk_file::<LittleEndian>(probe.into_inner(), tag)
+			return chunk_file::write_to_chunk_file::<LittleEndian>(data, &create_tag(tag)?)
 		},
 		Some(FileType::AIFF) => {
-			return write_id3v2_to_chunk_file::<BigEndian>(probe.into_inner(), tag)
+			return chunk_file::write_to_chunk_file::<BigEndian>(data, &create_tag(tag)?)
 		},
 		_ => return Err(LoftyError::UnsupportedTag),
 	}
 
-	let data = probe.into_inner();
-
 	let id3v2 = create_tag(tag)?;
 
 	// find_id3v2 will seek us to the end of the tag
@@ -48,19 +54,9 @@ pub(crate) fn write_id3v2(data: &mut File, tag: &mut Id3v2TagRef) -> Result<()>
 	Ok(())
 }
 
-// Formats such as WAV and AIFF store the ID3v2 tag in an 'ID3 ' chunk rather than at the beginning of the file
-fn write_id3v2_to_chunk_file<B>(data: &mut File, tag: &mut Id3v2TagRef) -> Result<()>
-where
-	B: ByteOrder,
-{
-	let id3v2 = create_tag(tag)?;
-
-	chunk_file::write_to_chunk_file::<B>(data, &id3v2)?;
-
-	Ok(())
-}
-
-pub(super) fn create_tag(tag: &mut Id3v2TagRef) -> Result<Vec<u8>> {
+pub(super) fn create_tag<'a, I: Iterator<Item = FrameRef<'a>> + 'a>(
+	tag: &mut Id3v2TagRef<'a, I>,
+) -> Result<Vec<u8>> {
 	let frames = &mut tag.frames;
 	let mut peek = frames.peekable();
 

src/iff/aiff/write.rs

@@ -1,6 +1,9 @@
 use crate::error::{LoftyError, Result};
 #[cfg(feature = "id3v2")]
-use crate::id3::v2::tag::Id3v2TagRef;
+use crate::id3::v2::{
+	tag::{tag_frames, Id3v2TagRef},
+	Id3v2TagFlags,
+};
 #[cfg(feature = "aiff_text_chunks")]
 use crate::iff::aiff::tag::AiffTextChunksRef;
 use crate::types::item::ItemKey;
@@ -22,7 +25,7 @@ pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 		)
 		.write_to(data),
 		#[cfg(feature = "id3v2")]
-		TagType::Id3v2 => Into::<Id3v2TagRef>::into(tag).write_to(data),
+		TagType::Id3v2 => Id3v2TagRef::new(Id3v2TagFlags::default(), tag_frames(tag)).write_to(data),
 		_ => Err(LoftyError::UnsupportedTag),
 	}
 }

src/iff/wav/write.rs

@@ -1,6 +1,9 @@
 use crate::error::{LoftyError, Result};
 #[cfg(feature = "id3v2")]
-use crate::id3::v2::tag::Id3v2TagRef;
+use crate::id3::v2::{
+	tag::{tag_frames, Id3v2TagRef},
+	Id3v2TagFlags,
+};
 #[cfg(feature = "riff_info_list")]
 use crate::iff::wav::tag::RiffInfoListRef;
 #[allow(unused_imports)]
@@ -14,7 +17,7 @@ pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 		#[cfg(feature = "riff_info_list")]
 		TagType::RiffInfo => Into::<RiffInfoListRef>::into(tag).write_to(data),
 		#[cfg(feature = "id3v2")]
-		TagType::Id3v2 => Into::<Id3v2TagRef>::into(tag).write_to(data),
+		TagType::Id3v2 => Id3v2TagRef::new(Id3v2TagFlags::default(), tag_frames(tag)).write_to(data),
 		_ => Err(LoftyError::UnsupportedTag),
 	}
 }

src/mp3/write.rs

@@ -4,7 +4,10 @@ use crate::error::{LoftyError, Result};
 #[cfg(feature = "id3v1")]
 use crate::id3::v1::tag::Id3v1TagRef;
 #[cfg(feature = "id3v2")]
-use crate::id3::v2::tag::Id3v2TagRef;
+use crate::id3::v2::{
+	tag::{tag_frames, Id3v2TagRef},
+	Id3v2TagFlags,
+};
 #[allow(unused_imports)]
 use crate::types::tag::{Tag, TagType};
 
@@ -18,7 +21,7 @@ pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 		#[cfg(feature = "id3v1")]
 		TagType::Id3v1 => Into::<Id3v1TagRef>::into(tag).write_to(data),
 		#[cfg(feature = "id3v2")]
-		TagType::Id3v2 => Into::<Id3v2TagRef>::into(tag).write_to(data),
+		TagType::Id3v2 => Id3v2TagRef::new(Id3v2TagFlags::default(), tag_frames(tag)).write_to(data),
 		_ => Err(LoftyError::UnsupportedTag),
 	}
 }

src/tag_utils.rs

@@ -4,7 +4,10 @@ use crate::error::{LoftyError, Result};
 #[cfg(feature = "id3v1")]
 use crate::id3::v1::tag::Id3v1TagRef;
 #[cfg(feature = "id3v2")]
-use crate::id3::v2::tag::Id3v2TagRef;
+use crate::id3::v2::{
+	tag::{tag_frames, Id3v2TagRef},
+	Id3v2TagFlags,
+};
 #[cfg(feature = "aiff_text_chunks")]
 use crate::iff::aiff::tag::AiffTextChunksRef;
 #[cfg(feature = "riff_info_list")]
@@ -52,7 +55,7 @@ pub(crate) fn dump_tag<W: Write>(tag: &Tag, writer: &mut W) -> Result<()> {
 		#[cfg(feature = "id3v1")]
 		TagType::Id3v1 => Into::<Id3v1TagRef>::into(tag).dump_to(writer),
 		#[cfg(feature = "id3v2")]
-		TagType::Id3v2 => Into::<Id3v2TagRef>::into(tag).dump_to(writer),
+		TagType::Id3v2 => Id3v2TagRef::new(Id3v2TagFlags::default(), tag_frames(tag)).dump_to(writer),
 		#[cfg(feature = "mp4_ilst")]
 		TagType::Mp4Ilst => Into::<IlstRef>::into(tag).dump_to(writer),
 		#[cfg(feature = "vorbis_comments")]