From cd78f3976ea44797a10486592880511227d31594 Mon Sep 17 00:00:00 2001 From: Jean-Marc Valin <jmvalin@jmvalin.ca> Date: Wed, 23 May 2018 16:59:23 -0400 Subject: [PATCH] Fixes a SILK bandwidth switching regression The bug was triggered because f982b84d started using prefill for SILK bandwidth changes, which reinitialized the encoder state and prevented the variable lowpass from working properly. To fix the problem, we preserve the sampling rate and variable low-pass when prefilling. --- silk/control_audio_bandwidth.c | 16 +++++++++++----- silk/enc_API.c | 10 ++++++++++ silk/structs.h | 1 + src/opus_encoder.c | 7 +++++-- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/silk/control_audio_bandwidth.c b/silk/control_audio_bandwidth.c index 4f9bc5cbd..f6d22d839 100644 --- a/silk/control_audio_bandwidth.c +++ b/silk/control_audio_bandwidth.c @@ -39,9 +39,15 @@ opus_int silk_control_audio_bandwidth( ) { opus_int fs_kHz; + opus_int orig_kHz; opus_int32 fs_Hz; - fs_kHz = psEncC->fs_kHz; + orig_kHz = psEncC->fs_kHz; + /* Handle a bandwidth-switching reset where we need to be aware what the last sampling rate was. */ + if( orig_kHz == 0 ) { + orig_kHz = psEncC->sLP.saved_fs_kHz; + } + fs_kHz = orig_kHz; fs_Hz = silk_SMULBB( fs_kHz, 1000 ); if( fs_Hz == 0 ) { /* Encoder has just been initialized */ @@ -61,7 +67,7 @@ opus_int silk_control_audio_bandwidth( } if( psEncC->allow_bandwidth_switch || encControl->opusCanSwitch ) { /* Check if we should switch down */ - if( silk_SMULBB( psEncC->fs_kHz, 1000 ) > psEncC->desiredInternal_fs_Hz ) + if( silk_SMULBB( orig_kHz, 1000 ) > psEncC->desiredInternal_fs_Hz ) { /* Switch down */ if( psEncC->sLP.mode == 0 ) { @@ -76,7 +82,7 @@ opus_int silk_control_audio_bandwidth( psEncC->sLP.mode = 0; /* Switch to a lower sample frequency */ - fs_kHz = psEncC->fs_kHz == 16 ? 12 : 8; + fs_kHz = orig_kHz == 16 ? 12 : 8; } else { if( psEncC->sLP.transition_frame_no <= 0 ) { encControl->switchReady = 1; @@ -90,12 +96,12 @@ opus_int silk_control_audio_bandwidth( } else /* Check if we should switch up */ - if( silk_SMULBB( psEncC->fs_kHz, 1000 ) < psEncC->desiredInternal_fs_Hz ) + if( silk_SMULBB( orig_kHz, 1000 ) < psEncC->desiredInternal_fs_Hz ) { /* Switch up */ if( encControl->opusCanSwitch ) { /* Switch to a higher sample frequency */ - fs_kHz = psEncC->fs_kHz == 8 ? 12 : 16; + fs_kHz = orig_kHz == 8 ? 12 : 16; /* New transition */ psEncC->sLP.transition_frame_no = 0; diff --git a/silk/enc_API.c b/silk/enc_API.c index 7ae31a9e0..55a33f37e 100644 --- a/silk/enc_API.c +++ b/silk/enc_API.c @@ -200,15 +200,25 @@ opus_int silk_Encode( /* O Returns error co tot_blocks = ( nBlocksOf10ms > 1 ) ? nBlocksOf10ms >> 1 : 1; curr_block = 0; if( prefillFlag ) { + silk_LP_state save_LP; /* Only accept input length of 10 ms */ if( nBlocksOf10ms != 1 ) { celt_assert( 0 ); RESTORE_STACK; return SILK_ENC_INPUT_INVALID_NO_OF_SAMPLES; } + if ( prefillFlag == 2 ) { + save_LP = psEnc->state_Fxx[ 0 ].sCmn.sLP; + /* Save the sampling rate so the bandwidth switching code can keep handling transitions. */ + save_LP.saved_fs_kHz = psEnc->state_Fxx[ 0 ].sCmn.fs_kHz; + } /* Reset Encoder */ for( n = 0; n < encControl->nChannelsInternal; n++ ) { ret = silk_init_encoder( &psEnc->state_Fxx[ n ], psEnc->state_Fxx[ n ].sCmn.arch ); + /* Restore the variable LP state. */ + if ( prefillFlag == 2 ) { + psEnc->state_Fxx[ n ].sCmn.sLP = save_LP; + } celt_assert( !ret ); } tmp_payloadSize_ms = encControl->payloadSize_ms; diff --git a/silk/structs.h b/silk/structs.h index 4ff590b04..3380c757b 100644 --- a/silk/structs.h +++ b/silk/structs.h @@ -78,6 +78,7 @@ typedef struct { opus_int32 In_LP_State[ 2 ]; /* Low pass filter state */ opus_int32 transition_frame_no; /* Counter which is mapped to a cut-off frequency */ opus_int mode; /* Operating mode, <0: switch down, >0: switch up; 0: do nothing */ + opus_int32 saved_fs_kHz; /* If non-zero, holds the last sampling rate before a bandwidth switching reset. */ } silk_LP_state; /* Structure containing NLSF codebook */ diff --git a/src/opus_encoder.c b/src/opus_encoder.c index 0be65a3d9..e6cca9013 100644 --- a/src/opus_encoder.c +++ b/src/opus_encoder.c @@ -1599,7 +1599,8 @@ opus_int32 opus_encode_native(OpusEncoder *st, const opus_val16 *pcm, int frame_ redundancy = 1; celt_to_silk = 1; st->silk_bw_switch = 0; - prefill=1; + /* Do a prefill without reseting the sampling rate control. */ + prefill=2; } /* If we decided to go with CELT, make sure redundancy is off, no matter what @@ -1825,7 +1826,9 @@ opus_int32 opus_encode_native(OpusEncoder *st, const opus_val16 *pcm, int frame_ for (i=0;i<st->encoder_buffer*st->channels;i++) pcm_silk[i] = FLOAT2INT16(st->delay_buffer[i]); #endif - silk_Encode( silk_enc, &st->silk_mode, pcm_silk, st->encoder_buffer, NULL, &zero, 1, activity ); + silk_Encode( silk_enc, &st->silk_mode, pcm_silk, st->encoder_buffer, NULL, &zero, prefill, activity ); + /* Prevent a second switch in the real encode call. */ + st->silk_mode.opusCanSwitch = 0; } #ifdef FIXED_POINT -- GitLab