MPEG: Improve duration calculation

For files with a VBR header:
Thanks to @naglis for correcting the length calculation. (issue: #412)

For files without a VBR header:
`rev_search_for_frame_header` would get tripped up on files with trailing data
that looked like a frame sync (ex. 0xFFFF). This would also result in durations that are
slightly off.

For now, VBR streams are still assumed to be CBR. I have not seen a file this does not
work for yet. Eventually it would be nice to have more accurate calculation, but that will require we read the *entire* file.
This commit is contained in:
Serial 2024-07-02 15:47:37 -04:00 committed by Alex
parent dde24d5b6d
commit b7adbd19ef
3 changed files with 87 additions and 59 deletions

View file

@ -1,5 +1,4 @@
use super::constants::{BITRATES, PADDING_SIZES, SAMPLES, SAMPLE_RATES, SIDE_INFORMATION_SIZES};
use crate::config::ParsingMode;
use crate::error::Result;
use crate::macros::decode_err;
@ -50,11 +49,7 @@ where
// 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: u64 = 1024;
pub(super) fn rev_search_for_frame_header<R>(
input: &mut R,
pos: &mut u64,
parse_mode: ParsingMode,
) -> Result<Option<Header>>
pub(super) fn rev_search_for_frame_header<R>(input: &mut R, pos: &mut u64) -> Result<Option<Header>>
where
R: Read + Seek,
{
@ -70,36 +65,33 @@ where
for (i, byte) in buf.iter().rev().enumerate() {
frame_sync[1] = frame_sync[0];
frame_sync[0] = *byte;
if verify_frame_sync(frame_sync) {
let relative_frame_start = (search_bounds as usize) - (i + 1);
if relative_frame_start + 4 > buf.len() {
if parse_mode == ParsingMode::Strict {
decode_err!(@BAIL Mpeg, "Expected full frame header (incomplete stream?)")
}
log::warn!("MPEG: Incomplete frame header, giving up");
break;
}
let header = Header::read(u32::from_be_bytes([
frame_sync[0],
frame_sync[1],
buf[relative_frame_start + 2],
buf[relative_frame_start + 3],
]));
// We need to check if the header is actually valid. For
// all we know, we could be in some junk (ex. 0xFF_FF_FF_FF).
if header.is_none() {
continue;
}
// Seek to the start of the frame sync
*pos += relative_frame_start as u64;
input.seek(SeekFrom::Start(*pos))?;
return Ok(header);
if !verify_frame_sync(frame_sync) {
continue;
}
let relative_frame_start = (search_bounds as usize) - (i + 1);
if relative_frame_start + 4 > buf.len() {
continue;
}
let header = Header::read(u32::from_be_bytes([
frame_sync[0],
frame_sync[1],
buf[relative_frame_start + 2],
buf[relative_frame_start + 3],
]));
// We need to check if the header is actually valid. For
// all we know, we could be in some junk (ex. 0xFF_FF_FF_FF).
if header.is_none() {
continue;
}
// Seek to the start of the frame sync
*pos += relative_frame_start as u64;
input.seek(SeekFrom::Start(*pos))?;
return Ok(header);
}
Ok(None)
@ -326,7 +318,16 @@ impl Header {
}
}
#[derive(Copy, Clone)]
pub(super) enum VbrHeaderType {
Xing,
Info,
Vbri,
}
#[derive(Copy, Clone)]
pub(super) struct VbrHeader {
pub ty: VbrHeaderType,
pub frames: u32,
pub size: u32,
}
@ -357,7 +358,13 @@ impl VbrHeader {
let frames = reader.read_u32::<BigEndian>()?;
let size = reader.read_u32::<BigEndian>()?;
Ok(Some(Self { frames, size }))
let ty = match &header {
b"Xing" => VbrHeaderType::Xing,
b"Info" => VbrHeaderType::Info,
_ => unreachable!(),
};
Ok(Some(Self { ty, frames, size }))
},
b"VBRI" => {
if reader_len < 32 {
@ -373,7 +380,11 @@ impl VbrHeader {
let size = reader.read_u32::<BigEndian>()?;
let frames = reader.read_u32::<BigEndian>()?;
Ok(Some(Self { frames, size }))
Ok(Some(Self {
ty: VbrHeaderType::Vbri,
frames,
size,
}))
},
_ => Ok(None),
}
@ -386,7 +397,6 @@ impl VbrHeader {
#[cfg(test)]
mod tests {
use crate::config::ParsingMode;
use crate::tag::utils::test_utils::read_path;
use std::io::{Cursor, Read, Seek, SeekFrom};
@ -410,7 +420,7 @@ mod tests {
// We have to start these at the end to do a reverse search, of course :)
let mut pos = reader.seek(SeekFrom::End(0)).unwrap();
let ret = super::rev_search_for_frame_header(reader, &mut pos, ParsingMode::Strict);
let ret = super::rev_search_for_frame_header(reader, &mut pos);
if expected_reader_position.is_some() {
assert!(ret.is_ok());

View file

@ -1,5 +1,4 @@
use super::header::{ChannelMode, Emphasis, Header, Layer, MpegVersion, VbrHeader};
use crate::config::ParsingMode;
use super::header::{ChannelMode, Emphasis, Header, Layer, MpegVersion, VbrHeader, VbrHeaderType};
use crate::error::Result;
use crate::mpeg::header::rev_search_for_frame_header;
use crate::properties::{ChannelMask, FileProperties};
@ -126,9 +125,8 @@ pub(super) fn read_properties<R>(
reader: &mut R,
first_frame: (Header, u64),
mut last_frame_offset: u64,
xing_header: Option<VbrHeader>,
vbr_header: Option<VbrHeader>,
file_length: u64,
parse_mode: ParsingMode,
) -> Result<()>
where
R: Read + Seek,
@ -150,15 +148,20 @@ where
2
};
if let Some(xing_header) = xing_header {
if first_frame_header.sample_rate > 0 && xing_header.is_valid() {
let frame_time = (u32::from(first_frame_header.samples) * 1000)
.div_round(first_frame_header.sample_rate);
let length = u64::from(frame_time) * u64::from(xing_header.frames);
if let Some(vbr_header) = vbr_header {
if first_frame_header.sample_rate > 0 && vbr_header.is_valid() {
log::debug!("MPEG: Valid VBR header; using it to calculate duration");
let sample_rate = u64::from(first_frame_header.sample_rate);
let samples_per_frame = u64::from(first_frame_header.samples);
let total_frames = u64::from(vbr_header.frames);
let length = (samples_per_frame * 1000 * total_frames).div_round(sample_rate);
properties.duration = Duration::from_millis(length);
properties.overall_bitrate = ((file_length * 8) / length) as u32;
properties.audio_bitrate = ((u64::from(xing_header.size) * 8) / length) as u32;
properties.audio_bitrate = ((u64::from(vbr_header.size) * 8) / length) as u32;
return Ok(());
}
@ -171,7 +174,14 @@ where
log::warn!("MPEG: Using bitrate to estimate duration");
properties.audio_bitrate = first_frame_header.bitrate;
// http://gabriel.mp3-tech.org/mp3infotag.html:
//
// "In the Info Tag, the "Xing" identification string (mostly at 0x24) of the header is replaced by "Info" in case of a CBR file."
let is_cbr = matches!(vbr_header.map(|h| h.ty), Some(VbrHeaderType::Info));
if is_cbr {
log::debug!("MPEG: CBR detected");
properties.audio_bitrate = first_frame_header.bitrate;
}
// Search for the last frame, starting at the end of the frames
reader.seek(SeekFrom::Start(last_frame_offset))?;
@ -179,7 +189,7 @@ where
let mut last_frame = None;
let mut pos = last_frame_offset;
while pos > 0 {
match rev_search_for_frame_header(reader, &mut pos, parse_mode) {
match rev_search_for_frame_header(reader, &mut pos) {
// Found a frame header
Ok(Some(header)) => {
// Move `last_frame_offset` back to the actual position
@ -197,14 +207,23 @@ where
}
}
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).div_round(u64::from(properties.audio_bitrate));
let Some(last_frame_header) = last_frame else {
log::warn!("MPEG: Could not find last frame, properties will be incomplete");
return Ok(());
};
if length > 0 {
properties.overall_bitrate = ((file_length * 8) / length) as u32;
properties.duration = Duration::from_millis(length);
}
let stream_len = (last_frame_offset + u64::from(last_frame_header.len)) - first_frame_offset;
if !is_cbr {
log::debug!("MPEG: VBR detected");
// TODO: Actually handle VBR streams, this still assumes CBR
properties.audio_bitrate = first_frame_header.bitrate;
}
let length = (stream_len * 8).div_round(u64::from(properties.audio_bitrate));
if length > 0 {
properties.overall_bitrate = ((file_length * 8) / length) as u32;
properties.duration = Duration::from_millis(length);
}
Ok(())

View file

@ -194,7 +194,6 @@ where
last_frame_offset,
xing_header,
file_length,
parse_options.parsing_mode,
)?;
}