diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b319886..ef97b913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - **Pictures**: Treat "image/jpg" as `MimeType::Jpeg` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/41)) -- **MP3**: Properly validate the contents of Xing/LAME/VBRI headers ([issue](https://github.com/Serial-ATA/lofty-rs/issues/42)) - - A header with any field zeroed out would result in a division by zero panic +- **MP3**: + - Properly validate the contents of Xing/LAME/VBRI headers ([issue](https://github.com/Serial-ATA/lofty-rs/issues/42)) + - A header with any field zeroed out would result in a division by zero panic + - Fix duration estimation for files with Xing headers without the necessary flags - **FLAC**: Fix property reading of zero-length files ([issue](https://github.com/Serial-ATA/lofty-rs/issues/46)) - **Vorbis Comments**: Fix reading of vendor strings with invalid mixed UTF-8 and UTF-16 encodings - **ID3v2**: diff --git a/src/mp3/header.rs b/src/mp3/header.rs index f8a95cdb..7c702d36 100644 --- a/src/mp3/header.rs +++ b/src/mp3/header.rs @@ -2,7 +2,8 @@ use super::constants::{BITRATES, PADDING_SIZES, SAMPLES, SAMPLE_RATES, SIDE_INFO use crate::error::{FileDecodingError, Result}; use crate::file::FileType; -use std::io::Read; +use std::io::{Read, Seek, SeekFrom}; +use std::ops::Neg; use byteorder::{BigEndian, ReadBytesExt}; @@ -43,6 +44,30 @@ where Ok(None) } +// If we need to find the last frame offset (the file has no Xing/LAME/VBRI header) +// +// This will search up to 1024 bytes preceding the APE tag/ID3v1/EOF. +// Unlike `search_for_frame_sync`, since this has the `Seek` bound, it will seek the reader +// back to the start of the header. +const REV_FRAME_SEARCH_BOUNDS: i64 = 1024; +pub(super) fn rev_search_for_frame_sync(input: &mut R) -> std::io::Result> +where + R: Read + Seek, +{ + let res = input.seek(SeekFrom::Current(REV_FRAME_SEARCH_BOUNDS.neg())); + if res.is_err() { + return Ok(None); + } + + let ret = search_for_frame_sync(&mut input.take(REV_FRAME_SEARCH_BOUNDS as u64)); + if let Ok(Some(_)) = ret { + // Seek to the start of the frame sync + input.seek(SeekFrom::Current(-2))?; + } + + ret +} + #[derive(PartialEq, Copy, Clone, Debug)] #[allow(missing_docs)] /// MPEG Audio version @@ -111,7 +136,6 @@ impl Default for Emphasis { #[derive(Copy, Clone)] pub(crate) struct Header { pub(crate) sample_rate: u32, - pub(crate) channels: u8, pub(crate) len: u32, pub(crate) data_start: u32, pub(crate) samples: u16, @@ -126,8 +150,8 @@ pub(crate) struct Header { } impl Header { - pub(crate) fn read(header: u32) -> Result { - let version = match (header >> 19) & 0b11 { + pub(super) fn read(data: u32) -> Result { + let version = match (data >> 19) & 0b11 { 0 => MpegVersion::V2_5, 2 => MpegVersion::V2, 3 => MpegVersion::V1, @@ -142,7 +166,7 @@ impl Header { let version_index = if version == MpegVersion::V1 { 0 } else { 1 }; - let layer = match (header >> 17) & 3 { + let layer = match (data >> 17) & 0b11 { 1 => Layer::Layer3, 2 => Layer::Layer2, 3 => Layer::Layer1, @@ -155,29 +179,46 @@ impl Header { }, }; + let mut header = Header { + sample_rate: 0, + len: 0, + data_start: 0, + samples: 0, + bitrate: 0, + version, + layer, + channel_mode: ChannelMode::default(), + mode_extension: None, + copyright: false, + original: false, + emphasis: Emphasis::default(), + }; + let layer_index = (layer as usize).saturating_sub(1); - 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 - // We will error if properties are read - 3 => sample_rate = 0, - _ => sample_rate = SAMPLE_RATES[version as usize][sample_rate as usize], + let bitrate_index = (data >> 12) & 0xF; + header.bitrate = BITRATES[version_index][layer_index][bitrate_index as usize]; + if header.bitrate == 0 { + return Ok(header); } - let has_padding = ((header >> 9) & 1) == 1; + // Sample rate index + let sample_rate_index = (data >> 10) & 0b11; + header.sample_rate = match sample_rate_index { + // This is invalid, but it doesn't seem worth it to error here + // We will error if properties are read + 3 => return Ok(header), + _ => SAMPLE_RATES[version as usize][sample_rate_index as usize], + }; + + let has_padding = ((data >> 9) & 1) == 1; let mut padding = 0; if has_padding { padding = u32::from(PADDING_SIZES[layer_index]); } - let channel_mode = match (header >> 6) & 3 { + header.channel_mode = match (data >> 6) & 3 { 0 => ChannelMode::Stereo, 1 => ChannelMode::JointStereo, 2 => ChannelMode::DualChannel, @@ -185,16 +226,16 @@ impl Header { _ => unreachable!(), }; - let mut mode_extension = None; - - if let ChannelMode::JointStereo = channel_mode { - mode_extension = Some(((header >> 4) & 3) as u8); + if let ChannelMode::JointStereo = header.channel_mode { + header.mode_extension = Some(((data >> 4) & 3) as u8); + } else { + header.mode_extension = None; } - let copyright = ((header >> 3) & 1) == 1; - let original = ((header >> 2) & 1) == 1; + header.copyright = ((data >> 3) & 1) == 1; + header.original = ((data >> 2) & 1) == 1; - let emphasis = match header & 3 { + header.emphasis = match data & 3 { 0 => Emphasis::None, 1 => Emphasis::MS5015, 2 => Emphasis::Reserved, @@ -202,39 +243,12 @@ impl Header { _ => unreachable!(), }; - let data_start = SIDE_INFORMATION_SIZES[version_index][channel_mode as usize] + 4; - let samples = SAMPLES[layer_index][version_index]; + header.data_start = SIDE_INFORMATION_SIZES[version_index][header.channel_mode as usize] + 4; + header.samples = SAMPLES[layer_index][version_index]; + header.len = + (u32::from(header.samples) * header.bitrate * 125 / header.sample_rate) + padding; - let len = if sample_rate == 0 { - 0 - } else { - match layer { - Layer::Layer1 => (bitrate * 12000 / sample_rate + padding) * 4, - Layer::Layer2 | Layer::Layer3 => bitrate * 144_000 / sample_rate + padding, - } - }; - - let channels = if channel_mode == ChannelMode::SingleChannel { - 1 - } else { - 2 - }; - - Ok(Self { - sample_rate, - channels, - len, - data_start, - samples, - bitrate, - version, - layer, - channel_mode, - mode_extension, - copyright, - original, - emphasis, - }) + Ok(header) } } @@ -264,11 +278,13 @@ impl XingHeader { reader.read_exact(&mut flags)?; if flags[3] & 0x03 != 0x03 { - return Err(FileDecodingError::new( - FileType::MP3, - "Xing header doesn't have required flags set (0x0001 and 0x0002)", - ) - .into()); + return Ok(None); + // TODO: Debug message? + // return Err(FileDecodingError::new( + // FileType::MP3, + // "Xing header doesn't have required flags set (0x0001 and 0x0002)", + // ) + // .into()); } let frames = reader.read_u32::()?; diff --git a/src/mp3/properties.rs b/src/mp3/properties.rs index bd2c11e9..7fbae5a6 100644 --- a/src/mp3/properties.rs +++ b/src/mp3/properties.rs @@ -1,8 +1,13 @@ use super::header::{ChannelMode, Emphasis, Header, Layer, MpegVersion, XingHeader}; +use crate::error::Result; +use crate::mp3::header::rev_search_for_frame_sync; use crate::properties::FileProperties; +use std::io::{Read, Seek, SeekFrom}; use std::time::Duration; +use byteorder::{BigEndian, ReadBytesExt}; + #[derive(Debug, Clone, Copy, PartialEq, Default)] #[non_exhaustive] /// An MP3 file's audio properties @@ -96,12 +101,16 @@ impl Mp3Properties { } } -pub(super) fn read_properties( +pub(super) fn read_properties( + reader: &mut R, first_frame: (Header, u64), - last_frame_offset: u64, + mut last_frame_offset: u64, xing_header: Option, file_length: u64, -) -> Mp3Properties { +) -> Result +where + R: Read + Seek, +{ let first_frame_header = first_frame.0; let first_frame_offset = first_frame.1; @@ -116,7 +125,11 @@ pub(super) fn read_properties( overall_bitrate: 0, audio_bitrate: 0, sample_rate: first_frame_header.sample_rate, - channels: first_frame_header.channels, + channels: if first_frame_header.channel_mode == ChannelMode::SingleChannel { + 1 + } else { + 2 + }, emphasis: first_frame_header.emphasis, }; @@ -131,18 +144,42 @@ pub(super) fn read_properties( properties.audio_bitrate = ((u64::from(xing_header.size) * 8) / length) as u32; }, _ if first_frame_header.bitrate > 0 => { - let audio_bitrate = first_frame_header.bitrate; + properties.audio_bitrate = first_frame_header.bitrate; - let stream_length = - last_frame_offset - first_frame_offset + u64::from(first_frame_header.len); - let length = (stream_length * 8) / u64::from(audio_bitrate); + // Search for the last frame, starting at the end of the frames + reader.seek(SeekFrom::Start(last_frame_offset))?; - properties.audio_bitrate = audio_bitrate; - properties.overall_bitrate = ((file_length * 8) / length) as u32; - properties.duration = Duration::from_millis(length); + let mut last_frame = None; + while last_frame_offset > 0 { + match rev_search_for_frame_sync(reader) { + // Found a frame sync, attempt to read a header + Ok(Some(_)) => { + // Move `last_frame_offset` back to the actual position + last_frame_offset = reader.stream_position()?; + last_frame = Some(Header::read(reader.read_u32::()?)?); + + break; + }, + // Encountered some IO error, just break + Err(_) => break, + // No frame sync found, continue further back in the file + _ => {}, + } + } + + if let Some(last_frame_header) = last_frame { + let stream_len = + last_frame_offset - first_frame_offset + u64::from(last_frame_header.len); + let length = (stream_len * 8) / u64::from(properties.audio_bitrate); + + if length > 0 { + properties.overall_bitrate = ((file_length * 8) / length) as u32; + properties.duration = Duration::from_millis(length); + } + } }, _ => {}, } - properties + Ok(properties) } diff --git a/src/mp3/read.rs b/src/mp3/read.rs index d8834749..91c158c5 100644 --- a/src/mp3/read.rs +++ b/src/mp3/read.rs @@ -80,20 +80,12 @@ where // Seek back the length of the temporary header buffer, to include them // in the frame sync search #[allow(clippy::neg_multiply)] - let start_of_search_area = reader.seek(SeekFrom::Current(-1 * header.len() as i64))?; + reader.seek(SeekFrom::Current(-1 * header.len() as i64))?; - if let Some(first_mp3_frame_start_relative) = search_for_frame_sync(reader)? { - let first_mp3_frame_start_absolute = - start_of_search_area + first_mp3_frame_start_relative; - - // Seek back to the start of the frame and read the header - reader.seek(SeekFrom::Start(first_mp3_frame_start_absolute))?; - let header = Header::read(reader.read_u32::()?)?; - - file.first_frame_offset = first_mp3_frame_start_absolute; - first_frame_header = Some(header); - - // We have found the first frame + #[allow(clippy::used_underscore_binding)] + if let Some((_first_first_header, first_frame_offset)) = find_next_frame(reader)? { + file.first_frame_offset = first_frame_offset; + first_frame_header = Some(_first_first_header); break; } }, @@ -161,14 +153,53 @@ where let xing_header = XingHeader::read(&mut &xing_reader[..])?; super::properties::read_properties( + reader, (first_frame_header, first_frame_offset), file.last_frame_offset, xing_header, file_length, - ) + )? } else { Mp3Properties::default() }; Ok(file) } + +// Searches for the next frame, comparing it to the following one +fn find_next_frame(reader: &mut R) -> Result> +where + R: Read + Seek, +{ + // Used to compare the versions, layers, and sample rates of two frame headers. + // If they aren't equal, something is broken. + const HEADER_MASK: u32 = 0xFFFE_0C00; + + let mut pos = reader.stream_position()?; + + while let Ok(Some(first_mp3_frame_start_relative)) = search_for_frame_sync(reader) { + let first_mp3_frame_start_absolute = pos + first_mp3_frame_start_relative; + + // Seek back to the start of the frame and read the header + reader.seek(SeekFrom::Start(first_mp3_frame_start_absolute))?; + let first_header_data = reader.read_u32::()?; + let first_header = Header::read(first_header_data)?; + + // Read the next header and see if they are the same + reader.seek(SeekFrom::Current(i64::from( + first_header.len.saturating_sub(4), + )))?; + + match reader.read_u32::() { + Ok(second_header_data) + if first_header_data & HEADER_MASK == second_header_data & HEADER_MASK => + { + return Ok(Some((first_header, first_mp3_frame_start_absolute))); + }, + Err(_) => return Ok(None), + _ => pos = reader.stream_position()?, + } + } + + Ok(None) +}