Break up logic::id3::v1, improve doc comments

src/lib.rs

@@ -105,7 +105,7 @@
 //! ## Utilities
 //! * `id3v2_restrictions` - Parses ID3v2 extended headers and exposes flags for fine grained control
 //!
-//! # Notes on ID3v2
+//! # Notes on ID3
 //!
 //! See [`id3`](crate::id3) for important warnings and notes on reading tags.
 
@@ -158,52 +158,85 @@ pub mod files {
 #[cfg(any(feature = "id3v1", feature = "id3v2"))]
 /// ID3v1/v2 specific items
 pub mod id3 {
-	//! # ID3v2 notes and warnings
+	//! ID3 does things differently than other tags, making working with them a little more effort than other formats.
-	//!
+	//! Check the other modules for important notes and/or warnings.
-	//! ID3v2 does things differently than other formats.
+
-	//!
+	#[cfg(feature = "id3v2")]
-	//! ## Unknown Keys
+	pub mod v2 {
-	//!
+		//! ID3v2 items and utilities
-	//! ID3v2 **does not** support [`ItemKey::Unknown`](crate::ItemKey::Unknown) and they will be ignored.
+		//!
-	//! Instead, [`ItemKey::Id3v2Specific`](crate::ItemKey::Id3v2Specific) with an [`Id3v2Frame`](crate::id3::Id3v2Frame) variant must be used.
+		//! # ID3v2 notes and warnings
-	//!
+		//!
-	//! ## Frame ID mappings
+		//! ID3v2 does things differently than other formats.
-	//!
+		//!
-	//! Certain [`ItemKey`](crate::ItemKey)s are unable to map to an ID3v2 frame, as they are a part of a larger
+		//! ## Unknown Keys
-	//! collection (such as `TIPL` and `TMCL`).
+		//!
-	//!
+		//! ID3v2 **does not** support [`ItemKey::Unknown`](crate::ItemKey::Unknown) and they will be ignored.
-	//! For example, if the key is `Arranger` (part of `TIPL`), there is no mapping available.
+		//! Instead, [`ItemKey::Id3v2Specific`](crate::ItemKey::Id3v2Specific) with an [`Id3v2Frame`](crate::id3::v2::Id3v2Frame) variant must be used.
-	//!
+		//!
-	//! In this case, the caller is expected to build these lists. If these [`ItemKey`](crate::ItemKey)s are inserted
+		//! ## Frame ID mappings
-	//! using [`Tag::insert_item_unchecked`], they will simply be ignored.
+		//!
-	//!
+		//! Certain [`ItemKey`](crate::ItemKey)s are unable to map to an ID3v2 frame, as they are a part of a larger
-	//! ## Special frames
+		//! collection (such as `TIPL` and `TMCL`).
-	//!
+		//!
-	//! ID3v2 has multiple frames that have no equivalent in other formats:
+		//! For example, if the key is `Arranger` (part of `TIPL`), there is no mapping available.
-	//!
+		//!
-	//! * COMM - Comments (Unlike comments in other formats)
+		//! In this case, the caller is expected to build these lists. If these [`ItemKey`](crate::ItemKey)s are inserted
-	//! * USLT - Unsynchronized text (Unlike lyrics/text in other formats)
+		//! using [`Tag::insert_item_unchecked`](crate::Tag::insert_item_unchecked), they will simply be ignored.
-	//! * TXXX - User defined text
+		//!
-	//! * WXXX - User defined URL
+		//! ## Special frames
-	//! * SYLT - Synchronized text
+		//!
-	//! * GEOB - Encapsulated object (file)
+		//! ID3v2 has multiple frames that have no equivalent in other formats:
-	//!
+		//!
-	//! These frames all require different amounts of information, so they cannot be mapped to a traditional [`ItemKey`](crate::ItemKey) variant.
+		//! * COMM - Comments (Unlike comments in other formats)
-	//! The solution is to use [`ItemKey::Id3v2Specific`](crate::ItemKey::Id3v2Specific) alongside [`Id3v2Frame`](crate::id3::Id3v2Frame).
+		//! * USLT - Unsynchronized text (Unlike lyrics/text in other formats)
-	//!
+		//! * TXXX - User defined text
-	//! NOTE: Unlike the above issue, this one does not require unchecked insertion.
+		//! * WXXX - User defined URL
-	pub use crate::logic::id3::v2::frame::{Id3v2Frame, LanguageSpecificFrame};
+		//! * SYLT - Synchronized text
-	pub use crate::logic::id3::v2::items::encapsulated_object::{
+		//! * GEOB - Encapsulated object (file)
-		GEOBInformation, GeneralEncapsulatedObject,
+		//!
-	};
+		//! These frames all require different amounts of information, so they cannot be mapped to a traditional [`ItemKey`](crate::ItemKey) variant.
-	#[cfg(feature = "id3v2_restrictions")]
+		//! The solution is to use [`ItemKey::Id3v2Specific`](crate::ItemKey::Id3v2Specific) alongside [`Id3v2Frame`](crate::id3::v2::Id3v2Frame).
-	pub use crate::logic::id3::v2::items::restrictions::*;
+		//!
-	pub use crate::logic::id3::v2::items::sync_text::{
+		//! NOTE: Unlike the above issue, this one does not require unchecked insertion.
-		SyncTextContentType, SyncTextInformation, SynchronizedText, TimestampFormat,
+
-	};
+		pub use {
-	pub use crate::logic::id3::v2::util::text_utils::TextEncoding;
+			crate::logic::id3::v2::frame::{Id3v2Frame, LanguageSpecificFrame},
-	pub use crate::logic::id3::v2::util::upgrade::{upgrade_v2, upgrade_v3};
+			crate::logic::id3::v2::items::encapsulated_object::{
-	pub use crate::logic::id3::v2::Id3v2Version;
+				GEOBInformation, GeneralEncapsulatedObject,
+			},
+			crate::logic::id3::v2::items::sync_text::{
+				SyncTextContentType, SyncTextInformation, SynchronizedText, TimestampFormat,
+			},
+			crate::logic::id3::v2::util::text_utils::TextEncoding,
+			crate::logic::id3::v2::util::upgrade::{upgrade_v2, upgrade_v3},
+			crate::logic::id3::v2::Id3v2Version,
+		};
+
+		#[cfg(feature = "id3v2_restrictions")]
+		pub use crate::logic::id3::v2::items::restrictions::*;
+	}
+
+	#[cfg(feature = "id3v1")]
+	pub mod v1 {
+		//! ID3v1 items
+		//!
+		//! # ID3v1 notes
+		//!
+		//! ## Genres
+		//!
+		//! ID3v1 stores the genre in a single byte ranging from 0 to 192.
+		//! The number can be stored in any of the following [`ItemValue`](crate::ItemValue) variants: `Text, UInt, UInt64, Int, Int64`, and will be discarded if it is unable to parse or is too big.
+		//! All possible genres have been stored in the [`GENRES`](crate::id3::v1::GENRES) constant.
+		//!
+		//! ## Track Numbers
+		//!
+		//! ID3v1 stores the track number in a non-zero byte.
+		//! A track number of 0 will be treated as an empty field.
+		//! Additionally, there is no track total field.
+
+		pub use crate::logic::id3::v1::constants::GENRES;
+	}
 }
 
 /// Various items related to [`Picture`](crate::picture::Picture)s

src/logic/ape/write.rs

@@ -6,7 +6,7 @@ use std::fs::File;
 pub(crate) fn write_to(data: &mut File, tag: &Tag) -> Result<()> {
 	match tag.tag_type() {
 		TagType::Ape => super::tag::write::write_to(data, tag),
-		TagType::Id3v1 => crate::logic::id3::v1::write_id3v1(data, tag),
+		TagType::Id3v1 => crate::logic::id3::v1::write::write_id3v1(data, tag),
 		TagType::Id3v2 => todo!(),
 		_ => Err(LoftyError::UnsupportedTag),
 	}

src/logic/id3/mod.rs

@@ -1,5 +1,3 @@
-mod constants;
-
 #[cfg(feature = "id3v1")]
 pub(crate) mod v1;
 

src/logic/id3/v1/constants.rs

@@ -1,4 +1,4 @@
-#[cfg(feature = "id3v1")]
+/// All possible genres for ID3v1
 pub const GENRES: [&str; 192] = [
 	"Blues",
 	"Classic rock",

src/logic/id3/v1/mod.rs

@@ -0,0 +1,41 @@
+pub(crate) mod constants;
+pub(in crate::logic) mod read;
+pub(in crate::logic) mod write;
+
+use crate::error::Result;
+use crate::types::tag::Tag;
+
+use std::io::{Read, Seek, SeekFrom};
+
+pub(in crate::logic) fn find_id3v1<R>(data: &mut R, read: bool) -> Result<(bool, Option<Tag>)>
+where
+	R: Read + Seek,
+{
+	let mut id3v1 = None;
+	let mut exists = false;
+
+	data.seek(SeekFrom::End(-128))?;
+
+	let mut id3v1_header = [0; 3];
+	data.read_exact(&mut id3v1_header)?;
+
+	data.seek(SeekFrom::Current(-3))?;
+
+	if &id3v1_header == b"TAG" {
+		exists = true;
+
+		if read {
+			let mut id3v1_tag = [0; 128];
+			data.read_exact(&mut id3v1_tag)?;
+
+			data.seek(SeekFrom::End(-128))?;
+
+			id3v1 = Some(read::parse_id3v1(id3v1_tag))
+		}
+	} else {
+		// No ID3v1 tag found
+		data.seek(SeekFrom::End(0))?;
+	}
+
+	Ok((exists, id3v1))
+}

src/logic/id3/v1/read.rs

@@ -0,0 +1,63 @@
+use super::constants::GENRES;
+use crate::types::item::{ItemKey, ItemValue, TagItem};
+use crate::types::tag::{Tag, TagType};
+
+pub fn parse_id3v1(reader: [u8; 128]) -> Tag {
+	let mut tag = Tag::new(TagType::Id3v1);
+
+	let reader = &reader[3..];
+
+	if let Some(title) = decode_text(ItemKey::TrackTitle, &reader[..30]) {
+		tag.insert_item_unchecked(title);
+	}
+
+	if let Some(artist) = decode_text(ItemKey::TrackArtist, &reader[30..60]) {
+		tag.insert_item_unchecked(artist);
+	}
+
+	if let Some(album) = decode_text(ItemKey::AlbumTitle, &reader[60..90]) {
+		tag.insert_item_unchecked(album);
+	}
+
+	if let Some(year) = decode_text(ItemKey::Year, &reader[90..94]) {
+		tag.insert_item_unchecked(year);
+	}
+
+	let range = if reader[119] == 0 && reader[122] != 0 {
+		tag.insert_item_unchecked(TagItem::new(
+			ItemKey::TrackNumber,
+			ItemValue::UInt(u32::from(reader[122])),
+		));
+
+		94_usize..123
+	} else {
+		94..124
+	};
+
+	if let Some(comment) = decode_text(ItemKey::Comment, &reader[range]) {
+		tag.insert_item_unchecked(comment);
+	}
+
+	if reader[124] < GENRES.len() as u8 {
+		tag.insert_item_unchecked(TagItem::new(
+			ItemKey::Genre,
+			ItemValue::Text(GENRES[reader[125] as usize].to_string()),
+		));
+	}
+
+	tag
+}
+
+fn decode_text(key: ItemKey, data: &[u8]) -> Option<TagItem> {
+	let read = data
+		.iter()
+		.filter(|c| **c != 0)
+		.map(|c| *c as char)
+		.collect::<String>();
+
+	if read.is_empty() {
+		None
+	} else {
+		Some(TagItem::new(key, ItemValue::Text(read)))
+	}
+}

src/logic/id3/v1/write.rs

@@ -1,112 +1,19 @@
-use super::constants::GENRES;
 use crate::error::Result;
 use crate::types::item::{ItemKey, ItemValue, TagItem};
-use crate::types::tag::{Tag, TagType};
+use crate::types::tag::Tag;
 
-use byteorder::WriteBytesExt;
 use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 
-pub(in crate::logic) fn find_id3v1<R>(data: &mut R, read: bool) -> Result<(bool, Option<Tag>)>
+use byteorder::WriteBytesExt;
-where
-	R: Read + Seek,
-{
-	let mut id3v1 = None;
-	let mut exists = false;
 
-	data.seek(SeekFrom::End(-128))?;
+pub fn write_id3v1<W>(writer: &mut W, tag: &Tag) -> Result<()>
-
-	let mut id3v1_header = [0; 3];
-	data.read_exact(&mut id3v1_header)?;
-
-	data.seek(SeekFrom::Current(-3))?;
-
-	if &id3v1_header == b"TAG" {
-		exists = true;
-
-		if read {
-			let mut id3v1_tag = [0; 128];
-			data.read_exact(&mut id3v1_tag)?;
-
-			data.seek(SeekFrom::End(-128))?;
-
-			id3v1 = Some(parse_id3v1(id3v1_tag))
-		}
-	} else {
-		// No ID3v1 tag found
-		data.seek(SeekFrom::End(0))?;
-	}
-
-	Ok((exists, id3v1))
-}
-
-pub(in crate::logic) fn parse_id3v1(reader: [u8; 128]) -> Tag {
-	let mut tag = Tag::new(TagType::Id3v1);
-
-	let reader = &reader[3..];
-
-	if let Some(title) = decode_text(ItemKey::TrackTitle, &reader[..30]) {
-		tag.insert_item_unchecked(title);
-	}
-
-	if let Some(artist) = decode_text(ItemKey::TrackArtist, &reader[30..60]) {
-		tag.insert_item_unchecked(artist);
-	}
-
-	if let Some(album) = decode_text(ItemKey::AlbumTitle, &reader[60..90]) {
-		tag.insert_item_unchecked(album);
-	}
-
-	if let Some(year) = decode_text(ItemKey::Year, &reader[90..94]) {
-		tag.insert_item_unchecked(year);
-	}
-
-	let range = if reader[119] == 0 && reader[122] != 0 {
-		tag.insert_item_unchecked(TagItem::new(
-			ItemKey::TrackNumber,
-			ItemValue::UInt(u32::from(reader[122])),
-		));
-
-		94_usize..123
-	} else {
-		94..124
-	};
-
-	if let Some(comment) = decode_text(ItemKey::Comment, &reader[range]) {
-		tag.insert_item_unchecked(comment);
-	}
-
-	if reader[124] < GENRES.len() as u8 {
-		tag.insert_item_unchecked(TagItem::new(
-			ItemKey::Genre,
-			ItemValue::Text(GENRES[reader[125] as usize].to_string()),
-		));
-	}
-
-	tag
-}
-
-fn decode_text(key: ItemKey, data: &[u8]) -> Option<TagItem> {
-	let read = data
-		.iter()
-		.filter(|c| **c != 0)
-		.map(|c| *c as char)
-		.collect::<String>();
-
-	if read.is_empty() {
-		None
-	} else {
-		Some(TagItem::new(key, ItemValue::Text(read)))
-	}
-}
-
-pub(in crate::logic) fn write_id3v1<W>(writer: &mut W, tag: &Tag) -> Result<()>
 where
 	W: Write + Read + Seek,
 {
 	let tag = encode(tag)?;
 
 	// This will seek us to the writing position
-	find_id3v1(writer, false)?;
+	super::find_id3v1(writer, false)?;
 
 	writer.write_all(&tag)?;
 

src/logic/id3/v2/frame/mod.rs

@@ -51,11 +51,11 @@ pub enum Id3v2Frame {
 	UserURL(TextEncoding, String),
 	/// Represents a "SYLT" frame
 	///
-	/// Nothing is required here, the entire frame is stored as [`ItemValue::Binary`](crate::ItemValue::Binary). For parsing see [`SynchronizedText::parse`](crate::id3::SynchronizedText::parse)
+	/// Nothing is required here, the entire frame is stored as [`ItemValue::Binary`](crate::ItemValue::Binary). For parsing see [`SynchronizedText::parse`](crate::id3::v2::SynchronizedText::parse)
 	SyncText,
 	/// Represents a "GEOB" frame
 	///
-	/// Nothing is required here, the entire frame is stored as [`ItemValue::Binary`](crate::ItemValue::Binary). For parsing see [`GeneralEncapsulatedObject::parse`](crate::id3::GeneralEncapsulatedObject::parse)
+	/// Nothing is required here, the entire frame is stored as [`ItemValue::Binary`](crate::ItemValue::Binary). For parsing see [`GeneralEncapsulatedObject::parse`](crate::id3::v2::GeneralEncapsulatedObject::parse)
 	EncapsulatedObject,
 	/// When an ID3v2.2 key couldn't be upgraded
 	///

src/logic/id3/v2/frame/read.rs

@@ -6,7 +6,6 @@ use crate::types::item::TagItemFlags;
 
 use std::io::Read;
 
-use crate::TagItem;
 use byteorder::{BigEndian, ReadBytesExt};
 
 pub(crate) struct Frame {

src/logic/mpeg/read.rs

@@ -115,7 +115,7 @@ where
 				let mut id3v1_read = [0; 128];
 				data.read_exact(&mut id3v1_read)?;
 
-				mpeg_file.id3v1 = Some(crate::logic::id3::v1::parse_id3v1(id3v1_read));
+				mpeg_file.id3v1 = Some(crate::logic::id3::v1::read::parse_id3v1(id3v1_read));
 				continue;
 			},
 			[b'A', b'P', b'E', b'T'] => {

src/types/item.rs

@@ -43,7 +43,7 @@ macro_rules! item_keys {
 			/// Map a format specific key to an ItemKey
 			///
 			/// NOTE: If used with ID3v2, this will only check against the ID3v2.4 keys.
-			/// If you wish to use a V2 or V3 key, see [`upgrade_v2`](crate::id3::upgrade_v2) and [`upgrade_v3`](crate::id3::upgrade_v3)
+			/// If you wish to use a V2 or V3 key, see [`upgrade_v2`](crate::id3::v2::upgrade_v2) and [`upgrade_v3`](crate::id3::v2::upgrade_v3)
 			pub fn from_key(tag_type: &TagType, key: &str) -> Option<Self> {
 				match tag_type {
 					$(
@@ -60,7 +60,7 @@ macro_rules! item_keys {
 
 			/// Maps the variant to a format-specific key
 			///
-			/// NOTE: Since all ID3v2 tags are upgraded to [`Id3v2Version::V4`](crate::id3::Id3v2Version), the
+			/// NOTE: Since all ID3v2 tags are upgraded to [`Id3v2Version::V4`](crate::id3::v2::Id3v2Version), the
 			/// version provided does not matter. They cannot be downgraded.
 			pub fn map_key(&self, tag_type: &TagType) -> Option<&str> {
 				match (tag_type, self) {
@@ -436,8 +436,8 @@ pub enum ItemValue {
 	Locator(String),
 	/// **(APE/ID3v2/MP4 ONLY)** Binary information
 	///
-	/// In the case of ID3v2, this is the type of a [`Id3v2Frame::EncapsulatedObject`](crate::id3::Id3v2Frame::EncapsulatedObject),
+	/// In the case of ID3v2, this is the type of a [`Id3v2Frame::EncapsulatedObject`](crate::id3::v2::Id3v2Frame::EncapsulatedObject),
-	/// [`Id3v2Frame::SyncText`](crate::id3::Id3v2Frame::SyncText), and any unknown frame.
+	/// [`Id3v2Frame::SyncText`](crate::id3::v2::Id3v2Frame::SyncText), and any unknown frame.
 	///
 	/// For APEv2 and MP4, the only use is for unknown items.
 	Binary(Vec<u8>),

src/types/tag.rs

@@ -207,7 +207,7 @@ impl Tag {
 	/// # Warning
 	///
 	/// When dealing with ID3v2, it may be necessary to use [`insert_item_unchecked`](Tag::insert_item_unchecked).
-	/// See [`id3`](crate::id3) for an explanation.
+	/// See [`id3`](crate::id3::v2) for an explanation.
 	pub fn insert_item(&mut self, item: TagItem) -> bool {
 		if item.re_map(&self.tag_type).is_some() {
 			self.insert_item_unchecked(item);