ID3v2: Turn Frame into an enum

CHANGELOG.md

@@ -12,11 +12,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     - This will allow for generic edits to the iTunes-style parental advisory tag. Note that this will use the
       numeric representation. For more information, see: https://docs.mp3tag.de/mapping/#itunesadvisory.
   - New `tag::items` module for generic representations of complex tag items
-  - New **Timestamp** item for ISO 8601 timestamps ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
-- **ID3v2**: Special handling for frames with timestamps with `FrameValue::Timestamp` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
+  - New `Timestamp` item for ISO 8601 timestamps ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
+- **ID3v2**: Special handling for frames with timestamps with `Frame::Timestamp` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/389))
 
 ### Changed
 - **VorbisComments**/**ApeTag**: Verify contents of `ItemKey::FlagCompilation` during `Tag` merge ([PR](https://github.com/Serial-ATA/lofty-rs/pull/387))
+- **ID3v2**:
+  - ⚠️ Important ⚠️: `Frame` has been converted to an `enum` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/390)):
+    - This makes it easier to validate frame contents, as one can no longer make an `AttachedPictureFrame` with the ID `"TALB"`, for example.
+      See the PR for a full description of the changes.
+    ```rust
+    // Old:
+    let frame = Frame::new(
+        "TALB",
+        FrameType::Text(TextInformationFrame {
+            TextEncoding::UTF8,
+            value: String::from("Foo album"),
+        }),
+        FrameFlags::default(),
+    ).unwrap();
+
+    // New:
+    let frame = Frame::Text(TextInformationFrame::new(
+        FrameId::new("TALB").unwrap(),
+        FrameFlags::default(),
+        TextEncoding::UTF8,
+        String::from("Foo album"),
+    ));
+    ```
+  - Renamed `Popularimeter` -> `PopularimeterFrame`
+  - Renamed `SynchronizedText` -> `SynchronizedTextFrame`
 
 ## [0.19.2] - 2024-04-26
 

lofty/src/id3/v2/frame/content.rs

@@ -1,13 +1,13 @@
 use crate::config::ParsingMode;
 use crate::error::{Id3v2Error, Id3v2ErrorKind, Result};
-use crate::id3::v2::frame::FrameValue;
 use crate::id3::v2::header::Id3v2Version;
 use crate::id3::v2::items::{
 	AttachedPictureFrame, CommentFrame, EventTimingCodesFrame, ExtendedTextFrame, ExtendedUrlFrame,
-	KeyValueFrame, OwnershipFrame, Popularimeter, PrivateFrame, RelativeVolumeAdjustmentFrame,
+	KeyValueFrame, OwnershipFrame, PopularimeterFrame, PrivateFrame, RelativeVolumeAdjustmentFrame,
 	TextInformationFrame, TimestampFrame, UniqueFileIdentifierFrame, UnsynchronizedTextFrame,
 	UrlLinkFrame,
 };
+use crate::id3::v2::{BinaryFrame, Frame, FrameFlags, FrameId};
 use crate::macros::err;
 use crate::util::text::TextEncoding;
 
@@ -16,39 +16,36 @@ use std::io::Read;
 #[rustfmt::skip]
 pub(super) fn parse_content<R: Read>(
     reader: &mut R,
-    id: &str,
+    id: FrameId<'static>,
+	flags: FrameFlags,
     version: Id3v2Version,
 	parse_mode: ParsingMode,
-) -> Result<Option<FrameValue>> {
-	Ok(match id {
+) -> Result<Option<Frame<'static>>> {
+	Ok(match id.as_str() {
 		// The ID was previously upgraded, but the content remains unchanged, so version is necessary
 		"APIC" => {
-			let attached_picture = AttachedPictureFrame::parse(reader, version)?;
-			Some(FrameValue::Picture(attached_picture))
+			Some(Frame::Picture(AttachedPictureFrame::parse(reader, flags, version)?))
 		},
-		"TXXX" => ExtendedTextFrame::parse(reader, version)?.map(FrameValue::UserText),
-		"WXXX" => ExtendedUrlFrame::parse(reader, version)?.map(FrameValue::UserUrl),
-		"COMM" => CommentFrame::parse(reader, version)?.map(FrameValue::Comment),
-		"USLT" => UnsynchronizedTextFrame::parse(reader, version)?.map(FrameValue::UnsynchronizedText),
-		"TIPL" | "TMCL" => KeyValueFrame::parse(reader, version)?.map(FrameValue::KeyValue),
-		"UFID" => UniqueFileIdentifierFrame::parse(reader, parse_mode)?.map(FrameValue::UniqueFileIdentifier),
-		"RVA2" => RelativeVolumeAdjustmentFrame::parse(reader, parse_mode)?.map(FrameValue::RelativeVolumeAdjustment),
-		"OWNE" => OwnershipFrame::parse(reader)?.map(FrameValue::Ownership),
-		"ETCO" => EventTimingCodesFrame::parse(reader)?.map(FrameValue::EventTimingCodes),
-		"PRIV" => PrivateFrame::parse(reader)?.map(FrameValue::Private),
-		_ if id.starts_with('T') => TextInformationFrame::parse(reader, version)?.map(FrameValue::Text),
+		"TXXX" => ExtendedTextFrame::parse(reader, flags, version)?.map(Frame::UserText),
+		"WXXX" => ExtendedUrlFrame::parse(reader, flags, version)?.map(Frame::UserUrl),
+		"COMM" => CommentFrame::parse(reader, flags, version)?.map(Frame::Comment),
+		"USLT" => UnsynchronizedTextFrame::parse(reader, flags, version)?.map(Frame::UnsynchronizedText),
+		"TIPL" | "TMCL" => KeyValueFrame::parse(reader, id, flags, version)?.map(Frame::KeyValue),
+		"UFID" => UniqueFileIdentifierFrame::parse(reader, flags, parse_mode)?.map(Frame::UniqueFileIdentifier),
+		"RVA2" => RelativeVolumeAdjustmentFrame::parse(reader, flags, parse_mode)?.map(Frame::RelativeVolumeAdjustment),
+		"OWNE" => OwnershipFrame::parse(reader, flags)?.map(Frame::Ownership),
+		"ETCO" => EventTimingCodesFrame::parse(reader, flags)?.map(Frame::EventTimingCodes),
+		"PRIV" => PrivateFrame::parse(reader, flags)?.map(Frame::Private),
+		i if i.starts_with('T') => TextInformationFrame::parse(reader, id, flags, version)?.map(Frame::Text),
 		// Apple proprietary frames
 		// WFED (Podcast URL), GRP1 (Grouping), MVNM (Movement Name), MVIN (Movement Number)
-		"WFED" | "GRP1" | "MVNM" | "MVIN" => TextInformationFrame::parse(reader, version)?.map(FrameValue::Text),
-		_ if id.starts_with('W') => UrlLinkFrame::parse(reader)?.map(FrameValue::Url),
-		"POPM" => Some(FrameValue::Popularimeter(Popularimeter::parse(reader)?)),
-		"TDEN" | "TDOR" | "TDRC" | "TDRL" | "TDTG" => TimestampFrame::parse(reader, parse_mode)?.map(FrameValue::Timestamp),
+		"WFED" | "GRP1" | "MVNM" | "MVIN" => TextInformationFrame::parse(reader, id, flags, version)?.map(Frame::Text),
+		i if i.starts_with('W') => UrlLinkFrame::parse(reader, id, flags)?.map(Frame::Url),
+		"POPM" => Some(Frame::Popularimeter(PopularimeterFrame::parse(reader, flags)?)),
+		"TDEN" | "TDOR" | "TDRC" | "TDRL" | "TDTG" => TimestampFrame::parse(reader, id, flags, parse_mode)?.map(Frame::Timestamp),
 		// SYLT, GEOB, and any unknown frames
 		_ => {
-			let mut content = Vec::new();
-			reader.read_to_end(&mut content)?;
-
-			Some(FrameValue::Binary(content))
+			Some(Frame::Binary(BinaryFrame::parse(reader, id, flags)?))
 		},
 	})
 }

lofty/src/id3/v2/frame/conversion.rs

@@ -0,0 +1,177 @@
+use crate::error::{Id3v2Error, Id3v2ErrorKind, LoftyError, Result};
+use crate::id3::v2::frame::{
+	FrameRef, EMPTY_CONTENT_DESCRIPTOR, MUSICBRAINZ_UFID_OWNER, UNKNOWN_LANGUAGE,
+};
+use crate::id3::v2::tag::{
+	new_binary_frame, new_comment_frame, new_text_frame, new_unsync_text_frame, new_url_frame,
+	new_user_text_frame, new_user_url_frame,
+};
+use crate::id3::v2::{
+	ExtendedTextFrame, ExtendedUrlFrame, Frame, FrameFlags, FrameId, PopularimeterFrame,
+	UniqueFileIdentifierFrame, UnsynchronizedTextFrame,
+};
+use crate::macros::err;
+use crate::tag::{ItemKey, ItemValue, TagItem, TagType};
+use crate::TextEncoding;
+
+use std::borrow::Cow;
+
+fn frame_from_unknown_item(id: FrameId<'_>, item_value: ItemValue) -> Result<Frame<'_>> {
+	match item_value {
+		ItemValue::Text(text) => Ok(new_text_frame(id, text)),
+		ItemValue::Locator(locator) => {
+			if TextEncoding::verify_latin1(&locator) {
+				Ok(new_url_frame(id, locator))
+			} else {
+				err!(TextDecode("ID3v2 URL frames must be Latin-1"));
+			}
+		},
+		ItemValue::Binary(binary) => Ok(new_binary_frame(id, binary.clone())),
+	}
+}
+
+impl From<TagItem> for Option<Frame<'static>> {
+	fn from(input: TagItem) -> Self {
+		let value;
+		match input.key().try_into().map(FrameId::into_owned) {
+			Ok(id) => {
+				match (&id, input.item_value) {
+					(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "COMM" => {
+						value = new_comment_frame(text);
+					},
+					(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "USLT" => {
+						value = Frame::UnsynchronizedText(UnsynchronizedTextFrame::new(
+							TextEncoding::UTF8,
+							UNKNOWN_LANGUAGE,
+							EMPTY_CONTENT_DESCRIPTOR,
+							text,
+						));
+					},
+					(FrameId::Valid(ref s), ItemValue::Locator(text) | ItemValue::Text(text))
+						if s == "WXXX" =>
+					{
+						value = Frame::UserUrl(ExtendedUrlFrame::new(
+							TextEncoding::UTF8,
+							EMPTY_CONTENT_DESCRIPTOR,
+							text,
+						));
+					},
+					(FrameId::Valid(ref s), ItemValue::Text(text)) if s == "TXXX" => {
+						value = new_user_text_frame(EMPTY_CONTENT_DESCRIPTOR, text);
+					},
+					(FrameId::Valid(ref s), ItemValue::Binary(text)) if s == "POPM" => {
+						value = Frame::Popularimeter(
+							PopularimeterFrame::parse(&mut &text[..], FrameFlags::default())
+								.ok()?,
+						);
+					},
+					(_, item_value) => value = frame_from_unknown_item(id, item_value).ok()?,
+				};
+			},
+			Err(_) => match input.item_key.map_key(TagType::Id3v2, true) {
+				Some(desc) => match input.item_value {
+					ItemValue::Text(text) => {
+						value = Frame::UserText(ExtendedTextFrame::new(
+							TextEncoding::UTF8,
+							String::from(desc),
+							text,
+						))
+					},
+					ItemValue::Locator(locator) => {
+						value = Frame::UserUrl(ExtendedUrlFrame::new(
+							TextEncoding::UTF8,
+							String::from(desc),
+							locator,
+						))
+					},
+					ItemValue::Binary(_) => return None,
+				},
+				None => match (input.item_key, input.item_value) {
+					(ItemKey::MusicBrainzRecordingId, ItemValue::Text(recording_id)) => {
+						if !recording_id.is_ascii() {
+							return None;
+						}
+						let frame = UniqueFileIdentifierFrame::new(
+							MUSICBRAINZ_UFID_OWNER.to_owned(),
+							recording_id.into_bytes(),
+						);
+						value = Frame::UniqueFileIdentifier(frame);
+					},
+					_ => {
+						return None;
+					},
+				},
+			},
+		}
+
+		Some(value)
+	}
+}
+
+impl<'a> TryFrom<&'a TagItem> for FrameRef<'a> {
+	type Error = LoftyError;
+
+	fn try_from(tag_item: &'a TagItem) -> std::result::Result<Self, Self::Error> {
+		let id: crate::error::Result<FrameId<'a>> = tag_item.key().try_into();
+		let value: Frame<'_>;
+		match id {
+			Ok(id) => {
+				let id_str = id.as_str();
+
+				match (id_str, tag_item.value()) {
+					("COMM", ItemValue::Text(text)) => {
+						value = new_comment_frame(text.clone());
+					},
+					("USLT", ItemValue::Text(text)) => {
+						value = new_unsync_text_frame(text.clone());
+					},
+					("WXXX", ItemValue::Locator(text) | ItemValue::Text(text)) => {
+						value = new_user_url_frame(EMPTY_CONTENT_DESCRIPTOR, text.clone());
+					},
+					(locator_id, ItemValue::Locator(text)) if locator_id.len() > 4 => {
+						value = new_user_url_frame(String::from(locator_id), text.clone());
+					},
+					("TXXX", ItemValue::Text(text)) => {
+						value = new_user_text_frame(EMPTY_CONTENT_DESCRIPTOR, text.clone());
+					},
+					(text_id, ItemValue::Text(text)) if text_id.len() > 4 => {
+						value = new_user_text_frame(String::from(text_id), text.clone());
+					},
+					("POPM", ItemValue::Binary(contents)) => {
+						value = Frame::Popularimeter(PopularimeterFrame::parse(
+							&mut &contents[..],
+							FrameFlags::default(),
+						)?);
+					},
+					(_, item_value) => value = frame_from_unknown_item(id, item_value.clone())?,
+				};
+			},
+			Err(_) => {
+				let item_key = tag_item.key();
+				let Some(desc) = item_key.map_key(TagType::Id3v2, true) else {
+					return Err(Id3v2Error::new(Id3v2ErrorKind::UnsupportedFrameId(
+						item_key.clone(),
+					))
+					.into());
+				};
+
+				match tag_item.value() {
+					ItemValue::Text(text) => {
+						value = new_user_text_frame(String::from(desc), text.clone());
+					},
+					ItemValue::Locator(locator) => {
+						value = new_user_url_frame(String::from(desc), locator.clone());
+					},
+					_ => {
+						return Err(Id3v2Error::new(Id3v2ErrorKind::UnsupportedFrameId(
+							item_key.clone(),
+						))
+						.into())
+					},
+				}
+			},
+		}
+
+		Ok(FrameRef(Cow::Owned(value)))
+	}
+}

lofty/src/id3/v2/frame/id.rs → lofty/src/id3/v2/frame/header/mod.rs

@@ -1,8 +1,38 @@
+pub(super) mod parse;
+
+use crate::error;
+use crate::error::{Id3v2Error, Id3v2ErrorKind, LoftyError};
+use crate::id3::v2::FrameFlags;
+use crate::prelude::ItemKey;
+use crate::tag::TagType;
+
 use std::borrow::Cow;
 use std::fmt::{Display, Formatter};
 
-use crate::error::{Id3v2Error, Id3v2ErrorKind, LoftyError, Result};
-use crate::tag::{ItemKey, TagType};
+/// An ID3v2 frame header
+///
+/// These are rarely constructed by hand. Usually they are created in the background
+/// when making a new [`Frame`](crate::id3::v2::Frame).
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[allow(missing_docs)]
+pub struct FrameHeader<'a> {
+	pub(crate) id: FrameId<'a>,
+	pub flags: FrameFlags,
+}
+
+impl<'a> FrameHeader<'a> {
+	/// Create a new [`FrameHeader`]
+	///
+	/// NOTE: Once the header is created, the ID becomes immutable.
+	pub const fn new(id: FrameId<'a>, flags: FrameFlags) -> Self {
+		Self { id, flags }
+	}
+
+	/// Get the ID of the frame
+	pub const fn id(&self) -> &FrameId<'a> {
+		&self.id
+	}
+}
 
 /// An `ID3v2` frame ID
 ///
@@ -22,21 +52,21 @@ pub enum FrameId<'a> {
 impl<'a> FrameId<'a> {
 	/// Attempts to create a `FrameId` from an ID string
 	///
-	/// NOTE: This will not upgrade IDs, for that behavior use [`Frame::new`](crate::id3::v2::Frame::new).
+	/// NOTE: This will not upgrade IDs.
 	///
 	/// # Errors
 	///
 	/// * `id` contains invalid characters (must be 'A'..='Z' and '0'..='9')
 	/// * `id` is an invalid length (must be 3 or 4)
-	pub fn new<I>(id: I) -> Result<Self>
+	pub fn new<I>(id: I) -> error::Result<Self>
 	where
 		I: Into<Cow<'a, str>>,
 	{
 		Self::new_cow(id.into())
 	}
 
 	// Split from generic, public method to avoid code bloat by monomorphization.
-	pub(super) fn new_cow(id: Cow<'a, str>) -> Result<Self> {
+	pub(in crate::id3::v2::frame) fn new_cow(id: Cow<'a, str>) -> error::Result<Self> {
 		Self::verify_id(&id)?;
 
 		match id.len() {
@@ -55,7 +85,7 @@ impl<'a> FrameId<'a> {
 		}
 	}
 
-	pub(super) fn verify_id(id_str: &str) -> Result<()> {
+	pub(in crate::id3::v2::frame) fn verify_id(id_str: &str) -> error::Result<()> {
 		for c in id_str.chars() {
 			if !c.is_ascii_uppercase() && !c.is_ascii_digit() {
 				return Err(Id3v2Error::new(Id3v2ErrorKind::BadFrameId(