PR #24686 opened by James Almer (jamrial) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24686 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24686.patch
Backport of !24650 >From 82a4f22c8333ddc63e141b7d8b89a8e3e279b960 Mon Sep 17 00:00:00 2001 From: Lynne <[email protected]> Date: Wed, 23 Sep 2026 20:27:15 +0900 Subject: [PATCH 1/5] avcodec/opus/rc: zero the unused bytes of an encoded frame When the range coded data and the raw bits of a frame did not meet, the bytes between them were copied from the stale contents of the range coder buffer, and when there were no raw bits, the bytes after the range coded data were left as whatever the packet buffer held. Zero them instead, like libopus does. (cherry picked from commit 574c34228cb1bb8e4aaec3f2d9ffaa90aad2e06f) --- libavcodec/opus/rc.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libavcodec/opus/rc.c b/libavcodec/opus/rc.c index 8e58a52b85..57ac272934 100644 --- a/libavcodec/opus/rc.c +++ b/libavcodec/opus/rc.c @@ -393,9 +393,17 @@ void ff_opus_rc_enc_end(OpusRangeCoder *rc, uint8_t *dst, int size) rb_src = rc->buf + OPUS_MAX_FRAME_SIZE + 12 - rc->rb.bytes; rb_dst = dst + FFMAX(size - rc->rb.bytes, 0); lap = &dst[rng_bytes] - rb_dst; + if (lap < 0) { + /* Zero the unused bytes between the range coded and the raw bits + * parts instead of copying whatever is in the buffer there */ + memset(&dst[rng_bytes], 0, -lap); + lap = 0; + } for (i = 0; i < lap; i++) rb_dst[i] |= rb_src[i]; memcpy(&rb_dst[lap], &rb_src[lap], FFMAX(rc->rb.bytes - lap, 0)); + } else if (rng_bytes < size) { + memset(&dst[rng_bytes], 0, size - rng_bytes); } } -- 2.52.0 >From 4076c581d66876dcb4639800504edaa312fa8d77 Mon Sep 17 00:00:00 2001 From: Lynne <[email protected]> Date: Wed, 23 Sep 2026 20:27:25 +0900 Subject: [PATCH 2/5] avcodec/opus/enc: fix the input setup of frames shorter than frame_size The overlap of a frame is the tail of the previous frame as it was encoded, which was zero padded if the input frame was shorter than frame_size. The last CELT_OVERLAP input samples were copied instead, so when the last input frame was short and a frame followed it to flush its overlap, the end of the stream was distorted. The padding of a short input frame was also written at the start of the sample buffer instead of after the input frame's samples. (cherry picked from commit 7e1bc799b2c36add077e1077b2eba8e2678294ff) --- libavcodec/opus/enc.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/libavcodec/opus/enc.c b/libavcodec/opus/enc.c index 74531f9142..39cab0dc04 100644 --- a/libavcodec/opus/enc.c +++ b/libavcodec/opus/enc.c @@ -131,20 +131,17 @@ static void celt_frame_setup_input(OpusEncContext *s, CeltFrame *f) for (int ch = 0; ch < f->channels; ch++) { CeltBlock *b = &f->block[ch]; - const char *input = cur->extended_data[ch]; - size_t bps = av_get_bytes_per_sample(cur->format); + const float *input = (const float *)cur->extended_data[ch]; /* The MDCT overlap is the trailing CELT_OVERLAP samples of the - * previous packet's last frame. Because the encoder advertises - * AV_CODEC_CAP_SMALL_LAST_FRAME, that frame can be shorter than - * CELT_OVERLAP; in that case, zero-pad the leading part of the - * overlap buffer and copy only what's available. */ - int n = FFMIN(cur->nb_samples, CELT_OVERLAP); - if (n < CELT_OVERLAP) { - memset(b->overlap, 0, (CELT_OVERLAP - n) * bps); - } - memcpy((char *)b->overlap + (CELT_OVERLAP - n) * bps, - input + (cur->nb_samples - n) * bps, - n * bps); + * previous packet's last frame, as they were when that frame was + * encoded. Because the encoder advertises AV_CODEC_CAP_SMALL_LAST_FRAME, + * that frame may have been shorter than frame_size, in which case it + * was zero padded, so the overlap has to be zero padded the same way. */ + const int start = subframesize - CELT_OVERLAP; + const int n = av_clip(cur->nb_samples - start, 0, CELT_OVERLAP); + if (n > 0) + memcpy(b->overlap, input + start, n * sizeof(float)); + memset(b->overlap + n, 0, (CELT_OVERLAP - n) * sizeof(float)); } av_frame_free(&cur); @@ -157,12 +154,11 @@ static void celt_frame_setup_input(OpusEncContext *s, CeltFrame *f) for (int ch = 0; ch < f->channels; ch++) { CeltBlock *b = &f->block[ch]; - const void *input = cur->extended_data[ch]; - const size_t bps = av_get_bytes_per_sample(cur->format); - const size_t left = (subframesize - cur->nb_samples)*bps; - const size_t len = FFMIN(subframesize, cur->nb_samples)*bps; - memcpy(&b->samples[sf*subframesize], input, len); - memset(&b->samples[cur->nb_samples], 0, left); + const float *input = (const float *)cur->extended_data[ch]; + float *dst = &b->samples[sf * subframesize]; + const int n = FFMIN(cur->nb_samples, subframesize); + memcpy(dst, input, n * sizeof(float)); + memset(dst + n, 0, (subframesize - n) * sizeof(float)); } /* Last frame isn't popped off and freed yet - we need it for overlap */ -- 2.52.0 >From a0980502d0329c429f6e794b423aa4a099d662f6 Mon Sep 17 00:00:00 2001 From: Lynne <[email protected]> Date: Wed, 23 Sep 2026 20:27:35 +0900 Subject: [PATCH 3/5] avcodec/opus/enc: fix the pre-emphasis filter state between frames The trailing CELT_OVERLAP samples of a frame are filtered again as the overlap of the next frame, so the filter state saved for the next frame has to be the one from right before them. The code relied on packets being assembled from 120 sample input frames for this, which is no longer the case since c3aea7628c, and saved the state from the end of the previous frame instead. The first overlap sample of every frame was then filtered differently than when it was encoded as part of the previous frame. (cherry picked from commit 49975b25a0c7986b3b10eef209018e097cb430f3) --- libavcodec/opus/enc.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/libavcodec/opus/enc.c b/libavcodec/opus/enc.c index 39cab0dc04..bd48960c5e 100644 --- a/libavcodec/opus/enc.c +++ b/libavcodec/opus/enc.c @@ -170,34 +170,29 @@ static void celt_frame_setup_input(OpusEncContext *s, CeltFrame *f) /* Apply the pre emphasis filter */ static void celt_apply_preemph_filter(OpusEncContext *s, CeltFrame *f) { - const int subframesize = s->avctx->frame_size; - const int subframes = OPUS_BLOCK_SIZE(s->packet.framesize) / subframesize; + const int frame_len = OPUS_BLOCK_SIZE(s->packet.framesize); const float c = ff_opus_deemph_weights[0]; - /* Filter overlap */ for (int ch = 0; ch < f->channels; ch++) { CeltBlock *b = &f->block[ch]; float m = b->emph_coeff; + + /* Filter the overlap (the trailing CELT_OVERLAP samples of the previous frame) */ for (int i = 0; i < CELT_OVERLAP; i++) { float sample = b->overlap[i]; b->overlap[i] = sample - m; m = sample * c; } - b->emph_coeff = m; - } - /* Filter the samples but do not update the last subframe's coeff - overlap ^^^ */ - for (int sf = 0; sf < subframes; sf++) { - for (int ch = 0; ch < f->channels; ch++) { - CeltBlock *b = &f->block[ch]; - float m = b->emph_coeff; - for (int i = 0; i < subframesize; i++) { - float sample = b->samples[sf*subframesize + i]; - b->samples[sf*subframesize + i] = sample - m; - m = sample * c; - } - if (sf != (subframes - 1)) + /* Filter the samples. The trailing CELT_OVERLAP samples are filtered + * again as the next frame's overlap, so the filter state saved for + * the next frame is the one from right before them. */ + for (int i = 0; i < frame_len; i++) { + float sample = b->samples[i]; + if (i == frame_len - CELT_OVERLAP) b->emph_coeff = m; + b->samples[i] = sample - m; + m = sample * c; } } } -- 2.52.0 >From 5ee24ba241f71e05c316291d8b675c5ec4affe81 Mon Sep 17 00:00:00 2001 From: Lynne <[email protected]> Date: Wed, 23 Sep 2026 20:27:47 +0900 Subject: [PATCH 4/5] avcodec/opus/enc: signal silence explicitly Silent frames were coded with zero bytes, which decoders take as a lost frame and conceal by extrapolating the preceding audio, rather than outputting silence. A packet of such frames is also only one or two bytes long, and the MPEG-TS muxer rejects packets shorter than two. Code the silence flag in a two byte frame like libopus does, and keep the encoder state in sync with the decoder after it: the decoder predicts the band energies of the next frame from CELT_ENERGY_SILENCE, and the pre-emphasis filter state at the start of the next overlap is zero. As silent frames no longer have a size of zero, leave them out of the rate control explicitly. (cherry picked from commit b726a3b8c48f0945a9fbee9a308e7b22b183db5c) --- libavcodec/opus/enc.c | 11 +++++++++-- libavcodec/opus/enc_psy.c | 7 +++++-- tests/ref/fate/opus-enc-silence | 4 ++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/libavcodec/opus/enc.c b/libavcodec/opus/enc.c index bd48960c5e..e63293b63e 100644 --- a/libavcodec/opus/enc.c +++ b/libavcodec/opus/enc.c @@ -437,8 +437,15 @@ static void celt_encode_frame(OpusEncContext *s, OpusRangeCoder *rc, if (f->silence) { if (f->framebits >= 16) ff_opus_rc_enc_log(rc, 1, 15); /* Silence (if using explicit signalling) */ - for (int ch = 0; ch < s->channels; ch++) - memset(s->last_quantized_energy[ch], 0.0f, sizeof(float)*CELT_MAX_BANDS); + for (int ch = 0; ch < s->channels; ch++) { + /* The decoder sets all band energies to CELT_ENERGY_SILENCE on + * a silence frame, and predicts the next frame's from them. */ + for (int i = 0; i < CELT_MAX_BANDS; i++) + s->last_quantized_energy[ch][i] = CELT_ENERGY_SILENCE; + /* The frame is all zeros, so this is the pre emphasis filter state + * at the point the next frame's overlap starts */ + f->block[ch].emph_coeff = 0.0f; + } return; } diff --git a/libavcodec/opus/enc_psy.c b/libavcodec/opus/enc_psy.c index a7a1f1ff8e..7b44978f19 100644 --- a/libavcodec/opus/enc_psy.c +++ b/libavcodec/opus/enc_psy.c @@ -311,7 +311,10 @@ void ff_opus_psy_celt_frame_init(OpusPsyContext *s, CeltFrame *f, int index) f->silence = silence; if (f->silence) { - f->framebits = 0; /* Otherwise the silence flag eats up 16(!) bits */ + /* Explicitly signal silence in a 2 byte frame, like libopus does. + * A zero length frame would instead signal a lost frame to decoders, + * which conceal it by extrapolating the preceding audio. */ + f->framebits = 16; f->intensity_stereo = f->end_band; /* Read by postencode_update for avg_is_band */ return; } @@ -546,7 +549,7 @@ void ff_opus_psy_postencode_update(OpusPsyContext *s, CeltFrame *f) for (i = 0; i < s->p.frames; i++) { s->avg_is_band += f[i].intensity_stereo; - if (f[i].framebits > 0) + if (!f[i].silence && f[i].framebits > 0) s->lambda *= ideal_fbits / f[i].framebits; } diff --git a/tests/ref/fate/opus-enc-silence b/tests/ref/fate/opus-enc-silence index dd8e02e02d..37f039f1f4 100644 --- a/tests/ref/fate/opus-enc-silence +++ b/tests/ref/fate/opus-enc-silence @@ -4,5 +4,5 @@ #codec_id 0: opus #sample_rate 0: 48000 #channel_layout_name 0: stereo -0, -120, -120, 960, 1, 0x00fc00fc -0, 840, 840, 168, 1, 0x00fc00fc, S=1, Skip Samples, 10, 0x009f001b +0, -120, -120, 960, 3, 0x05f002f9 +0, 840, 840, 168, 3, 0x05f002f9, S=1, Skip Samples, 10, 0x009f001b -- 2.52.0 >From b135b25c19b56f909b1d529dedfffbb14ddf170d Mon Sep 17 00:00:00 2001 From: Lynne <[email protected]> Date: Wed, 23 Sep 2026 20:28:01 +0900 Subject: [PATCH 5/5] avcodec/opus/enc_psy: don't make the last frame silent at EOF 816d74e0bb made frames without psy analysis at EOF silent. The frame that follows the last input frame to flush its MDCT overlap is always such a frame, so the last CELT_OVERLAP samples of the stream were lost whenever it was needed, i.e. when the length of the stream is a multiple of frame_size or less than CELT_OVERLAP samples short of one. It was also coded as a one byte packet, which the MPEG-TS muxer rejects. Encode it like any other frame instead. Its psy analysis data is zeroed, which leaves it with the default frame parameters. Fixes: #24196 (cherry picked from commit 2e0a770b8e63911cdbca40dcbd3ba279f99d23f5) --- libavcodec/opus/enc_psy.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/libavcodec/opus/enc_psy.c b/libavcodec/opus/enc_psy.c index 7b44978f19..6dda22e7da 100644 --- a/libavcodec/opus/enc_psy.c +++ b/libavcodec/opus/enc_psy.c @@ -300,15 +300,6 @@ void ff_opus_psy_celt_frame_init(OpusPsyContext *s, CeltFrame *f, int index) for (i = 0; i < steps_per_frame; i++) silence &= s->steps[index * steps_per_frame + i]->silence; - /* If we reach EOF with fewer collected psy steps than this packet wants - * to encode, the slots beyond buffered_steps were zeroed by the previous - * postencode_update and contain no valid analysis data. Encoding garbage - * with the full rate budget can overrun the range coder buffer (rng_bytes - * exceeds the per-frame size passed to ff_opus_rc_enc_end), so force a - * silent packet instead. */ - if (s->eof && step_offset >= s->buffered_steps) - silence = 1; - f->silence = silence; if (f->silence) { /* Explicitly signal silence in a 2 byte frame, like libopus does. @@ -556,7 +547,9 @@ void ff_opus_psy_postencode_update(OpusPsyContext *s, CeltFrame *f) s->avg_is_band /= (s->p.frames + 1); s->steps_to_process = 0; - s->buffered_steps -= steps_out; + /* At EOF, one more frame than there are psy steps is encoded to flush + * the last frame's MDCT overlap */ + s->buffered_steps = FFMAX(s->buffered_steps - steps_out, 0); s->total_packets_out += s->p.frames; s->inflection_points_count = 0; } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
