Various fixes

CHANGELOG.md

@@ -15,7 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **IFF**: Undersized ID3v2 chunks will no longer error outside of strict mode ([PR](https://github.com/Serial-ATA/lofty-rs/pull/644))
 - **Timestamp**: Support dot-separated dates (e.g. `2024.06.03`) ([issue](https://github.com/Serial-ATA/lofty-rs/issues/647)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/648))
-- **ID3v2**: Fixed UTF-16 description string termination in `APIC` and `SYLT` frames ([issue](https://github.com/Serial-ATA/lofty-rs/issues/653)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/654))
+- **ID3v2**:
+  - Fixed UTF-16 description string termination in `APIC` and `SYLT` frames ([issue](https://github.com/Serial-ATA/lofty-rs/issues/653)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/654))
+  - Fixed `Id3v2Tag::remove_disk_total()`, which incorrectly preserved the track number rather than disk number ([issue](https://github.com/Serial-ATA/lofty-rs/issues/656)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/657))
+  - Fixed handling of encryption method symbols when writing ([issue](https://github.com/Serial-ATA/lofty-rs/issues/656)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/657))
+  - Fixed parsing of extended headers in ID3v2.3 ([issue](https://github.com/Serial-ATA/lofty-rs/issues/656)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/657))
 
 ## [0.24.0] - 2026-04-12
 

Cargo.toml

@@ -18,7 +18,7 @@ license = "MIT OR Apache-2.0"
 [workspace.dependencies]
 lofty = { version = "0.24.0", path = "lofty" }
 lofty_attr = { version = "0.12.0", path = "lofty_attr" }
-ogg_pager = { version = "0.7.1", path = "ogg_pager" }
+ogg_pager = { version = "0.7.2", path = "ogg_pager" }
 
 byteorder = "1.5.0"
 

lofty/src/config/global_options.rs

@@ -4,8 +4,8 @@ thread_local! {
 	static GLOBAL_OPTIONS: UnsafeCell<GlobalOptions> = UnsafeCell::new(GlobalOptions::default());
 }
 
-pub(crate) unsafe fn global_options() -> &'static GlobalOptions {
-	GLOBAL_OPTIONS.with(|global_options| unsafe { &*global_options.get() })
+pub(crate) unsafe fn global_options() -> GlobalOptions {
+	GLOBAL_OPTIONS.with(|global_options| unsafe { *global_options.get() })
 }
 
 /// Options that control all interactions with Lofty for the current thread

lofty/src/flac/properties.rs

@@ -99,8 +99,9 @@ where
 	let bits_per_sample = ((info >> 4) & 0b11111) + 1;
 	let channels = ((info >> 9) & 7) + 1;
 
-	// Read the remaining 32 bits of the total samples
-	let total_samples = stream_info.read_u32::<BigEndian>()? | (info << 28);
+	// Read the remaining 32 bits of the total samples (36 bits total)
+	let total_samples =
+		(u64::from(info & 0xF) << 32) | u64::from(stream_info.read_u32::<BigEndian>()?);
 
 	let signature = stream_info.read_u128::<BigEndian>()?;
 
@@ -113,7 +114,7 @@ where
 	};
 
 	if sample_rate > 0 && total_samples > 0 {
-		let length = (u64::from(total_samples) * 1000) / u64::from(sample_rate);
+		let length = (total_samples * 1000) / u64::from(sample_rate);
 		properties.duration = Duration::from_millis(length);
 
 		if length > 0 && file_length > 0 && stream_length > 0 {

lofty/src/id3/v2/header.rs

@@ -1,6 +1,6 @@
 use crate::error::{Id3v2Error, Id3v2ErrorKind, Result};
 use crate::id3::v2::restrictions::TagRestrictions;
-use crate::id3::v2::util::synchsafe::SynchsafeInteger;
+use crate::id3::v2::util::synchsafe::{SynchsafeInteger, UnsynchronizedStream};
 use crate::macros::err;
 
 use std::io::Read;
@@ -141,32 +141,25 @@ impl Id3v2Header {
 			(version == Id3v2Version::V4 || version == Id3v2Version::V3) && flags & 0x40 == 0x40;
 
 		if extended_header {
-			extended_size = bytes.read_u32::<BigEndian>()?.unsynch();
-
-			if extended_size < 6 {
-				return Err(Id3v2Error::new(Id3v2ErrorKind::BadExtendedHeaderSize).into());
-			}
-
-			// Useless byte since there's only 1 byte for flags
-			let _num_flag_bytes = bytes.read_u8()?;
-
-			let extended_flags = bytes.read_u8()?;
-
-			// The only flags we care about here are the CRC and restrictions
-
-			if extended_flags & 0x20 == 0x20 {
-				flags_parsed.crc = true;
-
-				// We don't care about the existing CRC (5) or its length byte (1)
-				let mut crc = [0; 6];
-				bytes.read_exact(&mut crc)?;
-			}
-
-			if extended_flags & 0x10 == 0x10 {
-				// We don't care about the length byte, it is always 1
-				let _data_length = bytes.read_u8()?;
-
-				flags_parsed.restrictions = Some(TagRestrictions::from_byte(bytes.read_u8()?));
+			match version {
+				// In ID3v2.3, the extended header is considered separate from the frame header, and
+				// thus subject to unsynchronization
+				Id3v2Version::V3 => {
+					if flags_parsed.unsynchronisation {
+						let mut reader = UnsynchronizedStream::new(bytes);
+						extended_size = reader.read_u32::<BigEndian>()?;
+						read_extended_header(&mut reader, &mut flags_parsed, extended_size)?;
+					} else {
+						extended_size = bytes.read_u32::<BigEndian>()?;
+						read_extended_header(bytes, &mut flags_parsed, extended_size)?;
+					}
+				},
+				// In ID3v2.4, they seem to be considered one big header (?) so no flags apply to it
+				Id3v2Version::V4 => {
+					extended_size = bytes.read_u32::<BigEndian>()?.unsynch();
+					read_extended_header(bytes, &mut flags_parsed, extended_size)?;
+				},
+				_ => unreachable!(),
 			}
 		}
 
@@ -187,3 +180,37 @@ impl Id3v2Header {
 		self.size + 10 + self.extended_size + if self.flags.footer { 10 } else { 0 }
 	}
 }
+
+fn read_extended_header<R: Read>(
+	reader: &mut R,
+	flags: &mut Id3v2TagFlags,
+	header_size: u32,
+) -> Result<()> {
+	if header_size < 6 {
+		return Err(Id3v2Error::new(Id3v2ErrorKind::BadExtendedHeaderSize).into());
+	}
+
+	// Useless byte since there's only 1 byte for flags
+	let _num_flag_bytes = reader.read_u8()?;
+
+	let extended_flags = reader.read_u8()?;
+
+	// The only flags we care about here are the CRC and restrictions
+
+	if extended_flags & 0x20 == 0x20 {
+		flags.crc = true;
+
+		// We don't care about the existing CRC (5) or its length byte (1)
+		let mut crc = [0; 6];
+		reader.read_exact(&mut crc)?;
+	}
+
+	if extended_flags & 0x10 == 0x10 {
+		// We don't care about the length byte, it is always 1
+		let _data_length = reader.read_u8()?;
+
+		flags.restrictions = Some(TagRestrictions::from_byte(reader.read_u8()?));
+	}
+
+	Ok(())
+}

lofty/src/id3/v2/restrictions.rs

@@ -68,7 +68,7 @@ impl TagRestrictions {
 		let restriction_flags = byte;
 
 		// xx000000
-		match restriction_flags & 0x0C {
+		match restriction_flags & 0xC0 {
 			64 => restrictions.size = TagSizeRestrictions::S_64F_128K,
 			128 => restrictions.size = TagSizeRestrictions::S_32F_40K,
 			192 => restrictions.size = TagSizeRestrictions::S_32F_4K,
@@ -113,7 +113,7 @@ impl TagRestrictions {
 			TagSizeRestrictions::S_128F_1M => {},
 			TagSizeRestrictions::S_64F_128K => byte |= 0x40,
 			TagSizeRestrictions::S_32F_40K => byte |= 0x80,
-			TagSizeRestrictions::S_32F_4K => byte |= 0x0C,
+			TagSizeRestrictions::S_32F_4K => byte |= 0xC0,
 		}
 
 		if self.text_encoding {

lofty/src/id3/v2/tag.rs

@@ -746,11 +746,14 @@ impl Accessor for Id3v2Tag {
 	}
 
 	fn remove_disk_total(&mut self) {
-		let existing_track_number = self.track();
+		let existing_disk_number = self.disk();
 		let _ = self.remove(&DISC_ID);
 
-		if let Some(track) = existing_track_number {
-			self.insert(Frame::text(Cow::Borrowed("TPOS"), track.to_string()));
+		if let Some(disk) = existing_disk_number {
+			self.insert(Frame::text(
+				Cow::Borrowed(DISC_ID.as_str()),
+				disk.to_string(),
+			));
 		}
 	}
 

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

@@ -327,7 +327,7 @@ where
 	// Guaranteed to be `Some` at this point.
 	let method_symbol = flags.encryption.unwrap();
 
-	if method_symbol > 0x80 {
+	if method_symbol <= 0x80 {
 		return Err(
 			Id3v2Error::new(Id3v2ErrorKind::InvalidEncryptionMethodSymbol(method_symbol)).into(),
 		);

lofty/src/mp4/ilst/read.rs

@@ -347,7 +347,10 @@ fn parse_int(bytes: &[u8]) -> Result<i32> {
 	Ok(match bytes.len() {
 		1 => i32::from(bytes[0]),
 		2 => i32::from(i16::from_be_bytes([bytes[0], bytes[1]])),
-		3 => i32::from_be_bytes([0, bytes[0], bytes[1], bytes[2]]),
+		3 => {
+			let pad = if bytes[0] & 0x80 != 0 { 0xFF } else { 0x00 };
+			i32::from_be_bytes([pad, bytes[0], bytes[1], bytes[2]])
+		},
 		4 => i32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]),
 		_ => err!(BadAtom(
 			"Unexpected atom size for type \"BE signed integer\""

lofty/src/mp4/ilst/write.rs

@@ -458,8 +458,7 @@ fn update_offsets(
 			write_handle.write_u32::<BigEndian>((i64::from(read_offset) + difference) as u32)?;
 
 			log::trace!(
-				"Updated offset from {} to {}",
-				read_offset,
+				"Updated offset from {read_offset} to {}",
 				(i64::from(read_offset) + difference) as u32
 			);
 		}
@@ -469,12 +468,7 @@ fn update_offsets(
 	for co64 in moov.find_all_children(*b"co64", true) {
 		log::trace!("Found `co64` atom");
 
-		let co64_start = co64.start;
-		if !co64.extended {
-			decode_err!(@BAIL Mp4, "Expected `co64` atom to be extended");
-		}
-
-		write_handle.seek(SeekFrom::Start(co64_start + ATOM_HEADER_LEN + 8 + 4))?;
+		write_handle.seek(SeekFrom::Start(co64.start + ATOM_HEADER_LEN + 8 + 4))?;
 
 		let count = write_handle.read_u32::<BigEndian>()?;
 		for _ in 0..count {
@@ -487,8 +481,7 @@ fn update_offsets(
 			write_handle.write_u64::<BigEndian>((read_offset as i64 + difference) as u64)?;
 
 			log::trace!(
-				"Updated offset from {} to {}",
-				read_offset,
+				"Updated offset from {read_offset} to {}",
 				((read_offset as i64) + difference) as u64
 			);
 		}
@@ -525,8 +518,7 @@ fn update_offsets(
 			write_handle.write_u64::<BigEndian>((read_offset as i64 + difference) as u64)?;
 
 			log::trace!(
-				"Updated offset from {} to {}",
-				read_offset,
+				"Updated offset from {read_offset} to {}",
 				((read_offset as i64) + difference) as u64
 			);
 		}
@@ -643,7 +635,7 @@ where
 
 	drop(write_handle);
 
-	log::trace!("Built `ilst` atom, size: {} bytes", size);
+	log::trace!("Built `ilst` atom, size: {size} bytes");
 
 	Ok(ilst_writer.into_contents())
 }