[opus] Suggested patch, opus_encoder.c, decide_dtx_mode()

Andrew Larkin andrewl at arcadius.com.au
Thu Jul 6 04:28:52 UTC 2017


While porting opus 1.2.1 to esp32, my compiler had a grumble (error) about
possible use of uninitialized variables in decide_dtx_mode()

Existing code:

--------SNIP---------
/* Decides if DTX should be turned on (=1) or off (=0) */
static int decide_dtx_mode(float activity_probability,    /* probability
that current frame contains speech/music */
                           int *nb_no_activity_frames,    /* number of
consecutive frames with no activity */
                           opus_val32 peak_signal_energy, /* peak energy of
desired signal detected so far */
                           const opus_val16 *pcm,         /* input pcm
signal */
                           int frame_size,                /* frame size */
                           int channels,
                           int is_silence,                 /* only digital
silence detected in this frame */
                           int arch
                          )
{
   int is_noise;
   opus_val32 noise_energy;
   int is_sufficiently_quiet;

   if (!is_silence)
   {
      is_noise = activity_probability < DTX_ACTIVITY_THRESHOLD;
      if (is_noise)
      {
         noise_energy = compute_frame_energy(pcm, frame_size, channels,
arch);
         is_sufficiently_quiet = peak_signal_energy >= (PSEUDO_SNR_THRESHOLD
* noise_energy);
      }
   }

   if (is_silence || (is_noise && is_sufficiently_quiet))
   {
      /* The number of consecutive DTX frames should be within the allowed
bounds */
      (*nb_no_activity_frames)++;

      if (*nb_no_activity_frames > NB_SPEECH_FRAMES_BEFORE_DTX)
      {
         if (*nb_no_activity_frames <= (NB_SPEECH_FRAMES_BEFORE_DTX +
MAX_CONSECUTIVE_DTX))
            /* Valid frame for DTX! */
            return 1;
         else
            (*nb_no_activity_frames) = NB_SPEECH_FRAMES_BEFORE_DTX;
      }
   } else
      (*nb_no_activity_frames) = 0;

   return 0;
}
--------SNIP---------

The problem is in the case of function parameter is_silence != 0, the code:

if (is_silence || (is_noise && is_sufficiently_quiet))

is using uninitialized local variables "is_noise" and
"is_sufficiently_quiet". The compiler complaint is valid.

While this will have no functional effect due to the state of "is_silence",
the problem can be addressed by restructuring the code.  The proposed new
code also eliminates unnecessary temporary variables and reduces operators
as well.

New code:

--------SNIP---------
/* Decides if DTX should be turned on (=1) or off (=0) */
static int decide_dtx_mode(float activity_probability,    /* probability
that current frame contains speech/music */
                           int *nb_no_activity_frames,    /* number of
consecutive frames with no activity */
                           opus_val32 peak_signal_energy, /* peak energy of
desired signal detected so far */
                           const opus_val16 *pcm,         /* input pcm
signal */
                           int frame_size,                /* frame size */
                           int channels,
                           int is_silence,                 /* only digital
silence detected in this frame */
                           int arch
                          )
{
   opus_val32 noise_energy;

   if (!is_silence)
   {
      if( activity_probability < DTX_ACTIVITY_THRESHOLD )   /* is_noise */
      {
         noise_energy = compute_frame_energy(pcm, frame_size, channels,
arch);
         is_silence = peak_signal_energy >= (PSEUDO_SNR_THRESHOLD *
noise_energy);  /* but is_sufficiently_quiet */
      }
   }

   if (is_silence)
   {
       
      /* The number of consecutive DTX frames should be within the allowed
bounds */
      (*nb_no_activity_frames)++;

      if (*nb_no_activity_frames > NB_SPEECH_FRAMES_BEFORE_DTX)
      {
         if (*nb_no_activity_frames <= (NB_SPEECH_FRAMES_BEFORE_DTX +
MAX_CONSECUTIVE_DTX))
            /* Valid frame for DTX! */
            return 1;
         else
            (*nb_no_activity_frames) = NB_SPEECH_FRAMES_BEFORE_DTX;
      }
   } else
      (*nb_no_activity_frames) = 0;

   return 0;
}
--------SNIP---------




More information about the opus mailing list