Fix ID3v2.2 reading, cleanup MPEG header reading

Cargo.toml

@@ -13,7 +13,7 @@ categories = ["accessibility", "multimedia::audio"]
 # Id3
 flate2 = { version = "1.0.21", optional = true }
 # Ogg
-ogg_pager = { version = "0.1.7", optional = true }
+ogg_pager = "0.1.7"
 # Mp4
 simdutf8 = { version = "0.1.3", optional = true }
 
@@ -26,7 +26,7 @@ byteorder = "1.4.3"
 [features]
 default = ["mp4_atoms", "vorbis_comments", "ape", "id3v1", "id3v2", "aiff_text_chunks", "riff_info_list", "quick_tag_accessors"]
 mp4_atoms = ["simdutf8"]
-vorbis_comments = ["ogg_pager"]
+vorbis_comments = []
 ape = []
 id3v1 = []
 id3v2 = ["flate2"]

src/lib.rs

@@ -54,6 +54,9 @@
 //! let mpeg_file = MpegFile::read_from(&mut file_content).unwrap();
 //!
 //! assert_eq!(mpeg_file.properties().channels(), Some(2));
+//!
+//! // Here we have a file with multiple tags
+//! assert!(mpeg_file.contains_tag_type(&TagType::Id3v2));
 //! assert!(mpeg_file.contains_tag_type(&TagType::Ape));
 //! ```
 //!

src/logic/id3/v2/util/upgrade.rs

@@ -35,6 +35,7 @@ gen_upgrades!(
 		"IPL" => "TIPL",
 		"MCI" => "MCDI",
 		"MLL" => "MLLT",
+		"PIC" => "APIC",
 		"POP" => "POPM",
 		"REV" => "RVRB",
 		"SLT" => "SYLT",

src/logic/mpeg/constants.rs

@@ -29,13 +29,13 @@ pub const BITRATES: [[[u32; 16]; 3]; 2] = [
 	],
 ];
 
-pub const SAMPLE_RATES: [[u32; 4]; 3] = [
-	[44100, 48000, 32000, 0], // Version 1
-	[22050, 24000, 16000, 0], // Version 2
-	[11025, 12000, 8000, 0],  // Version 2.5
+pub const SAMPLE_RATES: [[u32; 3]; 3] = [
+	[44100, 48000, 32000], // Version 1
+	[22050, 24000, 16000], // Version 2
+	[11025, 12000, 8000],  // Version 2.5
 ];
 
-pub const SAMPLES_PER_FRAME: [[u16; 2]; 3] = [
+pub const SAMPLES: [[u16; 2]; 3] = [
 	// Order:
 	// [Version 1, Version 2/2.5]
 	// Layer 1

src/logic/mpeg/header.rs

@@ -1,14 +1,12 @@
-use super::constants::{
-	BITRATES, PADDING_SIZES, SAMPLES_PER_FRAME, SAMPLE_RATES, SIDE_INFORMATION_SIZES,
-};
-use crate::{LoftyError, Result};
+use super::constants::{BITRATES, PADDING_SIZES, SAMPLES, SAMPLE_RATES, SIDE_INFORMATION_SIZES};
+use crate::error::{LoftyError, Result};
 
 use std::io::Read;
 
 use byteorder::{BigEndian, ReadBytesExt};
 
-pub(crate) fn verify_frame_sync(byte_1: u8, byte_2: u8) -> bool {
-	byte_1 == 0xFF && byte_2 != 0xFF && (byte_2 & 0xE0) == 0xE0
+pub(crate) fn verify_frame_sync(frame_sync: u16) -> bool {
+	(frame_sync & 0xffe0) == 0xffe0
 }
 
 #[derive(PartialEq, Copy, Clone)]
@@ -39,7 +37,7 @@ pub(crate) struct Header {
 	pub channels: u8,
 	pub len: u32,
 	pub data_start: u32,
-	pub samples_per_frame: u16,
+	pub samples: u16,
 	pub bitrate: u32,
 }
 
@@ -52,21 +50,37 @@ impl Header {
 			_ => return Err(LoftyError::Mp3("Frame header has an invalid version")),
 		};
 
-		let layer = match (header >> 17) & 0b11 {
+		let version_index = if version == MpegVersion::V1 { 0 } else { 1 };
+
+		let layer = match (header >> 17) & 3 {
 			1 => Layer::Layer3,
 			2 => Layer::Layer2,
 			3 => Layer::Layer1,
 			_ => return Err(LoftyError::Mp3("Frame header uses a reserved layer")),
 		};
 
-		let bitrate = (header >> 12) & 0b1111;
-		let sample_rate = (header >> 10) & 0b11;
+		let layer_index = (layer as usize).saturating_sub(1);
 
-		if sample_rate == 0 {
-			return Err(LoftyError::Mp3("Frame header has a sample rate of 0"));
+		let bitrate_index = (header >> 12) & 0xf;
+		let bitrate = BITRATES[version_index][layer_index][bitrate_index as usize];
+
+		// Sample rate index
+		let mut sample_rate = (header >> 10) & 3;
+
+		match sample_rate {
+			// This is invalid, but it doesn't seem worth it to error here
+			3 => sample_rate = 0,
+			_ => sample_rate = SAMPLE_RATES[version as usize][sample_rate as usize],
 		}
 
-		let mode = match (header >> 6) & 0b11 {
+		let has_padding = ((header >> 9) & 1) != 0;
+		let mut padding = 0;
+
+		if has_padding {
+			padding = u32::from(PADDING_SIZES[layer_index]);
+		}
+
+		let mode = match (header >> 6) & 3 {
 			0 => Mode::Stereo,
 			1 => Mode::JointStereo,
 			2 => Mode::DualChannel,
@@ -74,23 +88,13 @@ impl Header {
 			_ => return Err(LoftyError::Mp3("Unreachable error")),
 		};
 
-		let layer_index = (layer as usize).saturating_sub(1);
-		let version_index = if version == MpegVersion::V1 { 0 } else { 1 };
-		let has_padding = ((header >> 9) & 1) != 0;
+		let data_start = SIDE_INFORMATION_SIZES[version_index][mode as usize] + 4;
+		let samples = SAMPLES[layer_index][version_index];
 
-		let mut data_start = SIDE_INFORMATION_SIZES[version_index][mode as usize] + 4;
-
-		let bitrate = BITRATES[version_index][layer_index][bitrate as usize];
-		let sample_rate = SAMPLE_RATES[version as usize][sample_rate as usize];
-		let samples_per_frame = SAMPLES_PER_FRAME[layer_index][version_index];
-
-		let mut len = (u32::from(samples_per_frame) * (bitrate * 125)) / sample_rate;
-
-		if has_padding {
-			let padding = u32::from(PADDING_SIZES[layer_index]);
-			len += padding;
-			data_start += padding
-		}
+		let len = match layer {
+			Layer::Layer1 => (bitrate * 12000 / sample_rate + padding) * 4,
+			Layer::Layer2 | Layer::Layer3 => bitrate * 144_000 / sample_rate + padding,
+		};
 
 		let channels = if mode == Mode::SingleChannel { 1 } else { 2 };
 
@@ -99,7 +103,7 @@ impl Header {
 			channels,
 			len,
 			data_start,
-			samples_per_frame,
+			samples,
 			bitrate,
 		})
 	}

src/logic/mpeg/read.rs

@@ -1,8 +1,9 @@
-use super::header::{Header, XingHeader};
-use crate::files::MpegFile;
+use super::header::{verify_frame_sync, Header, XingHeader};
+use super::MpegFile;
+use crate::error::{LoftyError, Result};
 use crate::logic::id3::unsynch_u32;
 use crate::logic::id3::v2::read::parse_id3v2;
-use crate::{FileProperties, LoftyError, Result};
+use crate::types::properties::FileProperties;
 
 use std::io::{Read, Seek, SeekFrom};
 use std::time::Duration;
@@ -16,9 +17,9 @@ fn read_properties(
 ) -> FileProperties {
 	let (duration, bitrate) = {
 		if let Some(xing_header) = xing_header {
-			if first_frame.0.samples_per_frame > 0 && first_frame.0.sample_rate > 0 {
+			if first_frame.0.samples > 0 && first_frame.0.sample_rate > 0 {
 				let frame_time =
-					u32::from(first_frame.0.samples_per_frame) * 1000 / first_frame.0.sample_rate;
+					u32::from(first_frame.0.samples) * 1000 / first_frame.0.sample_rate;
 				let length = u64::from(frame_time) * u64::from(xing_header.frames);
 
 				(
@@ -77,7 +78,7 @@ where
 
 	while let Ok(()) = data.read_exact(&mut header) {
 		match header {
-			_ if u32::from_be_bytes(header) >> 21 == 0x7FF => {
+			_ if verify_frame_sync(u16::from_be_bytes([header[0], header[1]])) => {
 				let start = data.seek(SeekFrom::Current(0))? - 4;
 				let header = Header::read(u32::from_be_bytes(header))?;
 				data.seek(SeekFrom::Current(i64::from(header.len - 4)))?;
@@ -88,6 +89,7 @@ where
 
 				last_mpeg_frame = (Some(header), start);
 			},
+			// [I, D, 3, ver_major, ver_minor, flags, size (4 bytes)]
 			[b'I', b'D', b'3', ..] => {
 				let mut remaining_header = [0; 6];
 				data.read_exact(&mut remaining_header)?;

src/types/file.rs

@@ -342,13 +342,13 @@ impl FileType {
 
 				if &ident == b"MAC" {
 					Ok(Self::APE)
-				} else if verify_frame_sync(ident[0], ident[1]) {
+				} else if verify_frame_sync(u16::from_be_bytes([ident[0], ident[1]])) {
 					Ok(Self::MP3)
 				} else {
 					Err(LoftyError::UnknownFormat)
 				}
 			},
-			_ if verify_frame_sync(sig[0], sig[1]) => Ok(Self::MP3),
+			_ if verify_frame_sync(u16::from_be_bytes([sig[0], sig[1]])) => Ok(Self::MP3),
 			70 if sig.starts_with(b"FORM") => {
 				let mut id_remaining = [0; 2];
 				data.read_exact(&mut id_remaining)?;