<div dir="ltr">Thanks Andrew, this has now been merged. <br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 5, 2017 at 9:36 PM Andrew Larkin <<a href="mailto:andrewl@arcadius.com.au">andrewl@arcadius.com.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">While porting opus 1.2.1 to esp32, my compiler had a grumble (error) about<br>
possible use of uninitialized variables in decide_dtx_mode()<br>
<br>
Existing code:<br>
<br>
--------SNIP---------<br>
/* Decides if DTX should be turned on (=1) or off (=0) */<br>
static int decide_dtx_mode(float activity_probability,    /* probability<br>
that current frame contains speech/music */<br>
                           int *nb_no_activity_frames,    /* number of<br>
consecutive frames with no activity */<br>
                           opus_val32 peak_signal_energy, /* peak energy of<br>
desired signal detected so far */<br>
                           const opus_val16 *pcm,         /* input pcm<br>
signal */<br>
                           int frame_size,                /* frame size */<br>
                           int channels,<br>
                           int is_silence,                 /* only digital<br>
silence detected in this frame */<br>
                           int arch<br>
                          )<br>
{<br>
   int is_noise;<br>
   opus_val32 noise_energy;<br>
   int is_sufficiently_quiet;<br>
<br>
   if (!is_silence)<br>
   {<br>
      is_noise = activity_probability < DTX_ACTIVITY_THRESHOLD;<br>
      if (is_noise)<br>
      {<br>
         noise_energy = compute_frame_energy(pcm, frame_size, channels,<br>
arch);<br>
         is_sufficiently_quiet = peak_signal_energy >= (PSEUDO_SNR_THRESHOLD<br>
* noise_energy);<br>
      }<br>
   }<br>
<br>
   if (is_silence || (is_noise && is_sufficiently_quiet))<br>
   {<br>
      /* The number of consecutive DTX frames should be within the allowed<br>
bounds */<br>
      (*nb_no_activity_frames)++;<br>
<br>
      if (*nb_no_activity_frames > NB_SPEECH_FRAMES_BEFORE_DTX)<br>
      {<br>
         if (*nb_no_activity_frames <= (NB_SPEECH_FRAMES_BEFORE_DTX +<br>
MAX_CONSECUTIVE_DTX))<br>
            /* Valid frame for DTX! */<br>
            return 1;<br>
         else<br>
            (*nb_no_activity_frames) = NB_SPEECH_FRAMES_BEFORE_DTX;<br>
      }<br>
   } else<br>
      (*nb_no_activity_frames) = 0;<br>
<br>
   return 0;<br>
}<br>
--------SNIP---------<br>
<br>
The problem is in the case of function parameter is_silence != 0, the code:<br>
<br>
if (is_silence || (is_noise && is_sufficiently_quiet))<br>
<br>
is using uninitialized local variables "is_noise" and<br>
"is_sufficiently_quiet". The compiler complaint is valid.<br>
<br>
While this will have no functional effect due to the state of "is_silence",<br>
the problem can be addressed by restructuring the code.  The proposed new<br>
code also eliminates unnecessary temporary variables and reduces operators<br>
as well.<br>
<br>
New code:<br>
<br>
--------SNIP---------<br>
/* Decides if DTX should be turned on (=1) or off (=0) */<br>
static int decide_dtx_mode(float activity_probability,    /* probability<br>
that current frame contains speech/music */<br>
                           int *nb_no_activity_frames,    /* number of<br>
consecutive frames with no activity */<br>
                           opus_val32 peak_signal_energy, /* peak energy of<br>
desired signal detected so far */<br>
                           const opus_val16 *pcm,         /* input pcm<br>
signal */<br>
                           int frame_size,                /* frame size */<br>
                           int channels,<br>
                           int is_silence,                 /* only digital<br>
silence detected in this frame */<br>
                           int arch<br>
                          )<br>
{<br>
   opus_val32 noise_energy;<br>
<br>
   if (!is_silence)<br>
   {<br>
      if( activity_probability < DTX_ACTIVITY_THRESHOLD )   /* is_noise */<br>
      {<br>
         noise_energy = compute_frame_energy(pcm, frame_size, channels,<br>
arch);<br>
         is_silence = peak_signal_energy >= (PSEUDO_SNR_THRESHOLD *<br>
noise_energy);  /* but is_sufficiently_quiet */<br>
      }<br>
   }<br>
<br>
   if (is_silence)<br>
   {<br>
<br>
      /* The number of consecutive DTX frames should be within the allowed<br>
bounds */<br>
      (*nb_no_activity_frames)++;<br>
<br>
      if (*nb_no_activity_frames > NB_SPEECH_FRAMES_BEFORE_DTX)<br>
      {<br>
         if (*nb_no_activity_frames <= (NB_SPEECH_FRAMES_BEFORE_DTX +<br>
MAX_CONSECUTIVE_DTX))<br>
            /* Valid frame for DTX! */<br>
            return 1;<br>
         else<br>
            (*nb_no_activity_frames) = NB_SPEECH_FRAMES_BEFORE_DTX;<br>
      }<br>
   } else<br>
      (*nb_no_activity_frames) = 0;<br>
<br>
   return 0;<br>
}<br>
--------SNIP---------<br>
<br>
<br>
_______________________________________________<br>
opus mailing list<br>
<a href="mailto:opus@xiph.org" target="_blank">opus@xiph.org</a><br>
<a href="http://lists.xiph.org/mailman/listinfo/opus" rel="noreferrer" target="_blank">http://lists.xiph.org/mailman/listinfo/opus</a><br>
</blockquote></div></div>