PR #24650 opened by Lynne URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24650 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24650.patch
@jamrial >From f1b3471b16b71b90e0fab9e00b8a33d2a9773445 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. --- 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 933bec5d1145118dc0a0f8c36285a390fd004bab 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. --- 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 79faade59a..8e942a293e 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 54da24ca66c4eaf75bc917bea4714c168ce5b0bc 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. --- 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 8e942a293e..6d402e6af9 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 f78cd37db0985f519f400e80a013fe3b3ee00719 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. --- 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 6d402e6af9..c451a5b552 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 8d3e71e1ca..6806ba3dda 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, S=1, Skip Samples, 10, 0x04b00078 -0, 840, 840, 168, 1, 0x00fc00fc, S=1, Skip Samples, 10, 0x009f001b +0, -120, -120, 960, 3, 0x05f002f9, S=1, Skip Samples, 10, 0x04b00078 +0, 840, 840, 168, 3, 0x05f002f9, S=1, Skip Samples, 10, 0x009f001b -- 2.52.0 >From 464df4fdf050d4652232b25fc100b21264d51c63 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 --- 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]
