Cleanup

README.md

@@ -8,17 +8,17 @@ Parse, convert, and write metadata to various audio formats.
 
 ## Supported Formats
 
-| File Format | Extensions                                      | Read  | Write | Metadata Format(s)                             |
-|-------------|-------------------------------------------------|-------|-------|------------------------------------------------|
-| Ape         | `ape`                                           | **X** | **X** | `APEv2`, `APEv1`, `ID3v2` (Read only), `ID3v1` |
-| AIFF        | `aiff`, `aif`, `aifc`, `afc`                    | **X** | **X** | `ID3v2`, `Text Chunks`                         |
-| FLAC        | `flac`                                          | **X** | **X** | `Vorbis Comments`                              |
-| MP3         | `mp3`                                           | **X** | **X** | `ID3v2`, `ID3v1`, `APEv2`, `APEv1`             |
-| MP4         | `mp4`, `m4a`, `m4b`, `m4p`, `m4r`, `m4v`, `3gp` | **X** | **X** | `iTunes-style ilst`                            |
-| Opus        | `opus`                                          | **X** | **X** | `Vorbis Comments`                              |
-| Ogg Vorbis  | `ogg`                                           | **X** | **X** | `Vorbis Comments`                              |
-| Speex       | `spx`                                           | **X** | **X** | `Vorbis Comments`                              |
-| WAV         | `wav`, `wave`                                   | **X** | **X** | `ID3v2`, `RIFF INFO`                           |
+| File Format | Metadata Format(s)                             |
+|-------------|------------------------------------------------|
+| Ape         | `APEv2`, `APEv1`, `ID3v2` (Read only), `ID3v1` |
+| AIFF        | `ID3v2`, `Text Chunks`                         |
+| FLAC        | `Vorbis Comments`                              |
+| MP3         | `ID3v2`, `ID3v1`, `APEv2`, `APEv1`             |
+| MP4         | `iTunes-style ilst`                            |
+| Opus        | `Vorbis Comments`                              |
+| Ogg Vorbis  | `Vorbis Comments`                              |
+| Speex       | `Vorbis Comments`                              |
+| WAV         | `ID3v2`, `RIFF INFO`                           |
 
 ## Examples
 

rustfmt.toml

@@ -9,3 +9,4 @@ format_strings = true
 normalize_comments = true
 match_block_trailing_comma = true
 hard_tabs = true
+newline_style = "Unix"

src/ape/tag/ape_tag.rs

@@ -139,7 +139,10 @@ impl TagIO for ApeTag {
 	/// * Attempting to write the tag to a format that does not support it
 	/// * An existing tag has an invalid size
 	fn save_to(&self, file: &mut File) -> std::result::Result<(), Self::Err> {
-		Into::<ApeTagRef>::into(self).write_to(file)
+		ApeTagRef {
+			read_only: self.read_only,
+			items: self.items.iter().map(Into::into),
+		}.write_to(file)
 	}
 
 	/// Dumps the tag to a writer
@@ -148,7 +151,10 @@ impl TagIO for ApeTag {
 	///
 	/// * [`std::io::Error`]
 	fn dump_to<W: Write>(&self, writer: &mut W) -> std::result::Result<(), Self::Err> {
-		Into::<ApeTagRef>::into(self).dump_to(writer)
+		ApeTagRef {
+			read_only: self.read_only,
+			items: self.items.iter().map(Into::into),
+		}.dump_to(writer)
 	}
 
 	fn remove_from_path<P: AsRef<Path>>(&self, path: P) -> std::result::Result<(), Self::Err> {
@@ -232,12 +238,13 @@ impl From<Tag> for ApeTag {
 	}
 }
 
-pub(crate) struct ApeTagRef<'a> {
+pub(crate) struct ApeTagRef<'a, I> where I: Iterator<Item = ApeItemRef<'a>>{
 	pub(crate) read_only: bool,
-	pub(super) items: Box<dyn Iterator<Item = ApeItemRef<'a>> + 'a>,
+	pub(crate) items: I,
 }
 
-impl<'a> ApeTagRef<'a> {
+impl<'a, I> ApeTagRef<'a, I>
+where I: Iterator<Item = ApeItemRef<'a>> {
 	pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> {
 		super::write::write_to(file, self)
 	}
@@ -250,28 +257,14 @@ impl<'a> ApeTagRef<'a> {
 	}
 }
 
-impl<'a> Into<ApeTagRef<'a>> for &'a Tag {
-	fn into(self) -> ApeTagRef<'a> {
-		ApeTagRef {
-			read_only: false,
-			items: Box::new(self.items.iter().filter_map(|i| {
+pub(crate) fn tagitems_into_ape(items: &[TagItem]) -> impl Iterator<Item = ApeItemRef> {
+	items.iter().filter_map(|i| {
 		i.key().map_key(TagType::Ape, true).map(|key| ApeItemRef {
 			read_only: false,
 			key,
 			value: (&i.item_value).into(),
 		})
-			})),
-		}
-	}
-}
-
-impl<'a> Into<ApeTagRef<'a>> for &'a ApeTag {
-	fn into(self) -> ApeTagRef<'a> {
-		ApeTagRef {
-			read_only: self.read_only,
-			items: Box::new(self.items.iter().map(Into::into)),
-		}
-	}
+	})
 }
 
 #[cfg(test)]

src/ape/tag/write.rs

@@ -1,6 +1,7 @@
 use super::read::read_ape_tag;
 use crate::ape::constants::APE_PREAMBLE;
-use crate::ape::tag::ape_tag::ApeTagRef;
+use super::ape_tag::ApeTagRef;
+use super::item::ApeItemRef;
 use crate::error::{ErrorKind, FileDecodingError, LoftyError, Result};
 use crate::id3::{find_id3v1, find_id3v2, find_lyrics3v2};
 use crate::probe::Probe;
@@ -14,7 +15,7 @@ use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 use byteorder::{LittleEndian, WriteBytesExt};
 
 #[allow(clippy::shadow_unrelated)]
-pub(crate) fn write_to(data: &mut File, tag: &mut ApeTagRef) -> Result<()> {
+pub(crate) fn write_to<'a, I>(data: &mut File, tag: &mut ApeTagRef<'a, I>) -> Result<()> where I: Iterator<Item = ApeItemRef<'a>>{
 	let probe = Probe::new(data).guess_file_type()?;
 
 	match probe.file_type() {
@@ -102,7 +103,10 @@ pub(crate) fn write_to(data: &mut File, tag: &mut ApeTagRef) -> Result<()> {
 
 	// Preserve any metadata marked as read only
 	let tag = if let Some(read_only) = read_only {
-		create_ape_tag(&mut Into::<ApeTagRef>::into(&read_only))?
+		create_ape_tag(&mut ApeTagRef {
+			read_only: read_only.read_only,
+			items: read_only.items.iter().map(Into::into),
+		})?
 	} else {
 		create_ape_tag(tag)?
 	};
@@ -131,7 +135,7 @@ pub(crate) fn write_to(data: &mut File, tag: &mut ApeTagRef) -> Result<()> {
 	Ok(())
 }
 
-pub(super) fn create_ape_tag(tag: &mut ApeTagRef) -> Result<Vec<u8>> {
+pub(super) fn create_ape_tag<'a, I>(tag: &mut ApeTagRef<'a, I>) -> Result<Vec<u8>> where I: Iterator<Item = ApeItemRef<'a>>{
 	let items = &mut tag.items;
 	let mut peek = items.peekable();
 

src/ape/write.rs

@@ -12,7 +12,11 @@ use std::fs::File;
 pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 	match tag.tag_type() {
 		#[cfg(feature = "ape")]
-		TagType::Ape => Into::<ape_tag::ApeTagRef>::into(tag).write_to(data),
+		TagType::Ape => ape_tag::ApeTagRef {
+			read_only: false,
+			items: ape_tag::tagitems_into_ape(tag.items()),
+		}
+		.write_to(data),
 		#[cfg(feature = "id3v1")]
 		TagType::Id3v1 => Into::<v1::tag::Id3v1TagRef>::into(tag).write_to(data),
 		_ => Err(LoftyError::new(ErrorKind::UnsupportedTag)),

src/iff/wav/tag/write.rs

@@ -85,8 +85,8 @@ pub(super) fn create_riff_info(
 		return Ok(());
 	}
 
-	bytes.extend(b"LIST".iter());
-	bytes.extend(b"INFO".iter());
+	bytes.extend(b"LIST");
+	bytes.extend(b"INFO");
 
 	for (k, v) in items {
 		if v.is_empty() {
@@ -100,10 +100,10 @@ pub(super) fn create_riff_info(
 		// Each value has to be null terminated and have an even length
 		let terminator: &[u8] = if len % 2 == 0 { &[0] } else { &[0, 0] };
 
-		bytes.extend(k.as_bytes().iter());
-		bytes.extend((len as u32).to_le_bytes().iter());
-		bytes.extend(val_b.iter());
-		bytes.extend(terminator.iter());
+		bytes.extend(k.as_bytes());
+		bytes.extend(&(len as u32).to_le_bytes());
+		bytes.extend(val_b);
+		bytes.extend(terminator);
 	}
 
 	let packet_size = bytes.len() - 4;

src/mp3/header.rs

@@ -103,7 +103,7 @@ pub(crate) struct Header {
 }
 
 impl Header {
-	pub fn read(header: u32) -> Result<Self> {
+	pub(crate) fn read(header: u32) -> Result<Self> {
 		let version = match (header >> 19) & 0b11 {
 			0 => MpegVersion::V2_5,
 			2 => MpegVersion::V2,
@@ -200,7 +200,7 @@ pub(crate) struct XingHeader {
 }
 
 impl XingHeader {
-	pub fn read(reader: &mut &[u8]) -> Result<Self> {
+	pub(crate) fn read(reader: &mut &[u8]) -> Result<Self> {
 		let reader_len = reader.len();
 
 		let mut header = [0; 4];
@@ -268,7 +268,7 @@ mod tests {
 		fn test(data: &[u8], expected_result: Option<u64>) {
 			use super::search_for_frame_sync;
 			assert_eq!(
-				search_for_frame_sync(&mut Box::new(data)).unwrap(),
+				search_for_frame_sync(&mut &*data).unwrap(),
 				expected_result
 			);
 		}

src/mp3/write.rs

@@ -14,7 +14,11 @@ use std::fs::File;
 pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 	match tag.tag_type() {
 		#[cfg(feature = "ape")]
-		TagType::Ape => Into::<ape_tag::ApeTagRef>::into(tag).write_to(data),
+		TagType::Ape => ape_tag::ApeTagRef {
+			read_only: false,
+			items: ape_tag::tagitems_into_ape(tag.items()),
+		}
+		.write_to(data),
 		#[cfg(feature = "id3v1")]
 		TagType::Id3v1 => Into::<v1::tag::Id3v1TagRef>::into(tag).write_to(data),
 		#[cfg(feature = "id3v2")]

src/ogg/mod.rs

@@ -1,4 +1,4 @@
-//! OPUS/FLAC/Vorbis specific items
+//! Items for OGG container formats
 //!
 //! ## File notes
 //!

src/ogg/read.rs

@@ -86,7 +86,7 @@ where
 
 	let mut md_pages: Vec<u8> = Vec::new();
 
-	md_pages.extend(md_page.content()[comment_sig.len()..].iter());
+	md_pages.extend_from_slice(&md_page.content()[comment_sig.len()..]);
 
 	while let Ok(page) = Page::read(data, false) {
 		if md_pages.len() > 125_829_120 {
@@ -94,7 +94,7 @@ where
 		}
 
 		if page.header_type() & 0x01 == 1 {
-			md_pages.extend(page.content().iter());
+			md_pages.extend_from_slice(page.content());
 		} else {
 			data.seek(SeekFrom::Start(page.start))?;
 			break;
@@ -103,11 +103,7 @@ where
 
 	#[cfg(feature = "vorbis_comments")]
 	{
-		let mut tag = VorbisComments {
-			vendor: String::new(),
-			items: vec![],
-			pictures: vec![],
-		};
+		let mut tag = VorbisComments::default();
 
 		let reader = &mut &md_pages[..];
 		read_comments(reader, &mut tag)?;

src/ogg/vorbis/write.rs

@@ -112,7 +112,7 @@ pub(crate) fn write_to(
 
 			// Replace segment table and checksum
 			p_bytes.splice(26..27 + seg_count, seg_table);
-			p_bytes.splice(22..26, ogg_pager::crc32(&*p_bytes).to_le_bytes().to_vec());
+			p_bytes.splice(22..26, ogg_pager::crc32(&*p_bytes).to_le_bytes());
 
 			writer.write_all(&*p_bytes)?;
 

src/ogg/write.rs

@@ -46,7 +46,10 @@ pub(crate) fn create_comments(
 	items: &mut dyn Iterator<Item = (&str, &str)>,
 ) -> Result<()> {
 	for (k, v) in items {
-		if !v.is_empty() {
+		if v.is_empty() {
+			continue;
+		}
+
 		let comment = format!("{}={}", k, v);
 
 		let comment_b = comment.as_bytes();
@@ -59,7 +62,6 @@ pub(crate) fn create_comments(
 			packet.write_all(comment_b)?;
 		}
 	}
-	}
 
 	Ok(())
 }
@@ -115,14 +117,12 @@ pub(super) fn write(data: &mut File, tag: &mut VorbisCommentsRef, format: OGGFor
 	let first_md_page = Page::read(data, false)?;
 
 	let comment_signature = format.comment_signature();
-	let verify_sig = comment_signature.is_some();
-
-	let comment_signature = format.comment_signature().unwrap_or(&[]);
-
-	if verify_sig {
+	if let Some(comment_signature) = comment_signature {
 		verify_signature(&first_md_page, comment_signature)?;
 	}
 
+	let comment_signature = comment_signature.unwrap_or_default();
+
 	// Retain the file's vendor string
 	let md_reader = &mut &first_md_page.content()[comment_signature.len()..];
 

src/tag_utils.rs

@@ -51,7 +51,11 @@ pub(crate) fn write_tag(tag: &Tag, file: &mut File, file_type: FileType) -> Resu
 pub(crate) fn dump_tag<W: Write>(tag: &Tag, writer: &mut W) -> Result<()> {
 	match tag.tag_type() {
 		#[cfg(feature = "ape")]
-		TagType::Ape => Into::<ApeTagRef>::into(tag).dump_to(writer),
+		TagType::Ape => ApeTagRef {
+			read_only: false,
+			items: ape::tag::ape_tag::tagitems_into_ape(tag.items()),
+		}
+		.dump_to(writer),
 		#[cfg(feature = "id3v1")]
 		TagType::Id3v1 => Into::<Id3v1TagRef>::into(tag).dump_to(writer),
 		#[cfg(feature = "id3v2")]

src/types/file.rs

@@ -367,7 +367,7 @@ impl FileType {
 			79 if buf.len() >= 36 && &buf[..4] == b"OggS" => {
 				if &buf[29..35] == b"vorbis" {
 					return Some(Self::Vorbis);
-				} else if &buf[28..36] == b"OpusHead" {
+				} else if &buf[28..] == b"OpusHead" {
 					return Some(Self::Opus);
 				} else if &buf[28..] == b"Speex   " {
 					return Some(Self::Speex);

src/types/picture.rs

@@ -853,14 +853,17 @@ impl Picture {
 	/// This function will return [`NotAPicture`](ErrorKind::NotAPicture)
 	/// if at any point it's unable to parse the data
 	pub fn from_ape_bytes(key: &str, bytes: &[u8]) -> Result<Self> {
-		if !bytes.is_empty() {
+		if bytes.is_empty() {
+			return Err(LoftyError::new(ErrorKind::NotAPicture));
+		}
+
 		let pic_type = PictureType::from_ape_key(key);
 
 		let reader = &mut &*bytes;
 		let mut pos = 0;
 
-			let description = {
-				let mut text = String::new();
+		let mut description = None;
+		let mut desc_text = String::new();
 
 		while let Ok(ch) = reader.read_u8() {
 			pos += 1;
@@ -869,11 +872,12 @@ impl Picture {
 				break;
 			}
 
-					text.push(char::from(ch));
+			desc_text.push(char::from(ch));
 		}
 
-				(!text.is_empty()).then(|| Cow::from(text))
-			};
+		if !desc_text.is_empty() {
+			description = Some(Cow::from(desc_text));
+		}
 
 		let mime_type = {
 			let mut identifier = [0; 8];
@@ -884,15 +888,12 @@ impl Picture {
 
 		let data = Cow::from(bytes[pos..].to_vec());
 
-			return Ok(Picture {
+		Ok(Picture {
 			pic_type,
 			mime_type,
 			description,
 			data,
-			});
-		}
-
-		Err(LoftyError::new(ErrorKind::NotAPicture))
+		})
 	}
 
 	fn mimetype_from_bin(bytes: &[u8]) -> Result<MimeType> {