ID3v2: Flatten SynchronizedText and GeneralEncapsulatedObject

CHANGELOG.md

@@ -34,7 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **ID3v2**: Compressed frames are now properly handled ([PR](https://github.com/Serial-ATA/lofty-rs/pull/191))
 
 ### Removed
-- **ID3v2**: All uses of `ID3v2ErrorKind::Other` have been replaced with concrete errors
+- **ID3v2**:
+  - All uses of `ID3v2ErrorKind::Other` have been replaced with concrete errors
+  - `SyncTextInformation` and `GEOBInformation` have been flattened into their respective items ([PR](https://github.com/Serial-ATA/lofty-rs/pull/196))
 
 ## [0.12.1] - 2023-04-10
 

src/id3/v2/items/encapsulated_object.rs

@@ -3,9 +3,9 @@ use crate::util::text::{decode_text, encode_text, TextEncoding};
 
 use std::io::{Cursor, Read};
 
-/// Information about a [`GeneralEncapsulatedObject`]
+/// Allows for encapsulation of any file type inside an ID3v2 tag
 #[derive(PartialEq, Clone, Debug, Eq, Hash)]
-pub struct GEOBInformation {
+pub struct GeneralEncapsulatedObject {
 	/// The text encoding of `file_name` and `description`
 	pub encoding: TextEncoding,
 	/// The file's mimetype
@@ -14,13 +14,6 @@ pub struct GEOBInformation {
 	pub file_name: Option<String>,
 	/// A unique content descriptor
 	pub descriptor: Option<String>,
-}
-
-/// Allows for encapsulation of any file type inside an ID3v2 tag
-#[derive(PartialEq, Clone, Debug, Eq, Hash)]
-pub struct GeneralEncapsulatedObject {
-	/// Information about the data
-	pub information: GEOBInformation,
 	/// The file's content
 	pub data: Vec<u8>,
 }
@@ -51,12 +44,10 @@ impl GeneralEncapsulatedObject {
 		cursor.read_to_end(&mut data)?;
 
 		Ok(Self {
-			information: GEOBInformation {
-				encoding,
-				mime_type: mime_type.text_or_none(),
-				file_name: file_name.text_or_none(),
-				descriptor: descriptor.text_or_none(),
-			},
+			encoding,
+			mime_type: mime_type.text_or_none(),
+			file_name: file_name.text_or_none(),
+			descriptor: descriptor.text_or_none(),
 			data,
 		})
 	}
@@ -65,20 +56,20 @@ impl GeneralEncapsulatedObject {
 	///
 	/// NOTE: This does not include a frame header
 	pub fn as_bytes(&self) -> Vec<u8> {
-		let encoding = self.information.encoding;
+		let encoding = self.encoding;
 
 		let mut bytes = vec![encoding as u8];
 
-		if let Some(ref mime_type) = self.information.mime_type {
+		if let Some(ref mime_type) = self.mime_type {
 			bytes.extend(mime_type.as_bytes())
 		}
 
 		bytes.push(0);
 
-		let file_name = self.information.file_name.as_deref();
+		let file_name = self.file_name.as_deref();
 		bytes.extend(&*encode_text(file_name.unwrap_or(""), encoding, true));
 
-		let descriptor = self.information.descriptor.as_deref();
+		let descriptor = self.descriptor.as_deref();
 		bytes.extend(&*encode_text(descriptor.unwrap_or(""), encoding, true));
 
 		bytes.extend(&self.data);
@@ -89,18 +80,16 @@ impl GeneralEncapsulatedObject {
 
 #[cfg(test)]
 mod tests {
-	use crate::id3::v2::{GEOBInformation, GeneralEncapsulatedObject};
+	use crate::id3::v2::GeneralEncapsulatedObject;
 	use crate::util::text::TextEncoding;
 
 	#[test]
 	fn geob_decode() {
 		let expected = GeneralEncapsulatedObject {
-			information: GEOBInformation {
-				encoding: TextEncoding::Latin1,
-				mime_type: Some(String::from("audio/mpeg")),
-				file_name: Some(String::from("a.mp3")),
-				descriptor: Some(String::from("Test Asset")),
-			},
+			encoding: TextEncoding::Latin1,
+			mime_type: Some(String::from("audio/mpeg")),
+			file_name: Some(String::from("a.mp3")),
+			descriptor: Some(String::from("Test Asset")),
 			data: crate::tag::utils::test_utils::read_path(
 				"tests/files/assets/minimal/full_test.mp3",
 			),
@@ -116,12 +105,10 @@ mod tests {
 	#[test]
 	fn geob_encode() {
 		let to_encode = GeneralEncapsulatedObject {
-			information: GEOBInformation {
-				encoding: TextEncoding::Latin1,
-				mime_type: Some(String::from("audio/mpeg")),
-				file_name: Some(String::from("a.mp3")),
-				descriptor: Some(String::from("Test Asset")),
-			},
+			encoding: TextEncoding::Latin1,
+			mime_type: Some(String::from("audio/mpeg")),
+			file_name: Some(String::from("a.mp3")),
+			descriptor: Some(String::from("Test Asset")),
 			data: crate::tag::utils::test_utils::read_path(
 				"tests/files/assets/minimal/full_test.mp3",
 			),

src/id3/v2/items/mod.rs

@@ -12,12 +12,12 @@ mod url_link_frame;
 
 pub use attached_picture_frame::AttachedPictureFrame;
 pub use audio_text_frame::{scramble, AudioTextFrame, AudioTextFrameFlags};
-pub use encapsulated_object::{GEOBInformation, GeneralEncapsulatedObject};
+pub use encapsulated_object::GeneralEncapsulatedObject;
 pub use extended_text_frame::ExtendedTextFrame;
 pub use extended_url_frame::ExtendedUrlFrame;
 pub use identifier::UniqueFileIdentifierFrame;
 pub use language_frame::{CommentFrame, UnsynchronizedTextFrame};
 pub use popularimeter::Popularimeter;
-pub use sync_text::{SyncTextContentType, SyncTextInformation, SynchronizedText, TimestampFormat};
+pub use sync_text::{SyncTextContentType, SynchronizedText, TimestampFormat};
 pub use text_information_frame::TextInformationFrame;
 pub use url_link_frame::UrlLinkFrame;

src/id3/v2/items/sync_text.rs

@@ -61,9 +61,9 @@ impl SyncTextContentType {
 	}
 }
 
-/// Information about a [`SynchronizedText`]
+/// Represents an ID3v2 synchronized text frame
 #[derive(PartialEq, Clone, Debug, Eq, Hash)]
-pub struct SyncTextInformation {
+pub struct SynchronizedText {
 	/// The text encoding (description/text)
 	pub encoding: TextEncoding,
 	/// ISO-639-2 language code (3 bytes)
@@ -74,13 +74,6 @@ pub struct SyncTextInformation {
 	pub content_type: SyncTextContentType,
 	/// Unique content description
 	pub description: Option<String>,
-}
-
-/// Represents an ID3v2 synchronized text frame
-#[derive(PartialEq, Clone, Debug, Eq, Hash)]
-pub struct SynchronizedText {
-	/// Information about the synchronized text
-	pub information: SyncTextInformation,
 	/// Collection of timestamps and text
 	pub content: Vec<(u32, String)>,
 }
@@ -175,13 +168,11 @@ impl SynchronizedText {
 		}
 
 		Ok(Self {
-			information: SyncTextInformation {
-				encoding,
-				language,
-				timestamp_format,
-				content_type,
-				description,
-			},
+			encoding,
+			language,
+			timestamp_format,
+			content_type,
+			description,
 			content,
 		})
 	}
@@ -196,25 +187,21 @@ impl SynchronizedText {
 	/// * `language` is not exactly 3 bytes
 	/// * `language` contains invalid characters (Only `'a'..='z'` and `'A'..='Z'` allowed)
 	pub fn as_bytes(&self) -> Result<Vec<u8>> {
-		let information = &self.information;
-
-		let mut data = vec![information.encoding as u8];
+		let mut data = vec![self.encoding as u8];
 
-		if information.language.len() == 3
-			&& information.language.iter().all(u8::is_ascii_alphabetic)
-		{
-			data.write_all(&information.language)?;
-			data.write_u8(information.timestamp_format as u8)?;
-			data.write_u8(information.content_type as u8)?;
+		if self.language.len() == 3 && self.language.iter().all(u8::is_ascii_alphabetic) {
+			data.write_all(&self.language)?;
+			data.write_u8(self.timestamp_format as u8)?;
+			data.write_u8(self.content_type as u8)?;
 
-			if let Some(description) = &information.description {
-				data.write_all(&encode_text(description, information.encoding, true))?;
+			if let Some(description) = &self.description {
+				data.write_all(&encode_text(description, self.encoding, true))?;
 			} else {
 				data.write_u8(0)?;
 			}
 
 			for (time, ref text) in &self.content {
-				data.write_all(&encode_text(text, information.encoding, true))?;
+				data.write_all(&encode_text(text, self.encoding, true))?;
 				data.write_u32::<BigEndian>(*time)?;
 			}
 
@@ -231,20 +218,16 @@ impl SynchronizedText {
 
 #[cfg(test)]
 mod tests {
-	use crate::id3::v2::{
-		SyncTextContentType, SyncTextInformation, SynchronizedText, TimestampFormat,
-	};
+	use crate::id3::v2::{SyncTextContentType, SynchronizedText, TimestampFormat};
 	use crate::util::text::TextEncoding;
 
 	fn expected(encoding: TextEncoding) -> SynchronizedText {
 		SynchronizedText {
-			information: SyncTextInformation {
-				encoding,
-				language: *b"eng",
-				timestamp_format: TimestampFormat::MS,
-				content_type: SyncTextContentType::Lyrics,
-				description: Some(String::from("Test Sync Text")),
-			},
+			encoding,
+			language: *b"eng",
+			timestamp_format: TimestampFormat::MS,
+			content_type: SyncTextContentType::Lyrics,
+			description: Some(String::from("Test Sync Text")),
 			content: vec![
 				(0, String::from("\nLofty")),
 				(10000, String::from("\nIs")),