WavPack: Fix property reading

CHANGELOG.md

@@ -25,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - The parser has been further restricted to avoid going out of bounds
     - This was only an issue if there was garbage data after the `moov` item *and* the parser had not yet found
       the `moov.udta` atom.
+- **WavPack**:
+  - Fewer errors are suppressed
+  - Metadata sub-blocks are now properly parsed
+  - Bitrate calculation will now properly round down
 
 ## [0.11.0] - 2023-1-29
 

src/properties.rs

@@ -319,8 +319,8 @@ mod tests {
 	const WAVPACK_PROPERTIES: WavPackProperties = WavPackProperties {
 		version: 1040,
 		duration: Duration::from_millis(1428),
-		overall_bitrate: 599,
-		audio_bitrate: 598,
+		overall_bitrate: 598,
+		audio_bitrate: 597,
 		sample_rate: 48000,
 		channels: 2,
 		bit_depth: 16,

src/wavpack/properties.rs

@@ -126,38 +126,9 @@ where
 			_ => break,
 		}
 
-		// Just skip any block with no samples
-		if block_header.samples == 0 {
-			offset += u64::from(block_header.block_size + 8);
-			continue;
-		}
-
 		let flags = block_header.flags;
 
-		let sample_rate_idx = ((flags >> 23) & 0xF) as usize;
-		let sample_rate = SAMPLE_RATES[sample_rate_idx];
-
-		// In the case of non-standard sample rates and DSD audio, we need to actually read the
-		// block to get the sample rate
-		if sample_rate == 0 || flags & FLAG_DSD == FLAG_DSD {
-			let mut block_contents = try_vec![0; (block_header.block_size - 24) as usize];
-			if reader.read_exact(&mut block_contents).is_err() {
-				parse_mode_choice!(
-					parse_mode,
-					STRICT: decode_err!(@BAIL WavPack, "Block size mismatch"),
-					DEFAULT: break
-				);
-			}
-
-			if get_extended_meta_info(reader, &mut properties, block_contents.len() as u64).is_err()
-			{
-				break;
-			}
-		} else {
-			properties.sample_rate = sample_rate;
-		}
-
-		if (flags & FLAG_INITIAL_BLOCK) == FLAG_INITIAL_BLOCK {
+		if flags & FLAG_INITIAL_BLOCK == FLAG_INITIAL_BLOCK {
 			if block_header.version < MIN_STREAM_VERSION
 				|| block_header.version > MAX_STREAM_VERSION
 			{
@@ -170,12 +141,55 @@ where
 
 			total_samples = block_header.total_samples;
 			properties.bit_depth = ((((flags & BYTES_PER_SAMPLE_MASK) + 1) * 8) - ((flags & BIT_DEPTH_SHIFT_MASK) >> BIT_DEPTH_SHL)) as u8;
+
+			let sample_rate_idx = ((flags >> 23) & 0xF) as usize;
+			let sample_rate = SAMPLE_RATES[sample_rate_idx];
+			properties.sample_rate = sample_rate;
+
 			properties.version = block_header.version;
 			properties.lossless = flags & FLAG_HYBRID_COMPRESSION == 0;
+
+			let is_mono = flags & FLAG_MONO > 0;
+			properties.channels = if is_mono { 1 } else { 2 };
+
+			// In the case of non-standard sample rates and DSD audio, we need to actually read the
+			// block to get the sample rate
+			if sample_rate == 0 || flags & FLAG_DSD == FLAG_DSD {
+				let mut block_contents = try_vec![0; (block_header.block_size - 24) as usize];
+				if reader.read_exact(&mut block_contents).is_err() {
+					parse_mode_choice!(
+						parse_mode,
+						STRICT: decode_err!(@BAIL WavPack, "Block size mismatch"),
+						DEFAULT: break
+					);
+				}
+
+				if let Err(e) = get_extended_meta_info(parse_mode, &block_contents, &mut properties)
+				{
+					parse_mode_choice!(
+						parse_mode,
+						STRICT: return Err(e),
+						DEFAULT: break
+					);
+				}
+			}
+
+			// A sample rate index of 15 indicates a custom sample rate, which should have been found
+			// when we just parsed the metadata blocks
+			if sample_rate_idx == 15 && properties.sample_rate == 0 {
+				parse_mode_choice!(
+					parse_mode,
+					STRICT: decode_err!(@BAIL WavPack, "Expected custom sample rate"),
+					DEFAULT: break
+				)
+			}
 		}
 
-		let is_mono = flags & FLAG_MONO > 0;
-		properties.channels = if is_mono { 1 } else { 2 };
+		// Just skip any block with no samples
+		if block_header.samples == 0 {
+			offset += u64::from(block_header.block_size + 8);
+			continue;
+		}
 
 		if flags & FLAG_FINAL_BLOCK == FLAG_FINAL_BLOCK {
 			break;
@@ -185,12 +199,12 @@ where
 	}
 
 	if total_samples > 0 && properties.sample_rate > 0 {
-		let length = u64::from(total_samples * 1000 / properties.sample_rate);
-		properties.duration = Duration::from_millis(length);
-		properties.audio_bitrate = crate::div_ceil(stream_length * 8, length) as u32;
+		let length = f64::from(total_samples) * 1000. / f64::from(properties.sample_rate);
+		properties.duration = Duration::from_millis((length + 0.5) as u64);
+		properties.audio_bitrate = (stream_length as f64 * 8. / length + 0.5) as u32;
 
 		let file_length = reader.seek(SeekFrom::End(0))?;
-		properties.overall_bitrate = crate::div_ceil(file_length * 8, length) as u32;
+		properties.overall_bitrate = (file_length as f64 * 8. / length + 0.5) as u32;
 	} else {
 		parse_mode_choice!(
 			parse_mode,
@@ -254,77 +268,93 @@ where
 	})
 }
 
-fn get_extended_meta_info<R>(
-	reader: &mut R,
+fn get_extended_meta_info(
+	parse_mode: ParsingMode,
+	block_content: &[u8],
 	properties: &mut WavPackProperties,
-	block_size: u64,
-) -> Result<()>
-where
-	R: Read + Seek,
-{
-	while reader.stream_position()? < block_size {
-		let id = reader.read_u8()?;
+) -> Result<()> {
+	let mut index = 0;
+	let block_size = block_content.len();
+	while index < block_size {
+		let id = block_content[index];
+		index += 1;
+
+		let mut size = u32::from(block_content[index]) << 1;
+		index += 1;
 
 		let is_large = id & ID_FLAG_LARGE_SIZE > 0;
-		let mut size = if is_large {
-			reader.read_u24::<LittleEndian>()? << 1
-		} else {
-			u32::from(reader.read_u8()?) << 1
-		};
+		if is_large {
+			size += u32::from(block_content[index]) << 9;
+			size += u32::from(block_content[index + 1]) << 17;
+			index += 2;
+		}
 
 		if id & ID_FLAG_ODD_SIZE > 0 {
 			size -= 1;
 		}
 
 		match id & 0x3F {
 			ID_NON_STANDARD_SAMPLE_RATE => {
-				properties.sample_rate = reader.read_u24::<LittleEndian>()?;
+				properties.sample_rate =
+					(&mut &block_content[index..]).read_u24::<LittleEndian>()?;
 			},
 			ID_DSD => {
 				if size <= 1 {
 					decode_err!(@BAIL WavPack, "Encountered an invalid DSD block size");
 				}
 
-				let rate_multiplier = u32::from(reader.read_u8()?);
-				if let (sample_rate, false) =
-					properties.sample_rate.overflowing_shl(rate_multiplier)
-				{
-					properties.sample_rate = sample_rate;
+				let mut rate_multiplier = u32::from(block_content[index]);
+				index += 1;
+
+				if rate_multiplier > 30 {
+					parse_mode_choice!(
+						parse_mode,
+						STRICT: decode_err!(@BAIL WavPack, "Encountered an invalid sample rate multiplier"),
+						DEFAULT: break
+					)
 				}
 
-				reader.seek(SeekFrom::Current(i64::from(size - 1)))?;
+				rate_multiplier = 1 << rate_multiplier;
+				properties.sample_rate = properties.sample_rate.wrapping_mul(rate_multiplier);
+
+				// Skip DSD mode
+				index += 1;
 			},
 			ID_MULTICHANNEL => {
 				if size <= 1 {
 					decode_err!(@BAIL WavPack, "Unable to extract channel information");
 				}
 
-				properties.channels = reader.read_u8()?;
+				properties.channels = block_content[index];
+				index += 1;
+
 				let s = size - 2;
 				match s {
 					0..=3 => {
-						reader.seek(SeekFrom::Current(i64::from(s + 1)))?;
+						// Skip the Microsoft channel mask
+						index += (s + 1) as usize;
 						continue;
 					},
 					4 | 5 => {},
 					_ => decode_err!(@BAIL WavPack, "Encountered invalid channel info size"),
 				}
 
-				reader.seek(SeekFrom::Current(1))?;
+				index += 1;
 
-				properties.channels |= reader.read_u8()? & 0xF;
+				properties.channels |= block_content[1] & 0xF;
 				properties.channels += 1;
+				index += 1;
 
 				// Skip the Microsoft channel mask
-				reader.seek(SeekFrom::Current(i64::from(s - 1)))?;
+				index += (s - 1) as usize;
 			},
 			_ => {
-				reader.seek(SeekFrom::Current(i64::from(size)))?;
+				index += size as usize;
 			},
 		}
 
 		if id & ID_FLAG_ODD_SIZE > 0 {
-			reader.seek(SeekFrom::Current(1))?;
+			index += 1;
 		}
 	}