diff mbox series

[FFmpeg-devel,4/8] avformat/matroskaenc: Don't override samplerate for CodecDelay

Message ID GV1P250MB07376EA844454584A31FD1878F7B9@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit be0a2515ab25e4531464f84c4c4d5a97e1a3e8ef
Headers show
Series [FFmpeg-devel,1/8] fftools/ffprobe: Report initial and trailing padding | expand


Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 1, 2022, 9:23 p.m. UTC
Opus can be decoded to multiple samplerates (namely 48kHz, 24KHz,
16Khz, 12 KHz and 8Khz); libopus as well as our encoder wrapper
support these sample rates. The OpusHead contains a field for
this original samplerate. Yet the pre-skip (and the granule-position
in the Ogg-Opus mapping in general) are always in the 48KHz clock,
irrespective of the original sample rate.

Before commit c3c22bee6362737cf290929b7f31df9fb88da983, our libopus
encoder was buggy: It did not account for the fact that the pre-skip
field is always according to a 48kHz clock and wrote a too small
value in case one uses the encoder with a sample rate other than 48kHz;
this discrepancy between CodecDelay and OpusHead led to Firefox
rejecting such streams.

In order to account for that, said commit made the muxer always use
48kHz instead of the actual sample rate to convert the initial_padding
(in samples in the stream's sample rate) to ns. This meant that both
fields are now off by the same factor, so Firefox was happy.

Then commit f4bdeddc3cab807e43e0450744dfe9a45661e1d7 fixed the issue
in libopusenc; so the OpusHead is correct, but the CodecDelay is
still off*. This commit fixes this by effectively reverting

*: Firefox seems to no longer abort when CodecDelay and OpusHead
are off.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Non-48kHz Opus is also weird in the demuxer: avformat_find_stream_info()
sets the samplerate to 48kHz. Therefore 49b0246635 calculated
initial_padding based upon 48kHz for Opus, regardless of the actual
sample rate indicated in the TrackEntry. This is wrong if
avformat_find_stream_info() is not used. I think a better (but still
hacky) solution would be to just report the sample rate as 48kHz
for Opus regardless of what the header says.

(Trying to pass initial padding through the decoder is problematic:
The corresponding field of AVCodecContext is unused for decoding,
instead there is a delay field which is always set by libavcodec
whose semantics are unclear (there are two inconsistent definitions:
"Number of frames delay in addition to what a standard decoder
as specified in the spec would produce." and "the number of samples
the decoder needs to output before the decoder's output is valid".
The Opus decoders and parser are the only components that set delay
at all (based upon OpusHead). When converting
AVCodecParameters->AVCodecContext, initial_padding is mapped to both
delay and initial_padding; in the other direction, initial_padding
is taken from initial_padding (which is encoder-only). If one wants
to pass this value through the decoder, then the best way to do so
is probably to use initial_padding for decoding, too, and let the
user set it and let lavc update it if the decoder encounters
information about this in the bitstream/extradata. This includes
that lavc updates initial_padding if it changes the sample rate
(as it does for Opus). This would also fix delay (the second definition
would be deprecated).)

 libavformat/matroskaenc.c | 2 +-
 libavformat/version.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff mbox series


diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index de6c993e6a..c525edb39f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1830,7 +1830,7 @@  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
         if (par->initial_padding && par->codec_id == AV_CODEC_ID_OPUS) {
             int64_t codecdelay = av_rescale_q(par->initial_padding,
-                                              (AVRational){ 1, 48000 },
+                                              (AVRational){ 1, par->sample_rate },
                                               (AVRational){ 1, 1000000000 });
             if (codecdelay < 0) {
                 av_log(s, AV_LOG_ERROR, "Initial padding is invalid\n");
diff --git a/libavformat/version.h b/libavformat/version.h
index 7b414039ad..a54ffd6c0e 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 #include "version_major.h"
                                                LIBAVFORMAT_VERSION_MINOR, \