From fbed746cb2d0fee57eae090e67d148d89923f6ff Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Fri, 22 Jul 2022 02:27:34 -0400 Subject: Relaxing checks for MULT16_32_QX() MULT16_32_QX() is now implemented using a signed-unsigned multiply, so the second argument can now have one extra bit compared to the old signed-signed implementation. Reviewed by Mark Harris --- celt/fixed_debug.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/celt/fixed_debug.h b/celt/fixed_debug.h index c2cf5a83..ef2e5d02 100644 --- a/celt/fixed_debug.h +++ b/celt/fixed_debug.h @@ -491,7 +491,7 @@ static OPUS_INLINE int MULT16_32_QX_(int a, opus_int64 b, int Q, char *file, int celt_assert(0); #endif } - if (ABS32(b)>=((opus_val32)(1)<<(15+Q))) + if (ABS32(b)>=((opus_int64)(1)<<(16+Q))) { fprintf (stderr, "MULT16_32_Q%d: second operand too large: %d %d in %s: line %d\n", Q, (int)a, (int)b, file, line); #ifdef FIXED_DEBUG_ASSERT @@ -524,7 +524,7 @@ static OPUS_INLINE int MULT16_32_PX_(int a, opus_int64 b, int Q, char *file, int celt_assert(0); #endif } - if (ABS32(b)>=((opus_int64)(1)<<(15+Q))) + if (ABS32(b)>=((opus_int64)(1)<<(16+Q))) { fprintf (stderr, "MULT16_32_Q%d: second operand too large: %d %d in %s: line %d\n\n", Q, (int)a, (int)b,file, line); #ifdef FIXED_DEBUG_ASSERT -- cgit v1.2.3 From e05aea9785f709f5aebb696ee5b4460681676e10 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Fri, 22 Jul 2022 02:29:05 -0400 Subject: Using saturating round to fix some wrap-arounds Reviewed by Mark Harris --- celt/celt_decoder.c | 8 ++++---- celt/celt_lpc.c | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/celt/celt_decoder.c b/celt/celt_decoder.c index 35a2073a..883dae15 100644 --- a/celt/celt_decoder.c +++ b/celt/celt_decoder.c @@ -629,7 +629,7 @@ static void celt_decode_lost(CELTDecoder * OPUS_RESTRICT st, int N, int LM) buf = decode_mem[c]; for (i=0;i Date: Fri, 22 Jul 2022 02:32:04 -0400 Subject: More ubsan fixes for the debug macros themselves Reviewed by Mark Harris --- silk/MacroDebug.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/silk/MacroDebug.h b/silk/MacroDebug.h index e505d02a..aceeef7e 100644 --- a/silk/MacroDebug.h +++ b/silk/MacroDebug.h @@ -101,9 +101,9 @@ static OPUS_INLINE opus_int16 silk_SUB16_(opus_int16 a, opus_int16 b, char *file #undef silk_SUB32 #define silk_SUB32(a,b) silk_SUB32_((a), (b), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_SUB32_(opus_int32 a, opus_int32 b, char *file, int line){ - opus_int32 ret; + opus_int64 ret; - ret = a - b; + ret = a - (opus_int64)b; if ( ret != silk_SUB_SAT32( a, b ) ) { fprintf (stderr, "silk_SUB32(%d, %d) in %s: line %d\n", a, b, file, line); @@ -333,8 +333,8 @@ static OPUS_INLINE opus_int32 silk_SMULWB_(opus_int32 a32, opus_int32 b32, char #define silk_SMLAWB(a,b,c) silk_SMLAWB_((a), (b), (c), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_SMLAWB_(opus_int32 a32, opus_int32 b32, opus_int32 c32, char *file, int line){ opus_int32 ret; - ret = silk_ADD32( a32, silk_SMULWB( b32, c32 ) ); - if ( silk_ADD32( a32, silk_SMULWB( b32, c32 ) ) != silk_ADD_SAT32( a32, silk_SMULWB( b32, c32 ) ) ) + ret = silk_ADD32_ovflw( a32, silk_SMULWB( b32, c32 ) ); + if ( ret != silk_ADD_SAT32( a32, silk_SMULWB( b32, c32 ) ) ) { fprintf (stderr, "silk_SMLAWB(%d, %d, %d) in %s: line %d\n", a32, b32, c32, file, line); #ifdef FIXED_DEBUG_ASSERT @@ -465,7 +465,7 @@ static OPUS_INLINE opus_int32 silk_SMULWW_(opus_int32 a32, opus_int32 b32, char if ( fail ) { - fprintf (stderr, "silk_SMULWT(%d, %d) in %s: line %d\n", a32, b32, file, line); + fprintf (stderr, "silk_SMULWW(%d, %d) in %s: line %d\n", a32, b32, file, line); #ifdef FIXED_DEBUG_ASSERT silk_assert( 0 ); #endif @@ -723,7 +723,7 @@ static OPUS_INLINE int silk_ADD_LSHIFT_(int a, int b, int shift, char *file, int #define silk_ADD_LSHIFT32(a,b,c) silk_ADD_LSHIFT32_((a), (b), (c), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_ADD_LSHIFT32_(opus_int32 a, opus_int32 b, opus_int32 shift, char *file, int line){ opus_int32 ret; - ret = a + (opus_int32)((opus_uint32)b << shift); + ret = silk_ADD32_ovflw(a, (opus_int32)((opus_uint32)b << shift)); if ( (shift < 0) || (shift>31) || ((opus_int64)ret != (opus_int64)a + (opus_int64)(((opus_uint64)b) << shift)) ) { fprintf (stderr, "silk_ADD_LSHIFT32(%d, %d, %d) in %s: line %d\n", a, b, shift, file, line); @@ -768,7 +768,7 @@ static OPUS_INLINE int silk_ADD_RSHIFT_(int a, int b, int shift, char *file, int #define silk_ADD_RSHIFT32(a,b,c) silk_ADD_RSHIFT32_((a), (b), (c), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_ADD_RSHIFT32_(opus_int32 a, opus_int32 b, opus_int32 shift, char *file, int line){ opus_int32 ret; - ret = a + (b >> shift); + ret = silk_ADD32_ovflw(a, (b >> shift)); if ( (shift < 0) || (shift>31) || ((opus_int64)ret != (opus_int64)a + (((opus_int64)b) >> shift)) ) { fprintf (stderr, "silk_ADD_RSHIFT32(%d, %d, %d) in %s: line %d\n", a, b, shift, file, line); @@ -798,7 +798,7 @@ static OPUS_INLINE opus_uint32 silk_ADD_RSHIFT_uint_(opus_uint32 a, opus_uint32 #define silk_SUB_LSHIFT32(a,b,c) silk_SUB_LSHIFT32_((a), (b), (c), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_SUB_LSHIFT32_(opus_int32 a, opus_int32 b, opus_int32 shift, char *file, int line){ opus_int32 ret; - ret = a - (opus_int32)((opus_uint32)b << shift); + ret = silk_SUB32_ovflw(a, (opus_int32)((opus_uint32)b << shift)); if ( (shift < 0) || (shift>31) || ((opus_int64)ret != (opus_int64)a - (opus_int64)(((opus_uint64)b) << shift)) ) { fprintf (stderr, "silk_SUB_LSHIFT32(%d, %d, %d) in %s: line %d\n", a, b, shift, file, line); @@ -813,7 +813,7 @@ static OPUS_INLINE opus_int32 silk_SUB_LSHIFT32_(opus_int32 a, opus_int32 b, opu #define silk_SUB_RSHIFT32(a,b,c) silk_SUB_RSHIFT32_((a), (b), (c), __FILE__, __LINE__) static OPUS_INLINE opus_int32 silk_SUB_RSHIFT32_(opus_int32 a, opus_int32 b, opus_int32 shift, char *file, int line){ opus_int32 ret; - ret = a - (b >> shift); + ret = silk_SUB32_ovflw(a, (b >> shift)); if ( (shift < 0) || (shift>31) || ((opus_int64)ret != (opus_int64)a - (((opus_int64)b) >> shift)) ) { fprintf (stderr, "silk_SUB_RSHIFT32(%d, %d, %d) in %s: line %d\n", a, b, shift, file, line); -- cgit v1.2.3 From 378b4e5fc31b63c1f1a9f6a87a62609c5a083724 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Fri, 22 Jul 2022 02:53:39 -0400 Subject: Ensuring we can see where crashes occur Reviewed by Mark Harris --- tests/test_opus_encode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_opus_encode.c b/tests/test_opus_encode.c index 00795a1e..d6e8e2d3 100644 --- a/tests/test_opus_encode.c +++ b/tests/test_opus_encode.c @@ -297,6 +297,7 @@ int run_test1(int no_fuzz) /*FIXME: encoder api tests, fs!=48k, mono, VBR*/ fprintf(stdout," Encode+Decode tests.\n"); + fflush(stdout); enc = opus_encoder_create(48000, 2, OPUS_APPLICATION_VOIP, &err); if(err != OPUS_OK || enc==NULL)test_failed(); @@ -466,6 +467,7 @@ int run_test1(int no_fuzz) count++; }while(i<(SSAMPLES-MAX_FRAME_SAMP)); fprintf(stdout," Mode %s FB encode %s, %6d bps OK.\n",mstrings[modes[j]],rc==0?" VBR":rc==1?"CVBR":" CBR",rate); + fflush(stdout); } } @@ -543,6 +545,7 @@ int run_test1(int no_fuzz) count++; }while(i<(SSAMPLES/12-MAX_FRAME_SAMP)); fprintf(stdout," Mode %s NB dual-mono MS encode %s, %6d bps OK.\n",mstrings[modes[j]],rc==0?" VBR":rc==1?"CVBR":" CBR",rate); + fflush(stdout); } } @@ -612,6 +615,7 @@ int run_test1(int no_fuzz) i+=frame_size; }while(i Date: Sun, 24 Jul 2022 02:12:03 -0400 Subject: Re-tuning the use of LTP scaling Making LTP scaling depend on the bitrate and whether FEC is on. The thresholds for scaling 1 and 2 are now independent. --- silk/fixed/LTP_scale_ctrl_FIX.c | 9 +++++++-- silk/float/LTP_scale_ctrl_FLP.c | 8 +++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/silk/fixed/LTP_scale_ctrl_FIX.c b/silk/fixed/LTP_scale_ctrl_FIX.c index 3dcedef8..b3afb70b 100644 --- a/silk/fixed/LTP_scale_ctrl_FIX.c +++ b/silk/fixed/LTP_scale_ctrl_FIX.c @@ -43,8 +43,13 @@ void silk_LTP_scale_ctrl_FIX( if( condCoding == CODE_INDEPENDENTLY ) { /* Only scale if first frame in packet */ round_loss = psEnc->sCmn.PacketLoss_perc + psEnc->sCmn.nFramesPerPacket; - psEnc->sCmn.indices.LTP_scaleIndex = (opus_int8)silk_LIMIT( - silk_SMULWB( silk_SMULBB( round_loss, psEncCtrl->LTPredCodGain_Q7 ), SILK_FIX_CONST( 0.1, 9 ) ), 0, 2 ); + if ( psEnc->sCmn.LBRR_flag ) { + /* LBRR reduces the effective loss. In practice, it does not square the loss because + losses aren't independent, but that still seems to work best. We also never go below 2%. */ + round_loss = 2 + silk_SMULBB( round_loss, round_loss ) / 100; + } + psEnc->sCmn.indices.LTP_scaleIndex = silk_SMULBB( psEncCtrl->LTPredCodGain_Q7, round_loss ) > silk_log2lin( 128*7 + 2900-psEnc->sCmn.SNR_dB_Q7 ); + psEnc->sCmn.indices.LTP_scaleIndex += silk_SMULBB( psEncCtrl->LTPredCodGain_Q7, round_loss ) > silk_log2lin( 128*7 + 3900-psEnc->sCmn.SNR_dB_Q7 ); } else { /* Default is minimum scaling */ psEnc->sCmn.indices.LTP_scaleIndex = 0; diff --git a/silk/float/LTP_scale_ctrl_FLP.c b/silk/float/LTP_scale_ctrl_FLP.c index 8dbe29d0..1fed0993 100644 --- a/silk/float/LTP_scale_ctrl_FLP.c +++ b/silk/float/LTP_scale_ctrl_FLP.c @@ -42,7 +42,13 @@ void silk_LTP_scale_ctrl_FLP( if( condCoding == CODE_INDEPENDENTLY ) { /* Only scale if first frame in packet */ round_loss = psEnc->sCmn.PacketLoss_perc + psEnc->sCmn.nFramesPerPacket; - psEnc->sCmn.indices.LTP_scaleIndex = (opus_int8)silk_LIMIT( round_loss * psEncCtrl->LTPredCodGain * 0.1f, 0.0f, 2.0f ); + if ( psEnc->sCmn.LBRR_flag ) { + /* LBRR reduces the effective loss. In practice, it does not square the loss because + losses aren't independent, but that still seems to work best. We also never go below 2%. */ + round_loss = 2 + silk_SMULBB( round_loss, round_loss) / 100; + } + psEnc->sCmn.indices.LTP_scaleIndex = silk_SMULBB( psEncCtrl->LTPredCodGain, round_loss ) > silk_log2lin( 2900 - psEnc->sCmn.SNR_dB_Q7 ); + psEnc->sCmn.indices.LTP_scaleIndex += silk_SMULBB( psEncCtrl->LTPredCodGain, round_loss ) > silk_log2lin( 3900 - psEnc->sCmn.SNR_dB_Q7 ); } else { /* Default is minimum scaling */ psEnc->sCmn.indices.LTP_scaleIndex = 0; -- cgit v1.2.3 From fd9c0f1e1f1b74c46c5872217e3289a9edf69d48 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Sun, 24 Jul 2022 02:14:53 -0400 Subject: More FEC tuning: lowering the LBRR bitrate a bit --- silk/control_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silk/control_codec.c b/silk/control_codec.c index 52aa8fde..784ffe66 100644 --- a/silk/control_codec.c +++ b/silk/control_codec.c @@ -415,7 +415,7 @@ static OPUS_INLINE opus_int silk_setup_LBRR( /* Previous packet did not have LBRR, and was therefore coded at a higher bitrate */ psEncC->LBRR_GainIncreases = 7; } else { - psEncC->LBRR_GainIncreases = silk_max_int( 7 - silk_SMULWB( (opus_int32)psEncC->PacketLoss_perc, SILK_FIX_CONST( 0.4, 16 ) ), 2 ); + psEncC->LBRR_GainIncreases = silk_max_int( 7 - silk_SMULWB( (opus_int32)psEncC->PacketLoss_perc, SILK_FIX_CONST( 0.2, 16 ) ), 3 ); } } -- cgit v1.2.3 From ab04fbb1b7d0b727636d28fc2cadb5df9febe515 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Sun, 24 Jul 2022 03:46:16 -0400 Subject: Smooth out the LBRR rate estimate Reduces fluctuations in the non-FEC target bitrate. --- silk/enc_API.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/silk/enc_API.c b/silk/enc_API.c index 55a33f37..548e0736 100644 --- a/silk/enc_API.c +++ b/silk/enc_API.c @@ -270,6 +270,7 @@ opus_int silk_Encode( /* O Returns error co psEnc->state_Fxx[ 0 ].sCmn.fs_kHz * 1000 ); ALLOC( buf, nSamplesFromInputMax, opus_int16 ); while( 1 ) { + int curr_nBitsUsedLBRR = 0; nSamplesToBuffer = psEnc->state_Fxx[ 0 ].sCmn.frame_length - psEnc->state_Fxx[ 0 ].sCmn.inputBufIx; nSamplesToBuffer = silk_min( nSamplesToBuffer, nSamplesToBufferMax ); nSamplesFromInput = silk_DIV32_16( nSamplesToBuffer * psEnc->state_Fxx[ 0 ].sCmn.API_fs_Hz, psEnc->state_Fxx[ 0 ].sCmn.fs_kHz * 1000 ); @@ -342,6 +343,7 @@ opus_int silk_Encode( /* O Returns error co opus_uint8 iCDF[ 2 ] = { 0, 0 }; iCDF[ 0 ] = 256 - silk_RSHIFT( 256, ( psEnc->state_Fxx[ 0 ].sCmn.nFramesPerPacket + 1 ) * encControl->nChannelsInternal ); ec_enc_icdf( psRangeEnc, 0, iCDF, 8 ); + curr_nBitsUsedLBRR = ec_tell( psRangeEnc ); /* Encode any LBRR data from previous packet */ /* Encode LBRR flags */ @@ -386,8 +388,7 @@ opus_int silk_Encode( /* O Returns error co for( n = 0; n < encControl->nChannelsInternal; n++ ) { silk_memset( psEnc->state_Fxx[ n ].sCmn.LBRR_flags, 0, sizeof( psEnc->state_Fxx[ n ].sCmn.LBRR_flags ) ); } - - psEnc->nBitsUsedLBRR = ec_tell( psRangeEnc ); + curr_nBitsUsedLBRR = ec_tell( psRangeEnc ) - curr_nBitsUsedLBRR; } silk_HP_variable_cutoff( psEnc->state_Fxx ); @@ -396,6 +397,16 @@ opus_int silk_Encode( /* O Returns error co nBits = silk_DIV32_16( silk_MUL( encControl->bitRate, encControl->payloadSize_ms ), 1000 ); /* Subtract bits used for LBRR */ if( !prefillFlag ) { + /* psEnc->nBitsUsedLBRR is an exponential moving average of the LBRR usage, + except that for the first LBRR frame it does no averaging and for the first + frame after after LBRR, it goes back to zero immediately. */ + if ( curr_nBitsUsedLBRR < 10 ) { + psEnc->nBitsUsedLBRR = 0; + } else if ( psEnc->nBitsUsedLBRR < 10) { + psEnc->nBitsUsedLBRR = curr_nBitsUsedLBRR; + } else { + psEnc->nBitsUsedLBRR = ( psEnc->nBitsUsedLBRR + curr_nBitsUsedLBRR ) / 2; + } nBits -= psEnc->nBitsUsedLBRR; } /* Divide by number of uncoded frames left in packet */ -- cgit v1.2.3 From 997fdf54e781ae1c04dee42018f35388a04fe483 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin Date: Thu, 4 Aug 2022 18:59:37 -0400 Subject: Change pitch scaling behavior wrt nFramesPerPacket Not sure if it was the original intent, but we now reduce the loss percentage threshold for pitch scaling as 1/nFramesPerPacket since only the first frame will have pitch scaling anyway. As a side effect, this brings back the original behavior of disabling pitch scaling for 0% loss. --- silk/fixed/LTP_scale_ctrl_FIX.c | 2 +- silk/float/LTP_scale_ctrl_FLP.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/silk/fixed/LTP_scale_ctrl_FIX.c b/silk/fixed/LTP_scale_ctrl_FIX.c index b3afb70b..db1016e0 100644 --- a/silk/fixed/LTP_scale_ctrl_FIX.c +++ b/silk/fixed/LTP_scale_ctrl_FIX.c @@ -42,7 +42,7 @@ void silk_LTP_scale_ctrl_FIX( if( condCoding == CODE_INDEPENDENTLY ) { /* Only scale if first frame in packet */ - round_loss = psEnc->sCmn.PacketLoss_perc + psEnc->sCmn.nFramesPerPacket; + round_loss = psEnc->sCmn.PacketLoss_perc * psEnc->sCmn.nFramesPerPacket; if ( psEnc->sCmn.LBRR_flag ) { /* LBRR reduces the effective loss. In practice, it does not square the loss because losses aren't independent, but that still seems to work best. We also never go below 2%. */ diff --git a/silk/float/LTP_scale_ctrl_FLP.c b/silk/float/LTP_scale_ctrl_FLP.c index 1fed0993..6f30ff09 100644 --- a/silk/float/LTP_scale_ctrl_FLP.c +++ b/silk/float/LTP_scale_ctrl_FLP.c @@ -41,7 +41,7 @@ void silk_LTP_scale_ctrl_FLP( if( condCoding == CODE_INDEPENDENTLY ) { /* Only scale if first frame in packet */ - round_loss = psEnc->sCmn.PacketLoss_perc + psEnc->sCmn.nFramesPerPacket; + round_loss = psEnc->sCmn.PacketLoss_perc * psEnc->sCmn.nFramesPerPacket; if ( psEnc->sCmn.LBRR_flag ) { /* LBRR reduces the effective loss. In practice, it does not square the loss because losses aren't independent, but that still seems to work best. We also never go below 2%. */ -- cgit v1.2.3 From bce1f392353d72d77d543bb2069a044ae1045e9d Mon Sep 17 00:00:00 2001 From: "Nathan E. Egge" Date: Sun, 4 Sep 2022 22:02:10 -0400 Subject: Fix typo in MacroDebug.h comment. Signed-off-by: Jean-Marc Valin --- silk/MacroDebug.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/silk/MacroDebug.h b/silk/MacroDebug.h index aceeef7e..3110da9a 100644 --- a/silk/MacroDebug.h +++ b/silk/MacroDebug.h @@ -829,7 +829,7 @@ static OPUS_INLINE opus_int32 silk_SUB_RSHIFT32_(opus_int32 a, opus_int32 b, opu static OPUS_INLINE opus_int32 silk_RSHIFT_ROUND_(opus_int32 a, opus_int32 shift, char *file, int line){ opus_int32 ret; ret = shift == 1 ? (a >> 1) + (a & 1) : ((a >> (shift - 1)) + 1) >> 1; - /* the marco definition can't handle a shift of zero */ + /* the macro definition can't handle a shift of zero */ if ( (shift <= 0) || (shift>31) || ((opus_int64)ret != ((opus_int64)a + ((opus_int64)1 << (shift - 1))) >> shift) ) { fprintf (stderr, "silk_RSHIFT_ROUND(%d, %d) in %s: line %d\n", a, shift, file, line); @@ -844,7 +844,7 @@ static OPUS_INLINE opus_int32 silk_RSHIFT_ROUND_(opus_int32 a, opus_int32 shift, #define silk_RSHIFT_ROUND64(a,b) silk_RSHIFT_ROUND64_((a), (b), __FILE__, __LINE__) static OPUS_INLINE opus_int64 silk_RSHIFT_ROUND64_(opus_int64 a, opus_int32 shift, char *file, int line){ opus_int64 ret; - /* the marco definition can't handle a shift of zero */ + /* the macro definition can't handle a shift of zero */ if ( (shift <= 0) || (shift>=64) ) { fprintf (stderr, "silk_RSHIFT_ROUND64(%lld, %d) in %s: line %d\n", (long long)a, shift, file, line); -- cgit v1.2.3