PR #25688 opened by Guillain-RDCDE URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25688 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25688.patch
These ten commits bring the G.723.1 decoder closer to the ITU-T reference decoder. They fix concealment after lost frames (Annex A when the first SID frame is lost, the excitation history after unvoiced concealment, the third lost frame, the state the decoder starts in). They saturate the LPC negation, the synthesis filter and the formant postfilter the way the reference does, round the reflection coefficient like the reference, and track bad frames per channel. A flush callback lets the decoder be reset. The existing g723_1-dec-1 to -8 references do not change. Six new tests, g723_1-dec-9 to -14, cover the fixed paths. They need three new files in fate-suite. They are Patrick's own test vectors, in `samples/g723_1/` of https://github.com/patrickdk77/FFmpeg/releases/download/n9.0.2d/fates.tar.gz : ``` g723_1/cng-erasures.tco 1377 bea9cee96038cd896345f5b7e9776f67 g723_1/lost-start.tco 138 20d4657c2010bddb8d08955b381767b5 g723_1/saturation.tco 961 d1b4d1672014a5232dcc2cfb447e804d ``` Every commit builds, and the g723_1 tests and fate-source pass at each one. The work is Patrick Domack's. He asked for it to be submitted, so I rebased it on current master and I will follow up on the review. >From 2f7442318841fbb815449faa19927bb8fa308593 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:37 -0500 Subject: [PATCH 01/10] avcodec/g723_1dec: add a flush callback The decoder had no flush callback, so after a seek it kept the LSP, excitation, filter, postfilter and comfort noise state from before the seek. Move the state setup from init into a reset. It first clears the channel contexts, which returns them to the state a new decoder starts with. Call it from init and as the flush callback. No FATE reference changes. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index 67a5df55ff0..8406655e44f 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -112,16 +112,11 @@ static const int cng_filt[4] = { 273, 998, 499, 333 }; static const int cng_bseg[3] = { 2048, 18432, 231233 }; -static av_cold int g723_1_decode_init(AVCodecContext *avctx) +static void g723_1_reset(AVCodecContext *avctx) { G723_1_Context *s = avctx->priv_data; - avctx->sample_fmt = AV_SAMPLE_FMT_S16P; - if (avctx->ch_layout.nb_channels < 1 || avctx->ch_layout.nb_channels > 2) { - av_log(avctx, AV_LOG_ERROR, "Only mono and stereo are supported (requested channels: %d).\n", - avctx->ch_layout.nb_channels); - return AVERROR(EINVAL); - } + memset(s->ch, 0, sizeof(s->ch)); for (int ch = 0; ch < avctx->ch_layout.nb_channels; ch++) { G723_1_ChannelContext *p = &s->ch[ch]; @@ -133,6 +128,18 @@ static av_cold int g723_1_decode_init(AVCodecContext *avctx) p->cng_random_seed = CNG_RANDOM_SEED; p->past_frame_type = SID_FRAME; } +} + +static av_cold int g723_1_decode_init(AVCodecContext *avctx) +{ + avctx->sample_fmt = AV_SAMPLE_FMT_S16P; + if (avctx->ch_layout.nb_channels < 1 || avctx->ch_layout.nb_channels > 2) { + av_log(avctx, AV_LOG_ERROR, "Only mono and stereo are supported (requested channels: %d).\n", + avctx->ch_layout.nb_channels); + return AVERROR(EINVAL); + } + + g723_1_reset(avctx); return 0; } @@ -1118,6 +1125,7 @@ const FFCodec ff_g723_1_decoder = { .priv_data_size = sizeof(G723_1_Context), .init = g723_1_decode_init, FF_CODEC_DECODE_CB(g723_1_decode_frame), + .flush = g723_1_reset, .p.capabilities = AV_CODEC_CAP_DR1, .p.priv_class = &g723_1dec_class, }; -- 2.54.0 >From 39662e153f75945e42d6e1ee78096567f21dbb65 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:38 -0500 Subject: [PATCH 02/10] avcodec/g723_1dec: follow Annex A when the first SID frame is lost An untransmitted frame right after an active frame means the SID frame that should have started the silence was lost. G.723.1 Annex A then rebuilds the comfort noise level from the energy of the last good active frame. FFmpeg's decoder has handled this case differently from the ITU reference decoder since comfort noise was added in 04fc5c6bde (2012), and n9.0.2 and master 81cf906e75 still do. estimate_sid_gain() quantizes that energy to a 6-bit SID gain index. It departs from the reference in three places: - It multiplies by 273 and shifts by 16. The reference uses mul32x16 with 273, a shift by 15, so FFmpeg compares half the energy with the segment bounds. - In segments 0 and 1 it steps by 1 and 2 instead of 2 and 4, and its index comes out 16 too low. - Its final rounding compares the wrong distances, so it always rounds up when the energy lies above the candidate. The caller then stores the index as the gain. The reference decodes it, as the SID frame path already does with sid_gain_to_lsp_index(). The reference also takes the comfort noise LSPs from the last good active frame. FFmpeg keeps sid_lsp from the last SID frame, or the initial DC vector when there was none. Quantize with MULL2(), which gives the reference's mul32x16 result here because nothing in this calculation saturates. Step through the three segments by 2, 4 and 8 with their index offsets 0, 16 and 32, round to the nearer index and decode it. Copy prev_lsp into sid_lsp at the end of every good active frame. The reference used here is the G.723.1 decoder of DS2-Anywhere, vendor/dss-codec/src/codec/g723_1.rs in https://github.com/Guillain-RDCDE/DS2-Anywhere. It matches the ITU reference decoder on the 11 ITU test vectors and on 17 more cases decoded with the ITU reference. One of those, a stream of random bits, has 283 first-SID-lost frames. Sections 4.4.4, 4.10.2, 4.10.4 and 4.10.5 of its docs/g723-1/SPEC.md describe each step. Its g723-mix.g723pk fixture holds 900 packets of every frame type. Before this change, 520 of its frames differed from the reference with the postfilter on and 529 with it off. Now none do. Over 35 random streams of 3000 frames each, the frames that differ drop from 100308 to 10169 with the postfilter on and from 99885 to 9656 with it off. The following commits fix the rest. Frame 16 of cng-erasures.tco, a stream that a later commit adds to FATE, is an untransmitted frame after a 6.3 kbit/s frame. With the postfilter on and every G.723.1 fix but this one, its first sample is -3881 where the reference gives -3458, the frame differs by up to 7448, and 79 of the 97 frames differ. fate-g723_1-dec-1 to -8 pass unchanged. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index 8406655e44f..2b433e4552a 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -761,7 +761,7 @@ static int estimate_sid_gain(G723_1_ChannelContext *p) t = (p->sid_gain < 0) ? -1 : 0; }else t = p->sid_gain >> -shift; - x = av_clipl_int32(t * (int64_t)cng_filt[0] >> 16); + x = MULL2(t, cng_filt[0]); if (x >= cng_bseg[2]) return 0x3F; @@ -773,14 +773,14 @@ static int estimate_sid_gain(G723_1_ChannelContext *p) shift = 3; seg = (x >= cng_bseg[0]); } - seg2 = FFMIN(seg, 3); + seg2 = FFMIN(seg + 1, 3); val = 1 << shift; val_add = val >> 1; for (i = 0; i < shift; i++) { t = seg * 32 + (val << seg2); t *= t; - if (x >= t) + if (x >= 2 * t) val += val_add; else val -= val_add; @@ -788,18 +788,18 @@ static int estimate_sid_gain(G723_1_ChannelContext *p) } t = seg * 32 + (val << seg2); - y = t * t - x; + y = 2 * t * t - x; if (y <= 0) { t = seg * 32 + (val + 1 << seg2); - t = t * t - x; + t = x - 2 * t * t; val = (seg2 - 1) * 16 + val; - if (t >= y) + if (y <= t) val++; } else { t = seg * 32 + (val - 1 << seg2); - t = t * t - x; + t = x - 2 * t * t; val = (seg2 - 1) * 16 + val; - if (t >= y) + if (y >= t) val--; } @@ -1037,6 +1037,7 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, /* Save the excitation for the next frame */ memcpy(p->prev_excitation, p->excitation + FRAME_LEN, PITCH_MAX * sizeof(*p->excitation)); + memcpy(p->sid_lsp, p->prev_lsp, LPC_ORDER * sizeof(*p->sid_lsp)); } else { p->interp_gain = (p->interp_gain * 3 + 2) >> 2; if (p->erased_frames == 3) { @@ -1065,7 +1066,7 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, p->sid_gain = sid_gain_to_lsp_index(p->subframe[0].amp_index); ff_g723_1_inverse_quant(p->sid_lsp, p->prev_lsp, p->lsp_index, 0); } else if (p->past_frame_type == ACTIVE_FRAME) { - p->sid_gain = estimate_sid_gain(p); + p->sid_gain = sid_gain_to_lsp_index(estimate_sid_gain(p)); } if (p->past_frame_type == ACTIVE_FRAME) -- 2.54.0 >From deeae00c5922e476c142f75db41b6f4e4c99e392 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:39 -0500 Subject: [PATCH 03/10] avcodec/g723_1dec: clear excitation history after unvoiced concealment When an active frame is lost and the last good frame was unvoiced, the decoder fills the frame with scaled noise. The ITU reference then clears its whole excitation buffer, so the next frame's adaptive codebook starts from silence (DS2-Anywhere SPEC.md section 4.9). residual_interp() still clears p->excitation, but since 4b728b4712 (2012) the caller copies prev_excitation from the output buffer, which holds the noise. Before that commit it copied from p->excitation. So every later frame that reads the adaptive codebook, active or comfort noise, starts from that noise. Clear prev_excitation after unvoiced concealment, and keep the copy for voiced concealment. Over the 35 random streams, the frames that differ from the reference drop from 10169 to 471 with the postfilter on and from 9656 to 214 with it off. The streams that differ drop from 35 to 14. Frame 55 of cng-erasures.tco, a stream that a later commit adds to FATE, is a SID frame after a lost 5.3 kbit/s frame that was concealed as unvoiced. With the postfilter on and every G.723.1 fix but this one, its second sample is 8 where the reference gives 20, the frame differs by up to 345, and 16 frames differ. fate-g723_1-dec-1 to -8 pass unchanged. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index 2b433e4552a..eaf31939ea4 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -1056,8 +1056,12 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, p->interp_gain, &p->random_seed); /* Save the excitation for the next frame */ - memcpy(p->prev_excitation, buf + (FRAME_LEN - PITCH_MAX), - PITCH_MAX * sizeof(*p->excitation)); + if (p->interp_index) + memcpy(p->prev_excitation, buf + (FRAME_LEN - PITCH_MAX), + PITCH_MAX * sizeof(*p->excitation)); + else + memset(p->prev_excitation, 0, + PITCH_MAX * sizeof(*p->excitation)); } } p->cng_random_seed = CNG_RANDOM_SEED; -- 2.54.0 >From 4eb615c71da63da95bc6c964f3012f592d7246d2 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:39 -0500 Subject: [PATCH 04/10] avcodec/g723_1dec: zero the synthesis input on the third lost frame On the third lost active frame in a row the ITU reference sets the excitation to zero, so the synthesis filter only rings out its memory (DS2-Anywhere SPEC.md section 4.9). FFmpeg clears frame->data[0] instead. That is the output buffer, which the postfilter or the scaling for the unfiltered output overwrites later anyway. The synthesis filter reads p->audio, which still holds the previous frame. The memset also writes FRAME_LEN + LPC_ORDER samples into a plane of FRAME_LEN. That stays in bounds only because the default allocator pads each plane, and a get_buffer2() callback that allocates exactly nb_samples gets 20 bytes written past its plane. It only clears channel 0 too. FFmpeg's first version of the decoder cleared the buffer it synthesized from. 70bcdfb39f (2012), which took Libav's version of this code, moved the memset to the output frame. Clear p->audio + LPC_ORDER, the synthesis input, instead. Over the 35 random streams, the frames that differ from the reference drop from 471 to 378 with the postfilter on and from 214 to 190 with it off. The streams that differ drop from 14 to 7. Frame 71 of cng-erasures.tco, a stream that a later commit adds to FATE, is the third lost active frame in a row. With the postfilter on and every G.723.1 fix but this one, its first sample is -1812 where the reference gives -447, and the frame differs by up to 4436. fate-g723_1-dec-1 to -8 pass unchanged. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index eaf31939ea4..ad335dc8db1 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -1046,8 +1046,8 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, (FRAME_LEN + PITCH_MAX) * sizeof(*p->excitation)); memset(p->prev_excitation, 0, PITCH_MAX * sizeof(*p->excitation)); - memset(frame->data[0], 0, - (FRAME_LEN + LPC_ORDER) * sizeof(int16_t)); + memset(p->audio + LPC_ORDER, 0, + FRAME_LEN * sizeof(*p->audio)); } else { int16_t *buf = p->audio + LPC_ORDER; -- 2.54.0 >From 94960c8b34cf196adcae7388b440f6ed14618c66 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:40 -0500 Subject: [PATCH 05/10] avcodec/g723_1dec: start as if the previous frame was active The ITU reference starts with the previous frame type set to active (DS2-Anywhere SPEC.md section 3). FFmpeg starts with SID_FRAME, as it has since 04fc5c6bde (2012). That changes the output when a stream starts with something other than a good active frame: - A SID frame: FFmpeg smooths its gain up from 0, (7 * 0 + gain) >> 3, so the comfort noise starts at an eighth of its level. The reference uses the gain as it is. - A lost or invalid frame: both decode it to silence, but the reference conceals it as an active frame and FFmpeg treats it as untransmitted. A SID frame after it then gets the full gain in the reference and the smoothed one in FFmpeg. FFmpeg's comfort noise generator has also drawn 30 random numbers more by then, because the reference resets its seed for the concealed frame. A stream that starts with an untransmitted frame decodes the same either way. Start with past_frame_type set to ACTIVE_FRAME, in g723_1_reset(), which init and the flush callback call. Over the 35 random streams, the frames that differ from the reference drop from 378 to 5 with the postfilter on and from 190 to 5 with it off. The streams that differ drop from 7 to 1. Frame 0 of cng-erasures.tco, a stream that a later commit adds to FATE, is a SID frame. With the postfilter on and every G.723.1 fix but this one, sample 9 is 82 where the reference gives 663, and 17 frames differ. lost-start.tco holds 17 frames in 138 bytes: two invalid 6.3 kbit/s frames, a SID frame, untransmitted frames, three valid 6.3 kbit/s frames, and untransmitted frames around a second SID frame. With this commit left out, frames 0 and 1 still match the reference, and frames 2 to 16 differ, with the postfilter on and off. fate-g723_1-dec-13 decodes lost-start.tco with the postfilter on, and fate-g723_1-dec-14 with it off. Their refs match the DS2-Anywhere G.723.1 decoder frame for frame. Both tests fail with this fix or the Annex A fix left out. fate-g723_1-dec-1 to -8 pass unchanged. Upload the sample to fate-suite as g723_1/lost-start.tco. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 2 +- tests/fate/voice.mak | 6 ++++++ tests/ref/fate/g723_1-dec-13 | 22 ++++++++++++++++++++++ tests/ref/fate/g723_1-dec-14 | 22 ++++++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 tests/ref/fate/g723_1-dec-13 create mode 100644 tests/ref/fate/g723_1-dec-14 diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index ad335dc8db1..845af6a6284 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -126,7 +126,7 @@ static void g723_1_reset(AVCodecContext *avctx) memcpy(p->sid_lsp, dc_lsp, LPC_ORDER * sizeof(*p->sid_lsp)); p->cng_random_seed = CNG_RANDOM_SEED; - p->past_frame_type = SID_FRAME; + p->past_frame_type = ACTIVE_FRAME; } } diff --git a/tests/fate/voice.mak b/tests/fate/voice.mak index 6def61689c5..8063b4697b0 100644 --- a/tests/fate/voice.mak +++ b/tests/fate/voice.mak @@ -47,6 +47,12 @@ fate-g723_1-dec-7: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/dtx6 FATE_G723_1 += fate-g723_1-dec-8 fate-g723_1-dec-8: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/dtx63e.tco -af aresample +FATE_G723_1 += fate-g723_1-dec-13 +fate-g723_1-dec-13: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/lost-start.tco -af aresample + +FATE_G723_1 += fate-g723_1-dec-14 +fate-g723_1-dec-14: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/lost-start.tco -af aresample + FATE_VOICE-$(call FRAMECRC, G723_1, G723_1, ARESAMPLE_FILTER) += $(FATE_G723_1) fate-g723_1: $(FATE_G723_1) diff --git a/tests/ref/fate/g723_1-dec-13 b/tests/ref/fate/g723_1-dec-13 new file mode 100644 index 00000000000..9b8fee9cc26 --- /dev/null +++ b/tests/ref/fate/g723_1-dec-13 @@ -0,0 +1,22 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x00000000 +0, 240, 240, 240, 480, 0x00000000 +0, 480, 480, 240, 480, 0x9480dd02 +0, 720, 720, 240, 480, 0xb057f95f +0, 960, 960, 240, 480, 0x33e3f142 +0, 1200, 1200, 240, 480, 0x3764efc1 +0, 1440, 1440, 240, 480, 0x160ae5b8 +0, 1680, 1680, 240, 480, 0xe2c1e2e3 +0, 1920, 1920, 240, 480, 0x34160118 +0, 2160, 2160, 240, 480, 0x381016e7 +0, 2400, 2400, 240, 480, 0xc895f248 +0, 2640, 2640, 240, 480, 0xc05feaf9 +0, 2880, 2880, 240, 480, 0xa143dc47 +0, 3120, 3120, 240, 480, 0xbe7ef523 +0, 3360, 3360, 240, 480, 0x8ed10498 +0, 3600, 3600, 240, 480, 0xf230076a +0, 3840, 3840, 240, 480, 0x3da7f716 diff --git a/tests/ref/fate/g723_1-dec-14 b/tests/ref/fate/g723_1-dec-14 new file mode 100644 index 00000000000..17a0e82bfd7 --- /dev/null +++ b/tests/ref/fate/g723_1-dec-14 @@ -0,0 +1,22 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x00000000 +0, 240, 240, 240, 480, 0x00000000 +0, 480, 480, 240, 480, 0xbec0dfdc +0, 720, 720, 240, 480, 0xa7eef911 +0, 960, 960, 240, 480, 0x1c69fcf1 +0, 1200, 1200, 240, 480, 0xc554f334 +0, 1440, 1440, 240, 480, 0xbf98ea0d +0, 1680, 1680, 240, 480, 0x6c49df70 +0, 1920, 1920, 240, 480, 0xf7810151 +0, 2160, 2160, 240, 480, 0xc4b61932 +0, 2400, 2400, 240, 480, 0x3bfff0d1 +0, 2640, 2640, 240, 480, 0xea4ce757 +0, 2880, 2880, 240, 480, 0xe463e27f +0, 3120, 3120, 240, 480, 0x850afacf +0, 3360, 3360, 240, 480, 0x67ddfc58 +0, 3600, 3600, 240, 480, 0xc89908b4 +0, 3840, 3840, 240, 480, 0x21fef29e -- 2.54.0 >From 35ff3561dbaf9de35fdd8941e6f3e39fc2bb5042 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:40 -0500 Subject: [PATCH 06/10] avcodec/g723_1: saturate the negated LPC coefficients like the reference The ITU reference computes each LPC coefficient as neg16(round16(x)), and neg16(-32768) saturates to 32767 (DS2-Anywhere SPEC.md section 4.3.2, step 4). FFmpeg stores the coefficients negated, as round16(x), and its synthesis filter subtracts them. When round16(x) is -32768, FFmpeg's filter uses +32768 where the reference uses 32767. This has been so since the decoder was added in 2011. Clamp the stored value at -INT16_MAX. The encoder shares this code through ff_g723_1_lsp_interpolate(), and fate-acodec-g723_1 passes unchanged. The cosine step at the top of lsp2lpc() negates too. It could only give -32768 for an LSP of 32736 or more. The largest LSP in any of the test streams is 31159, at frame 690 of g723-mix.g723pk. Over the 35 random streams, the last 5 frames that differed from the reference now match, with the postfilter on and with it off. These streams draw their gains at random and never get loud enough to saturate the synthesis filter or the formant postfilter. The next three commits handle that. Frame 91 of cng-erasures.tco is a 6.3 kbit/s frame after an untransmitted frame. Its last subframe saturates coefficient 4. With the postfilter on and every G.723.1 fix but this one, the frame parts from the reference at sample 193 by up to 46, and 4 frames differ. cng-erasures.tco holds 97 frames in 1377 bytes: 35 at 6.3 kbit/s, 22 at 5.3 kbit/s, 19 SID frames and 21 untransmitted ones. 9 active frames fail the validity checks. 7 of them follow an active frame and are concealed, and the other 2 count as untransmitted. 14 untransmitted frames follow an active frame, so their SID frame was lost. The stream starts with a SID frame, loses three active frames in a row at frames 69 to 71, and saturates an LPC coefficient at frame 91. Before the G.723.1 fixes, all 97 frames differ from the reference. fate-g723_1-dec-9 decodes cng-erasures.tco with the postfilter on, and fate-g723_1-dec-10 with it off. Their refs match the DS2-Anywhere G.723.1 decoder frame for frame. Both tests fail with this fix or any of the four G.723.1 fixes before it left out. fate-g723_1-dec-1 to -8 pass unchanged. Upload the sample to fate-suite as g723_1/cng-erasures.tco. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1.c | 8 +-- tests/fate/voice.mak | 6 +++ tests/ref/fate/g723_1-dec-10 | 102 +++++++++++++++++++++++++++++++++++ tests/ref/fate/g723_1-dec-9 | 102 +++++++++++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 tests/ref/fate/g723_1-dec-10 create mode 100644 tests/ref/fate/g723_1-dec-9 diff --git a/libavcodec/g723_1.c b/libavcodec/g723_1.c index d19d09dd899..1bd8f99770b 100644 --- a/libavcodec/g723_1.c +++ b/libavcodec/g723_1.c @@ -1243,9 +1243,11 @@ static void lsp2lpc(int16_t *lpc) int64_t ff1 = f1[i + 1] + f1[i]; int64_t ff2 = f2[i + 1] - f2[i]; - lpc[i] = av_clipl_int32(((ff1 + ff2) * 8) + (1 << 15)) >> 16; - lpc[LPC_ORDER - i - 1] = av_clipl_int32(((ff1 - ff2) * 8) + - (1 << 15)) >> 16; + lpc[i] = FFMAX(av_clipl_int32(((ff1 + ff2) * 8) + (1 << 15)) >> 16, + -INT16_MAX); + lpc[LPC_ORDER - i - 1] = FFMAX(av_clipl_int32(((ff1 - ff2) * 8) + + (1 << 15)) >> 16, + -INT16_MAX); } } diff --git a/tests/fate/voice.mak b/tests/fate/voice.mak index 8063b4697b0..f486e20c01d 100644 --- a/tests/fate/voice.mak +++ b/tests/fate/voice.mak @@ -47,6 +47,12 @@ fate-g723_1-dec-7: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/dtx6 FATE_G723_1 += fate-g723_1-dec-8 fate-g723_1-dec-8: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/dtx63e.tco -af aresample +FATE_G723_1 += fate-g723_1-dec-9 +fate-g723_1-dec-9: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/cng-erasures.tco -af aresample + +FATE_G723_1 += fate-g723_1-dec-10 +fate-g723_1-dec-10: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/cng-erasures.tco -af aresample + FATE_G723_1 += fate-g723_1-dec-13 fate-g723_1-dec-13: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/lost-start.tco -af aresample diff --git a/tests/ref/fate/g723_1-dec-10 b/tests/ref/fate/g723_1-dec-10 new file mode 100644 index 00000000000..6379f07528f --- /dev/null +++ b/tests/ref/fate/g723_1-dec-10 @@ -0,0 +1,102 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x87aeebcc +0, 240, 240, 240, 480, 0xbffe0499 +0, 480, 480, 240, 480, 0xe835ed9f +0, 720, 720, 240, 480, 0x7265eb0a +0, 960, 960, 240, 480, 0x651ce43c +0, 1200, 1200, 240, 480, 0x1f0ae5f9 +0, 1440, 1440, 240, 480, 0x661cf0d2 +0, 1680, 1680, 240, 480, 0xfbddda7c +0, 1920, 1920, 240, 480, 0xf852034c +0, 2160, 2160, 240, 480, 0xa2beed3a +0, 2400, 2400, 240, 480, 0x1d86e61a +0, 2640, 2640, 240, 480, 0xf1a4eedd +0, 2880, 2880, 240, 480, 0xaa8ff792 +0, 3120, 3120, 240, 480, 0xcad5f1e9 +0, 3360, 3360, 240, 480, 0xb868ee27 +0, 3600, 3600, 240, 480, 0x85f3f875 +0, 3840, 3840, 240, 480, 0x379ce9ab +0, 4080, 4080, 240, 480, 0xea74f333 +0, 4320, 4320, 240, 480, 0xb0eceead +0, 4560, 4560, 240, 480, 0x52c8d2fe +0, 4800, 4800, 240, 480, 0x85a3c73e +0, 5040, 5040, 240, 480, 0x9b96e38d +0, 5280, 5280, 240, 480, 0x0856f042 +0, 5520, 5520, 240, 480, 0x5a02e9c7 +0, 5760, 5760, 240, 480, 0x9d10ea4c +0, 6000, 6000, 240, 480, 0x3bcae0cb +0, 6240, 6240, 240, 480, 0xef47e838 +0, 6480, 6480, 240, 480, 0x293cecb0 +0, 6720, 6720, 240, 480, 0x6409f2fb +0, 6960, 6960, 240, 480, 0x9b83f342 +0, 7200, 7200, 240, 480, 0xa29ef241 +0, 7440, 7440, 240, 480, 0xdeaff0a8 +0, 7680, 7680, 240, 480, 0x5481ee53 +0, 7920, 7920, 240, 480, 0xf323eb4c +0, 8160, 8160, 240, 480, 0x8613e597 +0, 8400, 8400, 240, 480, 0x1f8ce94b +0, 8640, 8640, 240, 480, 0x29fddd40 +0, 8880, 8880, 240, 480, 0xeebde954 +0, 9120, 9120, 240, 480, 0xdd1de35c +0, 9360, 9360, 240, 480, 0x1e78d8e4 +0, 9600, 9600, 240, 480, 0x7e76ee55 +0, 9840, 9840, 240, 480, 0x3d18de27 +0, 10080, 10080, 240, 480, 0x9364ca4e +0, 10320, 10320, 240, 480, 0xcc9af167 +0, 10560, 10560, 240, 480, 0x65cfff58 +0, 10800, 10800, 240, 480, 0x051bee3a +0, 11040, 11040, 240, 480, 0x218bdf5c +0, 11280, 11280, 240, 480, 0x824befad +0, 11520, 11520, 240, 480, 0x7edaf36e +0, 11760, 11760, 240, 480, 0xa2ffe096 +0, 12000, 12000, 240, 480, 0xa0a4e1c1 +0, 12240, 12240, 240, 480, 0x391ef22f +0, 12480, 12480, 240, 480, 0xeab5f8a2 +0, 12720, 12720, 240, 480, 0x5654378c +0, 12960, 12960, 240, 480, 0xbd1be151 +0, 13200, 13200, 240, 480, 0xf34ddabd +0, 13440, 13440, 240, 480, 0x8c8fee74 +0, 13680, 13680, 240, 480, 0xedca1919 +0, 13920, 13920, 240, 480, 0x86df3022 +0, 14160, 14160, 240, 480, 0xdadc0390 +0, 14400, 14400, 240, 480, 0x185df533 +0, 14640, 14640, 240, 480, 0xa92bf03d +0, 14880, 14880, 240, 480, 0x5301f6a0 +0, 15120, 15120, 240, 480, 0x635df4d6 +0, 15360, 15360, 240, 480, 0xbc6ff11e +0, 15600, 15600, 240, 480, 0xb4aef63a +0, 15840, 15840, 240, 480, 0xbfdcf57e +0, 16080, 16080, 240, 480, 0x2e0cdf25 +0, 16320, 16320, 240, 480, 0xf7f9ee90 +0, 16560, 16560, 240, 480, 0x62c6ee26 +0, 16800, 16800, 240, 480, 0x355ef5b5 +0, 17040, 17040, 240, 480, 0x9853367a +0, 17280, 17280, 240, 480, 0xab5bd578 +0, 17520, 17520, 240, 480, 0xcd15f21b +0, 17760, 17760, 240, 480, 0x7bf3ee2b +0, 18000, 18000, 240, 480, 0x5ec2f0e3 +0, 18240, 18240, 240, 480, 0xfe0ff109 +0, 18480, 18480, 240, 480, 0xc701d5c4 +0, 18720, 18720, 240, 480, 0xbb3df7de +0, 18960, 18960, 240, 480, 0x9715f71f +0, 19200, 19200, 240, 480, 0x5bfef181 +0, 19440, 19440, 240, 480, 0xa3813b28 +0, 19680, 19680, 240, 480, 0x6b3745bc +0, 19920, 19920, 240, 480, 0x8b44e88c +0, 20160, 20160, 240, 480, 0xab94f540 +0, 20400, 20400, 240, 480, 0x773e0057 +0, 20640, 20640, 240, 480, 0x99b3f729 +0, 20880, 20880, 240, 480, 0x8fea1bb0 +0, 21120, 21120, 240, 480, 0x9d30f101 +0, 21360, 21360, 240, 480, 0x18e1fd48 +0, 21600, 21600, 240, 480, 0x46ecf14b +0, 21840, 21840, 240, 480, 0xecd4f472 +0, 22080, 22080, 240, 480, 0x75bee0cc +0, 22320, 22320, 240, 480, 0x99c5f819 +0, 22560, 22560, 240, 480, 0x929aec97 +0, 22800, 22800, 240, 480, 0x438be410 +0, 23040, 23040, 240, 480, 0x46e0ea5d diff --git a/tests/ref/fate/g723_1-dec-9 b/tests/ref/fate/g723_1-dec-9 new file mode 100644 index 00000000000..5fd55ca8f8d --- /dev/null +++ b/tests/ref/fate/g723_1-dec-9 @@ -0,0 +1,102 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x9960eb28 +0, 240, 240, 240, 480, 0x829b03b8 +0, 480, 480, 240, 480, 0x2ebce6c8 +0, 720, 720, 240, 480, 0xec07f417 +0, 960, 960, 240, 480, 0xebdddefa +0, 1200, 1200, 240, 480, 0xa6d6ed37 +0, 1440, 1440, 240, 480, 0x18e0e491 +0, 1680, 1680, 240, 480, 0x91b2e0ed +0, 1920, 1920, 240, 480, 0x5af805dc +0, 2160, 2160, 240, 480, 0x821aefb4 +0, 2400, 2400, 240, 480, 0x646be254 +0, 2640, 2640, 240, 480, 0xec8ff4d3 +0, 2880, 2880, 240, 480, 0x1e75f923 +0, 3120, 3120, 240, 480, 0x3f58eb3d +0, 3360, 3360, 240, 480, 0xa87af21b +0, 3600, 3600, 240, 480, 0x3ee1ea42 +0, 3840, 3840, 240, 480, 0x6122ed84 +0, 4080, 4080, 240, 480, 0xda0aef9c +0, 4320, 4320, 240, 480, 0x9c7ff1c6 +0, 4560, 4560, 240, 480, 0x3dcfdcae +0, 4800, 4800, 240, 480, 0x54a7d65a +0, 5040, 5040, 240, 480, 0xf6b8e44c +0, 5280, 5280, 240, 480, 0x5b7cf4ad +0, 5520, 5520, 240, 480, 0x6642e64e +0, 5760, 5760, 240, 480, 0x473ee490 +0, 6000, 6000, 240, 480, 0xe867f091 +0, 6240, 6240, 240, 480, 0xd7dbea63 +0, 6480, 6480, 240, 480, 0x6792f0b1 +0, 6720, 6720, 240, 480, 0xb67aef82 +0, 6960, 6960, 240, 480, 0x3688fb2a +0, 7200, 7200, 240, 480, 0x5c12e8be +0, 7440, 7440, 240, 480, 0xe293ed4c +0, 7680, 7680, 240, 480, 0x5108f3b7 +0, 7920, 7920, 240, 480, 0x469bd9fd +0, 8160, 8160, 240, 480, 0xbc60e7f1 +0, 8400, 8400, 240, 480, 0x0b9de2cc +0, 8640, 8640, 240, 480, 0xb073ec19 +0, 8880, 8880, 240, 480, 0x33f3ee20 +0, 9120, 9120, 240, 480, 0x1546e0a4 +0, 9360, 9360, 240, 480, 0xa7bdebc6 +0, 9600, 9600, 240, 480, 0xf096f4c4 +0, 9840, 9840, 240, 480, 0xf55de7b7 +0, 10080, 10080, 240, 480, 0x510bd374 +0, 10320, 10320, 240, 480, 0x159ee967 +0, 10560, 10560, 240, 480, 0x661b040d +0, 10800, 10800, 240, 480, 0x523fe64c +0, 11040, 11040, 240, 480, 0x10b5e79e +0, 11280, 11280, 240, 480, 0x982eed92 +0, 11520, 11520, 240, 480, 0x5d52ef3f +0, 11760, 11760, 240, 480, 0xe398d7a7 +0, 12000, 12000, 240, 480, 0xe759e4fc +0, 12240, 12240, 240, 480, 0xa4a3f5b1 +0, 12480, 12480, 240, 480, 0x8702f703 +0, 12720, 12720, 240, 480, 0x991731f2 +0, 12960, 12960, 240, 480, 0x47e3e201 +0, 13200, 13200, 240, 480, 0x6dc3db07 +0, 13440, 13440, 240, 480, 0x86f6ef1c +0, 13680, 13680, 240, 480, 0x869814c0 +0, 13920, 13920, 240, 480, 0x893721b5 +0, 14160, 14160, 240, 480, 0xa9b7039c +0, 14400, 14400, 240, 480, 0x1b27f7c5 +0, 14640, 14640, 240, 480, 0x4f8deff4 +0, 14880, 14880, 240, 480, 0xced6f164 +0, 15120, 15120, 240, 480, 0x571af02e +0, 15360, 15360, 240, 480, 0x7d93eebb +0, 15600, 15600, 240, 480, 0x4659f6ac +0, 15840, 15840, 240, 480, 0x51fbeaa6 +0, 16080, 16080, 240, 480, 0xd2a8e0f7 +0, 16320, 16320, 240, 480, 0xcfbae2e3 +0, 16560, 16560, 240, 480, 0xf908e9e0 +0, 16800, 16800, 240, 480, 0x06f2ed1b +0, 17040, 17040, 240, 480, 0x402534b0 +0, 17280, 17280, 240, 480, 0x86c9d532 +0, 17520, 17520, 240, 480, 0xab65f21f +0, 17760, 17760, 240, 480, 0xb543f0f5 +0, 18000, 18000, 240, 480, 0x4e44efbc +0, 18240, 18240, 240, 480, 0xfe7aea43 +0, 18480, 18480, 240, 480, 0x675cdc8b +0, 18720, 18720, 240, 480, 0x2536f58a +0, 18960, 18960, 240, 480, 0x1064f6c6 +0, 19200, 19200, 240, 480, 0xd712ede5 +0, 19440, 19440, 240, 480, 0x6ba13597 +0, 19680, 19680, 240, 480, 0x29e143cf +0, 19920, 19920, 240, 480, 0x3630da3f +0, 20160, 20160, 240, 480, 0xbce7f531 +0, 20400, 20400, 240, 480, 0xdab9ee8e +0, 20640, 20640, 240, 480, 0x534df31e +0, 20880, 20880, 240, 480, 0x3153ff06 +0, 21120, 21120, 240, 480, 0x6dacf172 +0, 21360, 21360, 240, 480, 0x833405ee +0, 21600, 21600, 240, 480, 0x92ffef77 +0, 21840, 21840, 240, 480, 0x307de935 +0, 22080, 22080, 240, 480, 0x4880e8cc +0, 22320, 22320, 240, 480, 0x4aa0f6e9 +0, 22560, 22560, 240, 480, 0x76f2f2de +0, 22800, 22800, 240, 480, 0xcdbdf36b +0, 23040, 23040, 240, 480, 0x2a29eed8 -- 2.54.0 >From b909ba2f5d71c8bbe5b0adb81767ead765949836 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:41 -0500 Subject: [PATCH 07/10] avcodec/g723_1dec: saturate the synthesis filter like the reference The ITU reference runs the LPC synthesis filter in 32 bits and saturates the sum after every product (DS2-Anywhere SPEC.md section 4.7). FFmpeg uses ff_celp_lp_synthesis_filter(), which sums with wraparound and clips once at the end. The two agree until a partial sum overflows. On loud signals with large LPC coefficients that happens, and FFmpeg's output can then be off by the whole 16-bit range. This has been so since the decoder was added in 2011. Give the decoder its own synthesis filter that does what the reference does: the input times 8192, a saturating multiply-subtract for each coefficient in the reference's order, a saturating shift by 2, then rounding to 16 bits. ff_celp_lp_synthesis_filter() stays as it is for the other decoders that use it. I checked the cause on three short cuts from loud streams. A Python model of the reference's filter reproduces the reference decoder sample for sample, a model of FFmpeg's filter reproduces FFmpeg, and the reference's sum saturates 17 to 50 times in each cut. The test set for this and the next two commits is 371 streams of random G.723.1 frames, 822343 frames in all: the 35 random streams of the earlier commits, 238 with loud gains or many lost frames, 90 that start or lose frames in unusual ways, and a few short cuts. Over them, the frames that differ from the reference drop from 61 to 0 with the postfilter off and from 263 to 145 with it on. saturation.tco holds 48 frames in 961 bytes, loud enough that the synthesis filter and the formant postfilter saturate, with a reflection coefficient that rounds differently at frame 20. Before the G.723.1 fixes, 25 of its 48 frames differ from the reference. Frame 42 of saturation.tco is a loud 5.3 kbit/s frame. With the postfilter off and every G.723.1 fix but this one, sample 224 is 32767 where the reference gives 25516, the frame differs by up to 65535, and 6 frames differ. fate-g723_1-dec-12 decodes saturation.tco with the postfilter off. Its ref matches the DS2-Anywhere G.723.1 decoder frame for frame. It fails with this fix or the Annex A fix left out. fate-g723_1-dec-1 to -8 pass unchanged. Upload the sample to fate-suite as g723_1/saturation.tco. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 19 ++++++++++--- tests/fate/voice.mak | 3 ++ tests/ref/fate/g723_1-dec-12 | 53 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/ref/fate/g723_1-dec-12 diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index 845af6a6284..b230b6f1c2a 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -32,7 +32,6 @@ #define BITSTREAM_READER_LE #include "acelp_vectors.h" #include "avcodec.h" -#include "celp_filters.h" #include "codec_internal.h" #include "decode.h" #include "get_bits.h" @@ -931,6 +930,20 @@ static void generate_noise(G723_1_ChannelContext *p) PITCH_MAX * sizeof(*p->excitation)); } +static void lp_synthesis_filter(int16_t *out, const int16_t *lpc, + const int16_t *in) +{ + for (int n = 0; n < SUBFRAME_LEN; n++) { + int sum = in[n] * (1 << 13); + + for (int i = 0; i < LPC_ORDER; i++) + sum = av_sat_dsub32(sum, lpc[i] * out[n - 1 - i]); + + sum = av_clipl_int32((int64_t)sum * 4); + out[n] = av_sat_add32(sum, 1 << 15) >> 16; + } +} + static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_frame_ptr, AVPacket *avpkt) { @@ -1087,9 +1100,7 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, memcpy(p->audio, p->synth_mem, LPC_ORDER * sizeof(*p->audio)); for (i = LPC_ORDER, j = 0; j < SUBFRAMES; i += SUBFRAME_LEN, j++) - ff_celp_lp_synthesis_filter(p->audio + i, &lpc[j * LPC_ORDER], - audio + i, SUBFRAME_LEN, LPC_ORDER, - 0, 1, 1 << 12); + lp_synthesis_filter(p->audio + i, &lpc[j * LPC_ORDER], audio + i); memcpy(p->synth_mem, p->audio + FRAME_LEN, LPC_ORDER * sizeof(*p->audio)); if (s->postfilter) { diff --git a/tests/fate/voice.mak b/tests/fate/voice.mak index f486e20c01d..c4236f99a1b 100644 --- a/tests/fate/voice.mak +++ b/tests/fate/voice.mak @@ -53,6 +53,9 @@ fate-g723_1-dec-9: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/cng- FATE_G723_1 += fate-g723_1-dec-10 fate-g723_1-dec-10: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/cng-erasures.tco -af aresample +FATE_G723_1 += fate-g723_1-dec-12 +fate-g723_1-dec-12: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/saturation.tco -af aresample + FATE_G723_1 += fate-g723_1-dec-13 fate-g723_1-dec-13: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/lost-start.tco -af aresample diff --git a/tests/ref/fate/g723_1-dec-12 b/tests/ref/fate/g723_1-dec-12 new file mode 100644 index 00000000000..0c24561525d --- /dev/null +++ b/tests/ref/fate/g723_1-dec-12 @@ -0,0 +1,53 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x01b2e49d +0, 240, 240, 240, 480, 0xdc83f49b +0, 480, 480, 240, 480, 0xdf12efa2 +0, 720, 720, 240, 480, 0x1b4e050f +0, 960, 960, 240, 480, 0x3939f6c5 +0, 1200, 1200, 240, 480, 0xd66def25 +0, 1440, 1440, 240, 480, 0xdfdbe8ed +0, 1680, 1680, 240, 480, 0x5d60f777 +0, 1920, 1920, 240, 480, 0xbcc9ed22 +0, 2160, 2160, 240, 480, 0x0590ef0a +0, 2400, 2400, 240, 480, 0x8ef0e897 +0, 2640, 2640, 240, 480, 0x0913d866 +0, 2880, 2880, 240, 480, 0xb51cded3 +0, 3120, 3120, 240, 480, 0x4e20e45a +0, 3360, 3360, 240, 480, 0xa2afebcf +0, 3600, 3600, 240, 480, 0xcdfef505 +0, 3840, 3840, 240, 480, 0xd99ae7c7 +0, 4080, 4080, 240, 480, 0x9072f588 +0, 4320, 4320, 240, 480, 0x627ae859 +0, 4560, 4560, 240, 480, 0x2a54e445 +0, 4800, 4800, 240, 480, 0xc903f0bd +0, 5040, 5040, 240, 480, 0xcb6cfcba +0, 5280, 5280, 240, 480, 0x8257f79c +0, 5520, 5520, 240, 480, 0xb4ade802 +0, 5760, 5760, 240, 480, 0x0242f960 +0, 6000, 6000, 240, 480, 0xd6f9ede7 +0, 6240, 6240, 240, 480, 0x87bee16b +0, 6480, 6480, 240, 480, 0x0326eeb0 +0, 6720, 6720, 240, 480, 0x8debf274 +0, 6960, 6960, 240, 480, 0x350ef634 +0, 7200, 7200, 240, 480, 0x7bd20d93 +0, 7440, 7440, 240, 480, 0x010cd501 +0, 7680, 7680, 240, 480, 0x5fe1d1a3 +0, 7920, 7920, 240, 480, 0x148f1e5d +0, 8160, 8160, 240, 480, 0x0d0c1340 +0, 8400, 8400, 240, 480, 0xf495db89 +0, 8640, 8640, 240, 480, 0x985ddd43 +0, 8880, 8880, 240, 480, 0xbbb9e18d +0, 9120, 9120, 240, 480, 0xe11dde0a +0, 9360, 9360, 240, 480, 0x87cef472 +0, 9600, 9600, 240, 480, 0x4c0fe75b +0, 9840, 9840, 240, 480, 0x46d5e56f +0, 10080, 10080, 240, 480, 0xdce2f488 +0, 10320, 10320, 240, 480, 0x4eddf50d +0, 10560, 10560, 240, 480, 0x19a2f38d +0, 10800, 10800, 240, 480, 0x4e29ee4d +0, 11040, 11040, 240, 480, 0xd66ee964 +0, 11280, 11280, 240, 480, 0xc9aeefb1 -- 2.54.0 >From 3fe9ebd7722877fc117c490a718daf6fea508937 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:42 -0500 Subject: [PATCH 08/10] avcodec/g723_1dec: saturate the formant postfilter like the reference The formant postfilter's ARMA filter has the same problem as the synthesis filter, and one more (DS2-Anywhere SPEC.md section 4.8.1, step 8). The reference saturates its sum after every product, while FFmpeg's iir_filter() macro sums in 64 bits and clips once. The reference then keeps the saturated 32-bit sum, adds the tilt term to it and rounds. FFmpeg adds the rounding constant before the clip and the tilt term after it, so near full scale its output can come out 1 lower. This has been so since the decoder was added in 2011. Replace the macro with a filter that saturates after every product and stores the unrounded sum. Round the previous output where the filter and the tilt term read it, and round the output only after adding the tilt term. Over the 371 test streams, the frames that differ from the reference with the postfilter on drop from 145 to 1, and the streams that differ from 32 to 1. With the postfilter off the formant postfilter does not run. Frame 42 of saturation.tco: with the postfilter on and every G.723.1 fix but this one, sample 121 is -14369 where the reference gives -14366, and the frame differs by up to 13. fate-g723_1-dec-1 to -8 pass unchanged. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 47 +++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index b230b6f1c2a..5414bf87127 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -583,31 +583,20 @@ static void residual_interp(int16_t *buf, int16_t *out, int lag, } } -/** - * Perform IIR filtering. - * - * @param fir_coef FIR coefficients - * @param iir_coef IIR coefficients - * @param src source vector - * @param dest destination vector - * @param width width of the output, 16 bits(0) / 32 bits(1) - */ -#define iir_filter(fir_coef, iir_coef, src, dest, width)\ -{\ - int m, n;\ - int res_shift = 16 & ~-(width);\ - int in_shift = 16 - res_shift;\ -\ - for (m = 0; m < SUBFRAME_LEN; m++) {\ - int64_t filter = 0;\ - for (n = 1; n <= LPC_ORDER; n++) {\ - filter -= (fir_coef)[n - 1] * (src)[m - n] -\ - (iir_coef)[n - 1] * ((dest)[m - n] >> in_shift);\ - }\ -\ - (dest)[m] = av_clipl_int32(((src)[m] * 65536) + (filter * 8) +\ - (1 << 15)) >> res_shift;\ - }\ +static void formant_filter(const int16_t *fir_coef, const int16_t *iir_coef, + const int16_t *src, int *dest) +{ + for (int m = 0; m < SUBFRAME_LEN; m++) { + int sum = src[m] * (1 << 14); + + for (int n = 1; n <= LPC_ORDER; n++) + sum = av_sat_dsub32(sum, fir_coef[n - 1] * src[m - n]); + for (int n = 1; n <= LPC_ORDER; n++) + sum = av_sat_dadd32(sum, iir_coef[n - 1] * + (av_sat_add32(dest[m - n], 1 << 15) >> 16)); + + dest[m] = av_clipl_int32((int64_t)sum * 4); + } } /** @@ -677,7 +666,7 @@ static void formant_postfilter(G723_1_ChannelContext *p, int16_t *lpc, filter_coef[1][k] = (-lpc[k] * postfilter_tbl[1][k] + (1 << 14)) >> 15; } - iir_filter(filter_coef[0], filter_coef[1], buf + i, filter_signal + i, 1); + formant_filter(filter_coef[0], filter_coef[1], buf + i, filter_signal + i); lpc += LPC_ORDER; } @@ -708,8 +697,10 @@ static void formant_postfilter(G723_1_ChannelContext *p, int16_t *lpc, /* Compensation filter */ for (j = 0; j < SUBFRAME_LEN; j++) { - dst[j] = av_sat_dadd32(signal_ptr[j], - (signal_ptr[j - 1] >> 16) * temp) >> 16; + int prev = av_sat_add32(signal_ptr[j - 1], 1 << 15) >> 16; + + dst[j] = av_sat_add32(av_sat_dadd32(signal_ptr[j], prev * temp), + 1 << 15) >> 16; } /* Compute normalized signal energy */ -- 2.54.0 >From b8cb3e3e9b4f896f545a8848cd638b1186343854 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:42 -0500 Subject: [PATCH 09/10] avcodec/g723_1dec: round the reflection coefficient like the reference The formant postfilter's tilt follows the first reflection coefficient k of each subframe (DS2-Anywhere SPEC.md section 4.8.1, step 5). The reference halves the autocorrelation R1 with a floor, divides its magnitude by twice the top 16 bits of R0, caps it at 32767 and puts the sign back. FFmpeg floors R1 / 4 and divides with truncation. For some negative R1 the two differ by 1, and the smoothed tilt carries the difference into the next subframes. This has been so since the decoder was added in 2011. Compute k as the reference does. Over the 371 test streams, the last frame that differed from the reference with the postfilter on now matches. All 371 streams decode exactly as the reference does, with the postfilter on and off. Frame 20 of saturation.tco comes from a stream where, in subframe 2, the reference's k is -2389 and FFmpeg's -2390. With the postfilter on and every G.723.1 fix but this one, the frame parts from the reference at sample 183 by up to 3. fate-g723_1-dec-11 decodes saturation.tco with the postfilter on. Its ref matches the DS2-Anywhere G.723.1 decoder frame for frame. It fails with any one of this fix, the formant postfilter fix, the synthesis filter fix and the Annex A fix left out. fate-g723_1-dec-1 to -8 pass unchanged. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 6 +++- tests/fate/voice.mak | 3 ++ tests/ref/fate/g723_1-dec-11 | 53 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 tests/ref/fate/g723_1-dec-11 diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index 5414bf87127..a4c5bdd1d42 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -690,7 +690,11 @@ static void formant_postfilter(G723_1_ChannelContext *p, int16_t *lpc, /* Compute reflection coefficient */ temp = auto_corr[1] >> 16; if (temp) { - temp = (auto_corr[0] >> 2) / temp; + int r1 = auto_corr[0] >> 1; + + temp = FFMIN(FFABS(r1) / (2 * temp), INT16_MAX); + if (r1 < 0) + temp = -temp; } p->reflection_coef = (3 * p->reflection_coef + temp + 2) >> 2; temp = -p->reflection_coef >> 1 & ~3; diff --git a/tests/fate/voice.mak b/tests/fate/voice.mak index c4236f99a1b..b43bd6bd78d 100644 --- a/tests/fate/voice.mak +++ b/tests/fate/voice.mak @@ -53,6 +53,9 @@ fate-g723_1-dec-9: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/cng- FATE_G723_1 += fate-g723_1-dec-10 fate-g723_1-dec-10: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/cng-erasures.tco -af aresample +FATE_G723_1 += fate-g723_1-dec-11 +fate-g723_1-dec-11: CMD = framecrc -postfilter 1 -i $(TARGET_SAMPLES)/g723_1/saturation.tco -af aresample + FATE_G723_1 += fate-g723_1-dec-12 fate-g723_1-dec-12: CMD = framecrc -postfilter 0 -i $(TARGET_SAMPLES)/g723_1/saturation.tco -af aresample diff --git a/tests/ref/fate/g723_1-dec-11 b/tests/ref/fate/g723_1-dec-11 new file mode 100644 index 00000000000..d76eec29792 --- /dev/null +++ b/tests/ref/fate/g723_1-dec-11 @@ -0,0 +1,53 @@ +#tb 0: 1/8000 +#media_type 0: audio +#codec_id 0: pcm_s16le +#sample_rate 0: 8000 +#channel_layout_name 0: mono +0, 0, 0, 240, 480, 0x8c91e30f +0, 240, 240, 240, 480, 0x8dc1f1f4 +0, 480, 480, 240, 480, 0x666fef0e +0, 720, 720, 240, 480, 0xdcaf07db +0, 960, 960, 240, 480, 0xdcecf37f +0, 1200, 1200, 240, 480, 0xc4aaf297 +0, 1440, 1440, 240, 480, 0xdda2f146 +0, 1680, 1680, 240, 480, 0xdffae723 +0, 1920, 1920, 240, 480, 0xf91eebe2 +0, 2160, 2160, 240, 480, 0x5950f145 +0, 2400, 2400, 240, 480, 0x56d9ec81 +0, 2640, 2640, 240, 480, 0x10f5dbf2 +0, 2880, 2880, 240, 480, 0xd107e9d3 +0, 3120, 3120, 240, 480, 0xd609e69c +0, 3360, 3360, 240, 480, 0xf7f4ecfe +0, 3600, 3600, 240, 480, 0xf6a5ef56 +0, 3840, 3840, 240, 480, 0x09b7e4b6 +0, 4080, 4080, 240, 480, 0x567af88b +0, 4320, 4320, 240, 480, 0x1cfefb42 +0, 4560, 4560, 240, 480, 0x17ceec74 +0, 4800, 4800, 240, 480, 0xdefff5e9 +0, 5040, 5040, 240, 480, 0x0447f824 +0, 5280, 5280, 240, 480, 0x252af234 +0, 5520, 5520, 240, 480, 0x7693f2c9 +0, 5760, 5760, 240, 480, 0x0c13f98b +0, 6000, 6000, 240, 480, 0x03feeecc +0, 6240, 6240, 240, 480, 0x1b5bf18d +0, 6480, 6480, 240, 480, 0x3bd0f781 +0, 6720, 6720, 240, 480, 0xe76bf794 +0, 6960, 6960, 240, 480, 0x3d14f718 +0, 7200, 7200, 240, 480, 0xb07a0267 +0, 7440, 7440, 240, 480, 0xcb1eea36 +0, 7680, 7680, 240, 480, 0x3767e516 +0, 7920, 7920, 240, 480, 0xb8cd1660 +0, 8160, 8160, 240, 480, 0xbf330dd7 +0, 8400, 8400, 240, 480, 0x764be98e +0, 8640, 8640, 240, 480, 0x2993e407 +0, 8880, 8880, 240, 480, 0xf1a6db54 +0, 9120, 9120, 240, 480, 0x0797e462 +0, 9360, 9360, 240, 480, 0x9b9bf011 +0, 9600, 9600, 240, 480, 0x67ceeb77 +0, 9840, 9840, 240, 480, 0xede5dd4a +0, 10080, 10080, 240, 480, 0x7787f6e2 +0, 10320, 10320, 240, 480, 0x98ebfc01 +0, 10560, 10560, 240, 480, 0x1129e64c +0, 10800, 10800, 240, 480, 0xd631e94f +0, 11040, 11040, 240, 480, 0x3120f0d7 +0, 11280, 11280, 240, 480, 0xc07cf758 -- 2.54.0 >From b1862306a35a64677587d9c95fff031996725ff1 Mon Sep 17 00:00:00 2001 From: Patrick Domack <[email protected]> Date: Sun, 1 Nov 2026 04:56:43 -0500 Subject: [PATCH 10/10] avcodec/g723_1dec: track bad frames per channel Stereo support, added in 62dbcb7ddf (2018), moved the decoding into a loop over the channels but left bad_frame declared once for the whole packet. Once one channel's frame fails the validity checks, every channel after it in that packet is decoded as a lost frame too, even when its own frame is fine, and its state then drifts away from what its own frames would give. Declare bad_frame inside the channel loop. No demuxer hands the decoder a stereo G.723.1 stream, so I tested this with a small libavcodec program. It sends 2000 packets of two 6.3 kbit/s frames to a stereo decoder, with about 10 percent of the frames in each channel made invalid at random, and sends each channel's frames to a mono decoder of its own. Before this change, channel 1 of the stereo output differed from its mono decode in 1788 of the 2000 frames. Now neither channel differs in any frame. Mono decoding does not change: all 374 test streams still match the reference with the postfilter on and off. No FATE test reaches stereo decoding, since the g723_1 demuxer is mono. Signed-off-by: Patrick Domack <[email protected]> --- libavcodec/g723_1dec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/g723_1dec.c b/libavcodec/g723_1dec.c index a4c5bdd1d42..0cede33724b 100644 --- a/libavcodec/g723_1dec.c +++ b/libavcodec/g723_1dec.c @@ -953,7 +953,7 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, int16_t lpc[SUBFRAMES * LPC_ORDER]; int16_t acb_vector[SUBFRAME_LEN]; int16_t *out; - int bad_frame = 0, i, j, ret; + int i, j, ret; if (buf_size < frame_size[dec_mode] * channels) { if (buf_size) @@ -971,6 +971,7 @@ static int g723_1_decode_frame(AVCodecContext *avctx, AVFrame *frame, for (int ch = 0; ch < channels; ch++) { G723_1_ChannelContext *p = &s->ch[ch]; int16_t *audio = p->audio; + int bad_frame = 0; if (unpack_bitstream(p, buf + ch * (buf_size / channels), buf_size / channels) < 0) { -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
