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

Opusenc: Pre-skip doesn't include extra_samples #55

Open
mattgwwalker opened this issue Jun 9, 2020 · 3 comments
Open

Opusenc: Pre-skip doesn't include extra_samples #55

mattgwwalker opened this issue Jun 9, 2020 · 3 comments

Comments

@mattgwwalker
Copy link
Contributor

On page 27 of "Ogg Encapsulation for the Opus Audio Codec" [1], the recommendations for encoders is to have a pre-skip value of (delay_samples + extra_samples). Extra_samples is defined there as at least 120 samples.

However, opusenc has a pre-skip of just delay_samples (see line 1008 of opusenc.c).

Why is this? Is this an oversight that should be corrected? Or, is the explanation in the RFC incorrect?

Cheers,

Matthew

[1] https://tools.ietf.org/html/rfc7845#page-27

@rillian
Copy link
Contributor

rillian commented Jun 9, 2020

As far as I can tell, you're correct here, and this hasn't been implemented.

The underlying libopusenc stream encoder implementation does lpc interpolation into the padding of the final packet, but I'm told it doesn't currently do pre-extrapolation into the pre-skip region at the beginning of a logical bitstream. To avoid shifting the start time by a non-integer number of samples the back-interpolation would need to happen after resampling, which means the libopus encoder could do this itself.

Perhaps it's not been a significant problem in practice because most tracks start with silence? Or at least a zero-crossing?

Another issue is that opusenc doesn't have a way to overlap audio data between adjacent tracks. This is supported by libopusenc where it will drop dependencies and copy audio across a chain boundary as the spec suggests, to improve performance with e.g. gapless compact disc track transitions. But since opusenc encodes one file at a time, there's no way for it to make use of this feature.

Maybe it would need an option to pass a .cue file (or read a CUESHEET block in flac input) or an option to encode a sequence of input files as a continuous set.

@rillian
Copy link
Contributor

rillian commented Jun 11, 2020

Corrections from Jean-Marc on IRC:

LPC at the beginning is tricky for the general case.

You can't do it after resampling because the resampling itself is going to glitch. And if you do it before resampling, you need to be really careful not to cause a fractional delay because then your entire stream is going to be off by a fraction of a sample, which is not only going to cause a glitch at the boundary, but is also more likely to cause clipping.

So you'd have to extrapolate out to some common-divisor of the two sample rates, attenuate toward silence, then resample and trim.

For 44.1 kHz, I'd have to extrapolate at least 441 samples. That or change the resampler to support "priming" samples at the beginning

@mattgwwalker
Copy link
Contributor Author

Sorry for my very delayed reply and thank you for such a detailed response.

I'm a very long way from being able to say that I "understand Opus", but would it not suffice to just create 120 samples of silence and consider those as the extra_samples? Or is the problem that I'm incorrectly assuming that the audio starts at a zero-crossing?

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