Musepack: Improve audio properties

CHANGELOG.md

@@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **MPEG**: Durations estimated by bitrate are more accurate ([PR](https://github.com/Serial-ATA/lofty-rs/pull/395))
 - **MP4**: Bitrate calculation is now more accurate ([PR](https://github.com/Serial-ATA/lofty-rs/pull/398))
 - **WAV**: Bitrate calculation is now more accurate ([PR](https://github.com/Serial-ATA/lofty-rs/pull/399))
+- **MusePack**: Overall improved audio properties  ([PR](https://github.com/Serial-ATA/lofty-rs/pull/402))
 
 ## [0.19.2] - 2024-04-26
 

lofty/src/id3/v2/header.rs

@@ -44,6 +44,7 @@ pub struct Id3v2TagFlags {
 pub(crate) struct Id3v2Header {
 	pub version: Id3v2Version,
 	pub flags: Id3v2TagFlags,
+	/// The size of the tag contents (**DOES NOT INCLUDE THE HEADER/FOOTER**)
 	pub size: u32,
 	pub extended_size: u32,
 }
@@ -140,4 +141,9 @@ impl Id3v2Header {
 			extended_size,
 		})
 	}
+
+	/// The total size of the tag, including the header, footer, and extended header
+	pub(crate) fn full_tag_size(&self) -> u32 {
+		self.size + 10 + self.extended_size + if self.flags.footer { 10 } else { 0 }
+	}
 }

lofty/src/musepack/constants.rs

@@ -13,5 +13,5 @@ pub(super) const FREQUENCY_TABLE: [u32; 8] = [44100, 48000, 37800, 32000, 0, 0,
 /// This is the gain reference used in old ReplayGain
 pub const MPC_OLD_GAIN_REF: f32 = 64.82;
 
-pub(super) const MPC_DECODER_SYNTH_DELAY: u32 = 481;
-pub(super) const MPC_FRAME_LENGTH: u32 = 36 * 32; // Samples per mpc frame
+pub(super) const MPC_DECODER_SYNTH_DELAY: u64 = 481;
+pub(super) const MPC_FRAME_LENGTH: u64 = 36 * 32; // Samples per mpc frame

lofty/src/musepack/read.rs

@@ -32,12 +32,7 @@ where
 		let id3v2 = parse_id3v2(reader, header, parse_options.parsing_mode)?;
 		file.id3v2_tag = Some(id3v2);
 
-		let mut size = header.size;
-		if header.flags.footer {
-			size += 10;
-		}
-
-		stream_length -= u64::from(size);
+		stream_length -= u64::from(header.full_tag_size());
 	}
 
 	// Save the current position, so we can go back and read the properties after the tags

lofty/src/musepack/sv4to6/properties.rs

@@ -1,8 +1,9 @@
 use crate::config::ParsingMode;
 use crate::error::Result;
-use crate::macros::{decode_err, parse_mode_choice};
+use crate::macros::decode_err;
 use crate::musepack::constants::{MPC_DECODER_SYNTH_DELAY, MPC_FRAME_LENGTH};
 use crate::properties::FileProperties;
+use crate::util::math::RoundedDivision;
 
 use std::io::Read;
 use std::time::Duration;
@@ -17,7 +18,7 @@ pub struct MpcSv4to6Properties {
 	pub(crate) sample_rate: u32, // NOTE: always 44100
 
 	// Fields actually contained in the header
-	pub(crate) audio_bitrate: u32,
+	pub(crate) average_bitrate: u32,
 	pub(crate) mid_side_stereo: bool,
 	pub(crate) stream_version: u16,
 	pub(crate) max_band: u8,
@@ -28,8 +29,8 @@ impl From<MpcSv4to6Properties> for FileProperties {
 	fn from(input: MpcSv4to6Properties) -> Self {
 		Self {
 			duration: input.duration,
-			overall_bitrate: Some(input.audio_bitrate),
-			audio_bitrate: Some(input.audio_bitrate),
+			overall_bitrate: Some(input.average_bitrate),
+			audio_bitrate: Some(input.average_bitrate),
 			sample_rate: Some(input.sample_rate),
 			bit_depth: None,
 			channels: Some(input.channels),
@@ -54,9 +55,9 @@ impl MpcSv4to6Properties {
 		self.sample_rate
 	}
 
-	/// Audio bitrate (kbps)
-	pub fn audio_bitrate(&self) -> u32 {
-		self.audio_bitrate
+	/// Average bitrate (kbps)
+	pub fn average_bitrate(&self) -> u32 {
+		self.average_bitrate
 	}
 
 	/// Whether MidSideStereo is used
@@ -92,7 +93,7 @@ impl MpcSv4to6Properties {
 
 		let mut properties = Self::default();
 
-		properties.audio_bitrate = (header_data[0] >> 23) & 0x1FF;
+		properties.average_bitrate = (header_data[0] >> 23) & 0x1FF;
 		let intensity_stereo = (header_data[0] >> 22) & 0x1 == 1;
 		properties.mid_side_stereo = (header_data[0] >> 21) & 0x1 == 1;
 
@@ -110,22 +111,19 @@ impl MpcSv4to6Properties {
 			properties.frame_count = header_data[1] >> 16; // 16 bit
 		}
 
-		parse_mode_choice!(
-			parse_mode,
-			STRICT: {
-				if properties.audio_bitrate != 0 {
-					decode_err!(@BAIL Mpc, "Encountered CBR stream")
-				}
+		if parse_mode == ParsingMode::Strict {
+			if properties.average_bitrate != 0 {
+				decode_err!(@BAIL Mpc, "Encountered CBR stream")
+			}
 
-				if intensity_stereo {
-					decode_err!(@BAIL Mpc, "Stream uses intensity stereo coding")
-				}
+			if intensity_stereo {
+				decode_err!(@BAIL Mpc, "Stream uses intensity stereo coding")
+			}
 
-				if block_size != 1 {
-					decode_err!(@BAIL Mpc, "Stream has an invalid block size (must be 1)")
-				}
-			},
-		);
+			if block_size != 1 {
+				decode_err!(@BAIL Mpc, "Stream has an invalid block size (must be 1)")
+			}
+		}
 
 		if properties.stream_version < 6 {
 			// Versions before 6 had an invalid last frame
@@ -135,18 +133,28 @@ impl MpcSv4to6Properties {
 		properties.sample_rate = 44100;
 		properties.channels = 2;
 
-		if properties.frame_count > 0 {
-			let samples =
-				(properties.frame_count * MPC_FRAME_LENGTH).saturating_sub(MPC_DECODER_SYNTH_DELAY);
-			let length = f64::from(samples) / f64::from(properties.sample_rate);
-			properties.duration = Duration::from_millis(length.ceil() as u64);
-
-			let pcm_frames = 1152 * u64::from(properties.frame_count) - 576;
-			properties.audio_bitrate = ((stream_length as f64
-				* 8.0 * f64::from(properties.sample_rate))
-				/ pcm_frames as f64) as u32;
+		// Nothing more we can do
+		if properties.frame_count == 0 {
+			return Ok(properties);
 		}
 
+		let samples = (u64::from(properties.frame_count) * MPC_FRAME_LENGTH)
+			.saturating_sub(MPC_DECODER_SYNTH_DELAY);
+		let length = (samples * 1000).div_round(u64::from(properties.sample_rate));
+		properties.duration = Duration::from_millis(length);
+
+		// 576 is a magic number from the reference decoder
+		//
+		// Quote from the reference source (libmpcdec/trunk/src/streaminfo.c:248 @rev 153):
+		// "estimation, exact value needs too much time"
+		let pcm_frames = (MPC_FRAME_LENGTH * u64::from(properties.frame_count)).saturating_sub(576);
+
+		// Is this accurate? If not, it really doesn't matter.
+		properties.average_bitrate = ((stream_length as f64
+			* 8.0 * f64::from(properties.sample_rate))
+			/ (pcm_frames as f64)
+			/ (MPC_FRAME_LENGTH as f64)) as u32;
+
 		Ok(properties)
 	}
 }

lofty/src/musepack/sv7/properties.rs

@@ -1,12 +1,14 @@
 use crate::error::Result;
 use crate::macros::decode_err;
-use crate::musepack::constants::{FREQUENCY_TABLE, MPC_OLD_GAIN_REF};
+use crate::musepack::constants::{
+	FREQUENCY_TABLE, MPC_DECODER_SYNTH_DELAY, MPC_FRAME_LENGTH, MPC_OLD_GAIN_REF,
+};
 use crate::properties::FileProperties;
 
 use std::io::Read;
 use std::time::Duration;
 
-use byteorder::{BigEndian, LittleEndian, ReadBytesExt};
+use byteorder::{LittleEndian, ReadBytesExt};
 
 /// Used profile
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
@@ -104,8 +106,7 @@ impl Link {
 #[allow(clippy::struct_excessive_bools)]
 pub struct MpcSv7Properties {
 	pub(crate) duration: Duration,
-	pub(crate) overall_bitrate: u32,
-	pub(crate) audio_bitrate: u32,
+	pub(crate) average_bitrate: u32,
 	pub(crate) channels: u8, // NOTE: always 2
 	// -- Section 1 --
 	pub(crate) frame_count: u32,
@@ -135,8 +136,8 @@ impl From<MpcSv7Properties> for FileProperties {
 	fn from(input: MpcSv7Properties) -> Self {
 		Self {
 			duration: input.duration,
-			overall_bitrate: Some(input.overall_bitrate),
-			audio_bitrate: Some(input.audio_bitrate),
+			overall_bitrate: Some(input.average_bitrate),
+			audio_bitrate: Some(input.average_bitrate),
 			sample_rate: Some(input.sample_freq),
 			bit_depth: None,
 			channels: Some(input.channels),
@@ -151,14 +152,9 @@ impl MpcSv7Properties {
 		self.duration
 	}
 
-	/// Overall bitrate (kbps)
-	pub fn overall_bitrate(&self) -> u32 {
-		self.overall_bitrate
-	}
-
-	/// Audio bitrate (kbps)
-	pub fn audio_bitrate(&self) -> u32 {
-		self.audio_bitrate
+	/// Average bitrate (kbps)
+	pub fn average_bitrate(&self) -> u32 {
+		self.average_bitrate
 	}
 
 	/// Sample rate (Hz)
@@ -208,7 +204,7 @@ impl MpcSv7Properties {
 
 	/// Change in the replay level
 	///
-	/// The value is a signed 16 bit integer, with the level being attenuated by that many mB
+	/// The value is a signed 16-bit integer, with the level being attenuated by that many mB
 	pub fn title_gain(&self) -> i16 {
 		self.title_gain
 	}
@@ -224,7 +220,7 @@ impl MpcSv7Properties {
 
 	/// Change in the replay level if the whole CD is supposed to be played with the same level change
 	///
-	/// The value is a signed 16 bit integer, with the level being attenuated by that many mB
+	/// The value is a signed 16-bit integer, with the level being attenuated by that many mB
 	pub fn album_gain(&self) -> i16 {
 		self.album_gain
 	}
@@ -280,6 +276,9 @@ impl MpcSv7Properties {
 			..Self::default()
 		};
 
+		// TODO: Make a Bitreader, would be nice crate-wide but especially here
+		// The SV7 header is split into 6 32-bit sections
+
 		// -- Section 1 --
 		properties.frame_count = reader.read_u32::<LittleEndian>()?;
 
@@ -294,11 +293,8 @@ impl MpcSv7Properties {
 
 		let byte2 = ((chunk & 0xFF_0000) >> 16) as u8;
 
-		let profile_index = (byte2 & 0xF0) >> 4;
-		properties.profile = Profile::from_u8(profile_index).unwrap(); // Infallible
-
-		let link_index = (byte2 & 0x0C) >> 2;
-		properties.link = Link::from_u8(link_index).unwrap(); // Infallible
+		properties.profile = Profile::from_u8((byte2 & 0xF0) >> 4).unwrap(); // Infallible
+		properties.link = Link::from_u8((byte2 & 0x0C) >> 2).unwrap(); // Infallible
 
 		let sample_freq_index = byte2 & 0x03;
 		properties.sample_freq = FREQUENCY_TABLE[sample_freq_index as usize];
@@ -307,27 +303,24 @@ impl MpcSv7Properties {
 		properties.max_level = remaining_bytes;
 
 		// -- Section 3 --
-		let title_gain = reader.read_i16::<BigEndian>()?;
-		let title_peak = reader.read_u16::<BigEndian>()?;
+		let title_peak = reader.read_u16::<LittleEndian>()?;
+		let title_gain = reader.read_u16::<LittleEndian>()?;
 
 		// -- Section 4 --
-		let album_gain = reader.read_i16::<BigEndian>()?;
-		let album_peak = reader.read_u16::<BigEndian>()?;
+		let album_peak = reader.read_u16::<LittleEndian>()?;
+		let album_gain = reader.read_u16::<LittleEndian>()?;
 
 		// -- Section 5 --
 		let chunk = reader.read_u32::<LittleEndian>()?;
 
-		let byte1 = ((chunk & 0xFF00_0000) >> 24) as u8;
-
-		properties.true_gapless = ((byte1 & 0x80) >> 7) == 1;
-
-		let byte2 = ((chunk & 0xFF_0000) >> 16) as u8;
+		properties.true_gapless = (chunk >> 31) == 1;
 
 		if properties.true_gapless {
-			properties.last_frame_length =
-				(u16::from(byte1 & 0x7F) << 4) | u16::from((byte2 & 0xF0) >> 4);
+			properties.last_frame_length = ((chunk >> 20) & 0x7FF) as u16;
 		}
 
+		properties.fast_seeking_safe = (chunk >> 19) & 1 == 1;
+
 		// NOTE: Rest of the chunk is zeroed and unused
 
 		// -- Section 6 --
@@ -336,41 +329,59 @@ impl MpcSv7Properties {
 		// -- End of parsing --
 
 		// Convert ReplayGain values
-		let set_replay_gain = |gain: i16| -> i16 {
-			let mut gain = (MPC_OLD_GAIN_REF - f32::from(gain) / 100.0) * 256.0 + 0.5;
-			if gain >= ((1 << 16) as f32) || gain < 0.0 {
-				gain = 0.0
+		let set_replay_gain = |gain: u16| -> i16 {
+			if gain == 0 {
+				return 0;
 			}
-			gain as i16
+
+			let gain = ((MPC_OLD_GAIN_REF - f32::from(gain) / 100.0) * 256.0 + 0.5) as i16;
+			if !(0..i16::MAX).contains(&gain) {
+				return 0;
+			}
+			gain
 		};
 		let set_replay_peak = |peak: u16| -> u16 {
 			if peak == 0 {
 				return 0;
 			}
 
-			((peak.ilog10() * 20 * 256) as f32 + 0.5) as u16
+			((f64::from(peak).log10() * 20.0 * 256.0) + 0.5) as u16
 		};
 
 		properties.title_gain = set_replay_gain(title_gain);
 		properties.title_peak = set_replay_peak(title_peak);
 		properties.album_gain = set_replay_gain(album_gain);
 		properties.album_peak = set_replay_peak(album_peak);
 
+		if properties.last_frame_length > MPC_FRAME_LENGTH as u16 {
+			decode_err!(@BAIL Mpc, "Invalid last frame length");
+		}
+
+		if properties.sample_freq == 0 {
+			log::warn!("Sample rate is 0, unable to calculate duration and bitrate");
+			return Ok(properties);
+		}
+
+		if properties.frame_count == 0 {
+			log::warn!("Frame count is 0, unable to calculate duration and bitrate");
+			return Ok(properties);
+		}
+
+		let time_per_frame = (MPC_FRAME_LENGTH as f64) / f64::from(properties.sample_freq);
+		let length = (f64::from(properties.frame_count) * time_per_frame) * 1000.0;
+		properties.duration = Duration::from_millis(length as u64);
+
 		let total_samples;
 		if properties.true_gapless {
-			total_samples =
-				(properties.frame_count * 1152) - u32::from(properties.last_frame_length);
+			total_samples = (u64::from(properties.frame_count) * MPC_FRAME_LENGTH)
+				- (MPC_FRAME_LENGTH - u64::from(properties.last_frame_length));
 		} else {
-			total_samples = (properties.frame_count * 1152) - 576;
+			total_samples =
+				(u64::from(properties.frame_count) * MPC_FRAME_LENGTH) - MPC_DECODER_SYNTH_DELAY;
 		}
 
-		if total_samples > 0 && properties.sample_freq > 0 {
-			let length =
-				(f64::from(total_samples) * 1000.0 / f64::from(properties.sample_freq)).ceil();
-			properties.duration = Duration::from_millis(length as u64);
-			properties.audio_bitrate = (stream_length * 8 / length as u64) as u32;
-			properties.overall_bitrate = properties.audio_bitrate;
-		}
+		properties.average_bitrate = ((stream_length * 8 * u64::from(properties.sample_freq))
+			/ (total_samples * 1000)) as u32;
 
 		Ok(properties)
 	}