Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/aac missing timeline #222

Merged
merged 6 commits into from
Mar 10, 2024
Merged

Bug/aac missing timeline #222

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7af4571
adts duration
Aug 27, 2023
8f057a8
adts: bit cleaner loop for estimating n frames
Mar 10, 2024
dd1a5e2
adts: only use ensure_seekback_buffer for estimation of duration when…
Mar 10, 2024
0cc3e6a
adts: parse less data to estimate duration
Mar 10, 2024
79a92e5
adts: moved call of approximate_frame_count
Mar 10, 2024
367c964
adts: minor fixes
pdeljanov Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion symphonia-codec-aac/src/adts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,20 @@ impl FormatReader for AdtsReader {
// Use the header to populate the codec parameters.
let mut params = CodecParameters::new();

params.for_codec(CODEC_TYPE_AAC).with_sample_rate(header.sample_rate);
params
.for_codec(CODEC_TYPE_AAC)
.with_sample_rate(header.sample_rate)
.with_time_base(TimeBase::new(1, header.sample_rate));

if let Some(channels) = header.channels {
params.with_channels(channels);
}

let n_frames = approximate_frame_count(&mut source)?;
if let Some(n_frames) = n_frames {
params.with_n_frames(n_frames);
}

// Rewind back to the start of the frame.
source.seek_buffered_rev(AdtsHeader::SIZE);

Expand Down Expand Up @@ -258,3 +266,72 @@ impl FormatReader for AdtsReader {
self.reader
}
}

fn approximate_frame_count(mut source: &mut MediaSourceStream) -> Result<Option<u64>> {
let original_pos = source.pos();
let total_len = match source.byte_len() {
Some(len) => len - original_pos,
_ => return Ok(None),
};

const ENSURED_SEEK_LEN: u64 = 1000;
source.ensure_seekback_buffer(ENSURED_SEEK_LEN as usize);
let mut scoped_stream = ScopedStream::new(&mut source, ENSURED_SEEK_LEN);

let mut parsed_n_frames = 0;
let mut n_bytes = 0;

loop {
let header = match AdtsHeader::read(&mut scoped_stream) {
Ok(header) => header,
_ => break,
};

if scoped_stream.ignore_bytes(header.frame_len as u64).is_err() {
break;
}

parsed_n_frames += 1;
n_bytes += header.frame_len + AdtsHeader::SIZE;
}
source.seek_buffered_rev((source.pos() - original_pos) as usize);
dedobbin marked this conversation as resolved.
Show resolved Hide resolved

let step = total_len / 3;
if source.is_seekable() {
let mut new_pos = total_len / 2;

loop {
if new_pos >= total_len {
break;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit strange to me. You are attempting to seek to and examine 3 location in the file (guessing because you're doing total_len / 3), but you start from the middle of the file (new_pos = total_len / 2) which means you are skipping half of the file.

Assuming you want to sample at points a, b, and c, shouldn't you calculate step = (total_len - original_pos) / 4, and start at new_pos = orignal_pos + step? Diagram below.

         a        b        c
|        |        |        |        |
^ original_pos                      ^ total_len

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop could also be made into a for loop:

for new_pos in (original_pos..total_len).step_by(step).skip(1) {

No need for mutable variables!


let res = source.seek(SeekFrom::Start(new_pos));
if res.is_err() {
break;
}

for _ in 0..=500 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

500 is a lot of frames! That's over 11 seconds of audio at 44.1 kHz. Can we do a magnitude less?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that seems on the extreme side. I was not sure what numbers to plug in, i lowered it now and it still gives acceptable output to me. Let me know if this works for you or how we should go about finding a sweet spot.

let header = match AdtsHeader::read(&mut source) {
Ok(header) => header,
_ => break,
};

if source.ignore_bytes(header.frame_len as u64).is_err() {
break;
}

parsed_n_frames += 1;
n_bytes += header.frame_len + AdtsHeader::SIZE;
}
new_pos += step;
}

let _ = source.seek(SeekFrom::Start(original_pos))?;
}
debug!("adts: Parsed {} of {} bytes to approximate duration", n_bytes, total_len);

match parsed_n_frames {
0 => Ok(None),
_ => Ok(Some(total_len / (n_bytes as u64 / parsed_n_frames) * SAMPLES_PER_AAC_PACKET)),
}
}