Fixed off-by-one issue in Silk DTX

[gustafullberg]

Fixes an issue in Silk mode causing the codec going into DTX one frame too early.

#72

[jmvalin]

I don't know that one behaviour is better than the other. Is the current code causing issues?

[jmvalin]

Nm, just read your explanation in issue #72

[gustafullberg]

Is something holding the merge of this PL back? Is more information needed?
IMHO, this is clearly a bug with a clean (two-character) fix suggested.

[jmvalin]

Sorry for the delay. Looking at this now, do you already have some test code/sample for this or should I build one myself. (your patch looks sane, I just want to make sure I test it properly)

[gustafullberg]

No worries.
I don't think I have a test wired up at the moment. I'm a developer of WebRTC, and discovered the bug when a test didn't show the expected DTX pattern in lower complexity modes (Silk-only mode). I tried to describe the pattern in #72 .
If this turns out to be hard to reproduce I'll try to put something together.

[gustafullberg]

I wrote a tiny test that shows the issue. It basically encodes zeros with DTX enabled. Please try it out at different complexity levels with and without my pull request.

#include <stdio.h>
#include <opus/opus.h>

int main(int argc, char **argv)
{
  int error;
  OpusEncoder *enc = opus_encoder_create(24000, 1, OPUS_APPLICATION_VOIP, &error);
  opus_encoder_ctl(enc, OPUS_SET_COMPLEXITY(6));
  opus_encoder_ctl(enc, OPUS_SET_DTX(1));

  short audio_frame[480] = { 0 };
  int num_dtx = 0, num_coded = 0;
  for(int i = 0; i < 60; i++) {
    int packet_size = 1024*10;
    unsigned char packet[packet_size];
    int len = opus_encode(enc, audio_frame, 240, packet, packet_size);
    if(len <= 2) {
      // DTX
      num_dtx++;
      if(num_coded > 0) {
        printf("Number of consecutive encoded frames %d\n", num_coded);
        num_coded = 0;
      }
    } else {
      // Encoded
      num_coded++;
      if(num_dtx > 0) {
        printf("Number of consecutive DTX frames %d\n", num_dtx);
        num_dtx = 0;
      }
    }
  }
  return 0;
}

Output at complexity level 7:

Number of consecutive encoded frames 10
Number of consecutive DTX frames 20
Number of consecutive encoded frames 1
Number of consecutive DTX frames 20
Number of consecutive encoded frames 1

Output at complexity level 6 without pull request:

Number of consecutive encoded frames 9
Number of consecutive DTX frames 21
Number of consecutive encoded frames 1
Number of consecutive DTX frames 20
Number of consecutive encoded frames 1

Output at complexity level 6 with pull request

Number of consecutive encoded frames 10
Number of consecutive DTX frames 20
Number of consecutive encoded frames 1
Number of consecutive DTX frames 20
Number of consecutive encoded frames 1

As you can see from the output at complexity level <= 6 without the PL we enter DTX one frame too early and stay in DTX one frame too long.

The intended behavior is pretty clear in the code (from silk/define.h):

/* DTX settings */
#define NB_SPEECH_FRAMES_BEFORE_DTX             10      /* eq 200 ms */
#define MAX_CONSECUTIVE_DTX                     20      /* eq 400 ms */

[jmvalin]

merged