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.

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

"Could not update timestamps for skipped samples." warning on AAC file #30

Closed
dceddia opened this issue Sep 5, 2021 · 3 comments
Closed

Comments

@dceddia
Copy link
Contributor

dceddia commented Sep 5, 2021

Hey, first off, amazing work on this crate! It's really nice to use and feels more Rust-y than the others I've tried :)

I'm putting together a video editing app and working on the basics right now, reading audio + video frames. I'm getting this warning while reading packets from an AAC (m4a) audio-only file:

[aac @ 0x7fd3bc009400] Could not update timestamps for skipped samples.

I think this has something to do with AAC's "priming samples", where there are 2112 samples at the start that get skipped.

Here's a minimal example repo (includes a test audio file recorded with QuickTime). It's pretty much the decoding.rs example but with "video" replaced by "audio".

I tracked it down to this block of code in ffmpeg's libavcodec/decode.c:

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/decode.c#L395-L407

if(avctx->pkt_timebase.num && avctx->sample_rate) {
    int64_t diff_ts = av_rescale_q(avci->skip_samples,
                                   (AVRational){1, avctx->sample_rate},
                                   avctx->pkt_timebase);
    if(frame->pts!=AV_NOPTS_VALUE)
        frame->pts += diff_ts;
    if(frame->pkt_dts!=AV_NOPTS_VALUE)
        frame->pkt_dts += diff_ts;
    if (frame->pkt_duration >= diff_ts)
        frame->pkt_duration -= diff_ts;
} else {
    av_log(avctx, AV_LOG_WARNING, "Could not update timestamps for skipped samples.\n");
}

It looks like this code is trying to increment the PTS of the packet by however many samples were skipped, and it only logs the warning if the AVCodecContext's pkt_timebase.num or sample_rate are 0.

I threw a print into ffw_decoder_push_packet and it shows a pkt_timebase of 0/1 and sample_rate of 48000.

Docs for pkt_timebase say it's "set by the user" for decoding, so I guess I need to set this somewhere.

I'm not sure where's the right place to set it, though. I thought of setting it in ffw_decoder_push_packet if it's not set yet, but packets don't seem to know their timebase, so I guess it needs to come from the stream. It does seem like this should ideally be set during initialization somehow. Maybe the Decoder could take a stream instead of codec_parameters?

@operutka
Copy link
Member

operutka commented Sep 6, 2021

Hi @dceddia. Thank you. It is certainly the main point of this library to make the FFmpeg interface more Rust friendly.

Regarding your issue - you can set pkt_timebase (along with many other options) using the AudioDecoderBuilder::set_option() method. The initialization would look like:

let num = ...;
let den = ...;

let mut decoder = AudioDecoder::from_codec_parameters(params)?
    .set_option("pkt_timebase", format!("{}/{}", num, den))
    .build()?;

This library attempts to deal with timebase-unaware AVPackets by adding timebase to each Packet. There is also a method named time_base() on the AudioDecoderBuilder that allows rescaling packet timestamps into a given timebase. However, this method does not set the pkt_timebase field on the codec context. I can update the AudioDecoderBuilder in a way that the pkt_timebase is always set according to the timebase of the builder. This way you wouldn't have to use the set_option() method in future versions.

I will also consider adding extra methods for constructing audio/video decoders from Stream instances. It's actually a good idea.

@operutka
Copy link
Member

operutka commented Sep 6, 2021

d9ee10b should fix the issue.

@dceddia
Copy link
Contributor Author

dceddia commented Sep 6, 2021

Awesome, thanks @operutka! That solves it :) I've got my audio + video frames reading out and pairing together nicely now!

I ended up implementing Debug for TimeBase to make it easier to print, I'll put up a PR in case that's useful.

@dceddia dceddia closed this as completed Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants