Prevent Opus DTX from generating intermittent noise during silence

webrtc/modules/audio_coding/codecs/opus/opus_interface.c

Index: webrtc/modules/audio_coding/codecs/opus/opus_interface.c
diff --git a/webrtc/modules/audio_coding/codecs/opus/opus_interface.c b/webrtc/modules/audio_coding/codecs/opus/opus_interface.c
index 2ac53736650907ca4d54cfb1e1ccc42646f1116a..6c51f940f5e2df45e82d5e80fe752771896eebc2 100644
--- a/webrtc/modules/audio_coding/codecs/opus/opus_interface.c
+++ b/webrtc/modules/audio_coding/codecs/opus/opus_interface.c
@@ -29,48 +29,74 @@ enum {
 
   /* Default frame size, 20 ms @ 48 kHz, in samples (for one channel). */
   kWebRtcOpusDefaultFrameSize = 960,
+
+  // Maximum number of consecutive zeros, beyond or equal to which DTX can fail.
+  kZeroBreakCount = 158,
 };
 
 int16_t WebRtcOpus_EncoderCreate(OpusEncInst** inst,
                                  int32_t channels,
                                  int32_t application) {
-  OpusEncInst* state;
-  if (inst != NULL) {
-    state = (OpusEncInst*) calloc(1, sizeof(OpusEncInst));
-    if (state) {
-      int opus_app;
-      switch (application) {
-        case 0: {
-          opus_app = OPUS_APPLICATION_VOIP;
-          break;
-        }
-        case 1: {
-          opus_app = OPUS_APPLICATION_AUDIO;
-          break;
-        }
-        default: {
-          free(state);
-          return -1;
-        }
-      }
+  if (inst == NULL)
+    return -1;
 
-      int error;
-      state->encoder = opus_encoder_create(48000, channels, opus_app,
-                                           &error);
-      state->in_dtx_mode = 0;
-      if (error == OPUS_OK && state->encoder != NULL) {
-        *inst = state;
-        return 0;
-      }
-      free(state);
+  OpusEncInst* state = (OpusEncInst*)calloc(1, sizeof(OpusEncInst));
+  if (!state)
+    return -1;
+
+  // Allocate buffer for input signal.
+  state->buffer =
+      (int16_t*)calloc(channels * 48 * kWebRtcOpusMaxEncodeFrameSizeMs,
+                       sizeof(int16_t));
+  if (!state->buffer) {
+    free(state);
+    return -1;
+  }
+
+  // Allocate buffer for zero counters.
+  state->zero_counts = (size_t*)calloc(channels, sizeof(size_t));
+  if (!state->zero_counts) {
+    free(state->buffer);
+    free(state);
+    return -1;
+  }
+
+  int opus_app;
+  switch (application) {
+    case 0: {
+      opus_app = OPUS_APPLICATION_VOIP;
+      break;
+    }
+    case 1: {
+      opus_app = OPUS_APPLICATION_AUDIO;
+      break;
+    }
+    default: {
+      WebRtcOpus_EncoderFree(state);
+      return -1;
     }
   }
-  return -1;
+
+  int error;
+  state->encoder = opus_encoder_create(48000, channels, opus_app,
+                                       &error);
+  if (error != OPUS_OK || state->encoder == NULL) {
+    WebRtcOpus_EncoderFree(state);
+    return -1;
+  }
+
+  state->in_dtx_mode = 0;
+  state->channels = channels;
+
+  *inst = state;
+  return 0;
 }
 
 int16_t WebRtcOpus_EncoderFree(OpusEncInst* inst) {
   if (inst) {
     opus_encoder_destroy(inst->encoder);
+    free(inst->zero_counts);
+    free(inst->buffer);
     free(inst);
     return 0;
   } else {
@@ -89,8 +115,29 @@ int WebRtcOpus_Encode(OpusEncInst* inst,
     return -1;
   }
 
+  const int channels = inst->channels;
+  int16_t* pointer = inst->buffer;
+  // Break long consecutive zeros by forcing a "1" every |kZeroBreakCount|
+  // samples.
+  memcpy(pointer, audio_in, samples * channels * sizeof(int16_t));
+  if (inst->in_dtx_mode) {
+    for (size_t i = 0; i < samples; ++i) {
+      for (int c = 0; c < channels; ++c, ++pointer) {
+        if (*pointer == 0) {
+          ++inst->zero_counts[c];
+          if (inst->zero_counts[c] == kZeroBreakCount) {
+            *pointer = 1;
+            inst->zero_counts[c] = 0;
+          }
+        } else {
+          inst->zero_counts[c] = 0;
+        }
+      }
+    }
+  }

[tlegrand-webrtc, 2015/10/27 13:51:22]

I think there is a much simpler way of doing this, that doesn't require the two
extra vectors. My thinking it that we simply check if the input is all-zeros or
not (per channel, I guess), and if it is, we replace it with a predefined
vector. Or do we need to keep track of what happened in the previous frame?

[minyue-webrtc, 2015/10/27 18:29:01]

There is a problem with checking if whole frame are zeros. Since a frame
contains 960 samples (48 Khz * 20 ms). But I found that the longest zero segment
that guarantee the problem not to happen is less than 158 (or now I found 157)
samples, much smaller than a frame.

So both in-frame and cross-frame zeros shall be considered.

But of course, there is a trade-off between complexity and quality. Maybe we can
still allow some noise pumping as long as it does not last for long period. Then
checking all-zeros of frame might be ok.

Another point to mention is that the signals are interleaved in stereo case.
Therefore the check of zeros in each channel may still need a sample-by-sample
check (we cannot use memcmp), then the complexity may be quite the same.

[tlegrand-webrtc, 2015/10/28 13:20:37]

Ah, I see now why you need to check for shorter periods of all zeros. As per our
offline discussion, consider changing to stack allocated memory.

+
   res = opus_encode(inst->encoder,
-                    (const opus_int16*)audio_in,
+                    inst->buffer,
                     (int)samples,
                     encoded,
                     (opus_int32)length_encoded_buffer);