ID3v2: Use Option<T> for optional extra flag data

CHANGELOG.md

@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - **FileProperties**: `FileProperties::new`
 
+### Changed
+- **ID3v2**: Frame/tag flags with optional additional data are now `Option<T>` instead of `(bool, T)`
+
 ## [0.8.1] - 2022-09-09
 
 ### Added

lofty_attr/src/lib.rs

@@ -15,6 +15,7 @@ const LOFTY_FILE_TYPES: [&str; 10] = [
 /// Creates a file usable by Lofty
 ///
 /// See [here](https://github.com/Serial-ATA/lofty-rs/tree/main/examples/custom_resolver) for an example of how to use it.
+// TODO: #[internal]
 #[proc_macro_derive(LoftyFile, attributes(lofty))]
 pub fn lofty_file(input: TokenStream) -> TokenStream {
 	let input = parse_macro_input!(input as DeriveInput);

src/id3/v2/flags.rs

@@ -21,5 +21,5 @@ pub struct ID3v2TagFlags {
 	/// Restrictions on the tag, written in the extended header
 	///
 	/// In addition to being setting this flag, all restrictions must be provided. See [`TagRestrictions`]
-	pub restrictions: (bool, TagRestrictions),
+	pub restrictions: Option<TagRestrictions>,
 }

src/id3/v2/frame/header.rs

@@ -108,29 +108,15 @@ pub(crate) fn parse_flags(flags: u16, v4: bool) -> FrameFlags {
 		} else {
 			flags & 0x2000 == 0x2000
 		},
-		grouping_identity: (
-			if v4 {
-				flags & 0x0040 == 0x0040
-			} else {
-				flags & 0x0020 == 0x0020
-			},
-			0,
-		),
+		grouping_identity: ((v4 && flags & 0x0040 == 0x0040) || (flags & 0x0020 == 0x0020))
+			.then(|| 0),
 		compression: if v4 {
 			flags & 0x0008 == 0x0008
 		} else {
 			flags & 0x0080 == 0x0080
 		},
-		encryption: if v4 {
-			(flags & 0x0004 == 0x0004, 0)
-		} else {
-			(flags & 0x0040 == 0x0040, 0)
-		},
+		encryption: ((v4 && flags & 0x0004 == 0x0004) || flags & 0x0040 == 0x0040).then(|| 0),
 		unsynchronisation: if v4 { flags & 0x0002 == 0x0002 } else { false },
-		data_length_indicator: if v4 {
-			(flags & 0x0001 == 0x0001, 0)
-		} else {
-			(false, 0)
-		},
+		data_length_indicator: (v4 && flags & 0x0001 == 0x0001).then(|| 0),
 	}
 }

src/id3/v2/frame/mod.rs

@@ -224,22 +224,20 @@ pub struct FrameFlags {
 	pub file_alter_preservation: bool,
 	/// Item cannot be written to
 	pub read_only: bool,
-	/// Frame belongs in a group
+	/// The group identifier the frame belongs to
 	///
-	/// In addition to setting this flag, a group identifier byte must be added.
 	/// All frames with the same group identifier byte belong to the same group.
-	pub grouping_identity: (bool, u8),
+	pub grouping_identity: Option<u8>,
 	/// Frame is zlib compressed
 	///
 	/// It is **required** `data_length_indicator` be set if this is set.
 	pub compression: bool,
-	/// Frame is encrypted
+	/// Frame encryption method symbol
 	///
 	/// NOTE: Since the encryption method is unknown, lofty cannot do anything with these frames
 	///
-	/// In addition to setting this flag, an encryption method symbol must be added.
-	/// The method symbol **must** be > 0x80.
-	pub encryption: (bool, u8),
+	/// The encryption method symbol **must** be > 0x80.
+	pub encryption: Option<u8>,
 	/// Frame is unsynchronised
 	///
 	/// In short, this makes all "0xFF X (X >= 0xE0)" combinations into "0xFF 0x00 X" to avoid confusion
@@ -255,7 +253,7 @@ pub struct FrameFlags {
 	/// This is usually used in combination with `compression` and `encryption` (depending on encryption method).
 	///
 	/// If using `encryption`, the final size must be added.
-	pub data_length_indicator: (bool, u32),
+	pub data_length_indicator: Option<u32>,
 }
 
 impl From<TagItem> for Option<Frame> {

src/id3/v2/frame/read.rs

@@ -47,22 +47,22 @@ impl Frame {
 		let mut content_reader = &*content;
 
 		// Get the encryption method symbol
-		if flags.encryption.0 {
-			flags.encryption.1 = content_reader.read_u8()?;
+		if let Some(enc) = flags.encryption.as_mut() {
+			*enc = content_reader.read_u8()?;
 		}
 
 		// Get the group identifier
-		if flags.grouping_identity.0 {
-			flags.grouping_identity.1 = content_reader.read_u8()?;
+		if let Some(group) = flags.grouping_identity.as_mut() {
+			*group = content_reader.read_u8()?;
 		}
 
 		// Get the real data length
-		if flags.data_length_indicator.0 {
-			flags.data_length_indicator.1 = content_reader.read_u32::<BigEndian>()?;
+		if let Some(len) = flags.data_length_indicator.as_mut() {
+			*len = content_reader.read_u32::<BigEndian>()?;
 		}
 
-		let value = if flags.encryption.0 {
-			if !flags.data_length_indicator.0 {
+		let value = if flags.encryption.is_some() {
+			if flags.data_length_indicator.is_none() {
 				return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
 					"Encountered an encrypted frame without a data length indicator",
 				))

src/id3/v2/mod.rs

@@ -118,7 +118,7 @@ where
 			&& flags & 0x10 == 0x10,
 		crc: false, // Retrieved later if applicable
 		#[cfg(feature = "id3v2_restrictions")]
-		restrictions: (false, TagRestrictions::default()), // Retrieved later if applicable
+		restrictions: None, // Retrieved later if applicable
 	};
 
 	let size = unsynch_u32(BigEndian::read_u32(&header[6..]));
@@ -154,12 +154,10 @@ where
 
 		#[cfg(feature = "id3v2_restrictions")]
 		if extended_flags & 0x10 == 0x10 {
-			flags_parsed.restrictions.0 = true;
-
 			// We don't care about the length byte, it is always 1
 			let _data_length = bytes.read_u8()?;
 
-			flags_parsed.restrictions.1 = TagRestrictions::from_byte(bytes.read_u8()?);
+			flags_parsed.restrictions = Some(TagRestrictions::from_byte(bytes.read_u8()?));
 		}
 	}
 

src/id3/v2/write/frame.rs

@@ -56,13 +56,13 @@ fn write_frame<W>(writer: &mut W, name: &str, flags: FrameFlags, value: &[u8]) -
 where
 	W: Write,
 {
-	if flags.encryption.0 {
+	if flags.encryption.is_some() {
 		write_encrypted(writer, name, value, flags)?;
 		return Ok(());
 	}
 
 	let len = value.len() as u32;
-	let is_grouping_identity = flags.grouping_identity.0;
+	let is_grouping_identity = flags.grouping_identity.is_some();
 
 	write_frame_header(
 		writer,
@@ -72,7 +72,8 @@ where
 	)?;
 
 	if is_grouping_identity {
-		writer.write_u8(flags.grouping_identity.1)?;
+		// Guaranteed to be `Some` at this point.
+		writer.write_u8(flags.grouping_identity.unwrap())?;
 	}
 
 	writer.write_all(value)?;
@@ -84,8 +85,8 @@ fn write_encrypted<W>(writer: &mut W, name: &str, value: &[u8], flags: FrameFlag
 where
 	W: Write,
 {
-	let method_symbol = flags.encryption.1;
-	let data_length_indicator = flags.data_length_indicator;
+	// Guaranteed to be `Some` at this point.
+	let method_symbol = flags.encryption.unwrap();
 
 	if method_symbol > 0x80 {
 		return Err(ID3v2Error::new(ID3v2ErrorKind::Other(
@@ -94,13 +95,15 @@ where
 		.into());
 	}
 
-	if data_length_indicator.0 && data_length_indicator.1 > 0 {
-		write_frame_header(writer, name, (value.len() + 1) as u32, flags)?;
-		writer.write_u32::<BigEndian>(synch_u32(data_length_indicator.1)?)?;
-		writer.write_u8(method_symbol)?;
-		writer.write_all(value)?;
+	if let Some(len) = flags.data_length_indicator {
+		if len > 0 {
+			write_frame_header(writer, name, (value.len() + 1) as u32, flags)?;
+			writer.write_u32::<BigEndian>(synch_u32(len)?)?;
+			writer.write_u8(method_symbol)?;
+			writer.write_all(value)?;
 
-		return Ok(());
+			return Ok(());
+		}
 	}
 
 	Err(ID3v2Error::new(ID3v2ErrorKind::Other(
@@ -139,7 +142,7 @@ fn get_flags(tag_flags: FrameFlags) -> u16 {
 		flags |= 0x1000
 	}
 
-	if tag_flags.grouping_identity.0 {
+	if tag_flags.grouping_identity.is_some() {
 		flags |= 0x0040
 	}
 
@@ -147,7 +150,7 @@ fn get_flags(tag_flags: FrameFlags) -> u16 {
 		flags |= 0x0008
 	}
 
-	if tag_flags.encryption.0 {
+	if tag_flags.encryption.is_some() {
 		flags |= 0x0004
 	}
 
@@ -155,7 +158,7 @@ fn get_flags(tag_flags: FrameFlags) -> u16 {
 		flags |= 0x0002
 	}
 
-	if tag_flags.data_length_indicator.0 {
+	if tag_flags.data_length_indicator.is_some() {
 		flags |= 0x0001
 	}
 

src/id3/v2/write/mod.rs

@@ -86,7 +86,7 @@ pub(super) fn create_tag<'a, I: Iterator<Item = FrameRef<'a>> + 'a>(
 	let has_footer = tag.flags.footer;
 	let needs_crc = tag.flags.crc;
 	#[cfg(feature = "id3v2_restrictions")]
-	let has_restrictions = tag.flags.restrictions.0;
+	let has_restrictions = tag.flags.restrictions.is_some();
 
 	let (mut id3v2, extended_header_len) = create_tag_header(tag.flags)?;
 	let header_len = id3v2.get_ref().len();
@@ -153,7 +153,7 @@ fn create_tag_header(flags: ID3v2TagFlags) -> Result<(Cursor<Vec<u8>>, u32)> {
 	let extended_header = flags.crc;
 
 	#[cfg(feature = "id3v2_restrictions")]
-	let extended_header = flags.crc || flags.restrictions.0;
+	let extended_header = flags.crc || flags.restrictions.is_some();
 
 	if flags.footer {
 		tag_flags |= 0x10
@@ -193,12 +193,12 @@ fn create_tag_header(flags: ID3v2TagFlags) -> Result<(Cursor<Vec<u8>>, u32)> {
 		}
 
 		#[cfg(feature = "id3v2_restrictions")]
-		if flags.restrictions.0 {
+		if let Some(restrictions) = flags.restrictions {
 			ext_flags |= 0x10;
 			extended_header_size += 2;
 
 			header.write_u8(1)?;
-			header.write_u8(flags.restrictions.1.as_bytes())?;
+			header.write_u8(restrictions.as_bytes())?;
 		}
 
 		header.seek(SeekFrom::Start(10))?;