NetEq: Implement muted output

webrtc/modules/audio_coding/neteq/neteq_impl.cc

Index: webrtc/modules/audio_coding/neteq/neteq_impl.cc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
index 945ac5ce161bd15ffb0fbf9cb20ea905fa533021..c68730dd7e98e8fbaeec489ec1e4bf50177424a4 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -108,7 +108,8 @@ NetEqImpl::NetEqImpl(const NetEq::Config& config,
       background_noise_mode_(config.background_noise_mode),
       playout_mode_(config.playout_mode),
       enable_fast_accelerate_(config.enable_fast_accelerate),
-      nack_enabled_(false) {
+      nack_enabled_(false),
+      enable_muted_state_(config.enable_muted_state) {
   LOG(LS_INFO) << "NetEq config: " << config.ToString();
   int fs = config.sample_rate_hz;
   if (fs != 8000 && fs != 16000 && fs != 32000 && fs != 48000) {
@@ -205,10 +206,10 @@ void SetAudioFrameActivityAndType(bool vad_enabled,
 }
 }  // namespace
 

[minyue-webrtc, 2016/05/11 11:29:54]

how about allowing the bool* to be nullptr. calling does not need to define for
it if it is not cared.

[hlundin-webrtc, 2016/05/12 07:44:40]

That is going to be a bit tricky, I think. What if you have enabled muted state
(through config.enable_muted_state), but you don't provide a valid pointer?
Should NetEq still be allowed to enter muted state, and leave the output samples
possibly undefined?

[minyue-webrtc, 2016/05/12 10:56:06]

good point.

-int NetEqImpl::GetAudio(AudioFrame* audio_frame) {
+int NetEqImpl::GetAudio(AudioFrame* audio_frame, bool* muted_output) {
   TRACE_EVENT0("webrtc", "NetEqImpl::GetAudio");
   rtc::CritScope lock(&crit_sect_);
-  int error = GetAudioInternal(audio_frame);
+  int error = GetAudioInternal(audio_frame, muted_output);
   RTC_DCHECK_EQ(
       audio_frame->sample_rate_hz_,
       rtc::checked_cast<int>(audio_frame->samples_per_channel_ * 100));
@@ -809,13 +810,31 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
   return 0;
 }
 
-int NetEqImpl::GetAudioInternal(AudioFrame* audio_frame) {
+int NetEqImpl::GetAudioInternal(AudioFrame* audio_frame, bool* muted_output) {
   PacketList packet_list;
   DtmfEvent dtmf_event;
   Operations operation;
   bool play_dtmf;
+  *muted_output = false;
   tick_timer_->Increment();
   stats_.IncreaseCounter(output_size_samples_, fs_hz_);
+
+  // Check for muted state.
+  if (enable_muted_state_ && expand_->Muted() && packet_buffer_->Empty()) {

[minyue-webrtc, 2016/05/11 11:29:53]

what if packet_buffer_ are not empty but all old packets?

[hlundin-webrtc, 2016/05/12 07:44:40]

I think we want to exit muted state for this. It is safer, I think. It should be
rare, so exiting muted state for this reason is an acceptable cost IMO.

+    RTC_DCHECK_EQ(last_mode_, kModeExpand);

[minyue-webrtc, 2016/05/11 11:29:53]

why should this be a CHECK but not a if condition?

[hlundin-webrtc, 2016/05/12 07:44:40]

The logic in NetEq should lead to this being always true. Otherwise, we are
doing something wrong in the programming. Hence, DCHECK.

+    playout_timestamp_ += static_cast<uint32_t>(output_size_samples_);
+    audio_frame->sample_rate_hz_ = fs_hz_;
+    audio_frame->samples_per_channel_ = output_size_samples_;
+    audio_frame->timestamp_ =

[hlundin-webrtc, 2016/05/10 12:28:15]

The setting of timestamp is identical to line 1028 at the end of
GetAudioInternal.

+        first_packet_
+            ? 0
+            : timestamp_scaler_->ToExternal(playout_timestamp_) -
+                  static_cast<uint32_t>(audio_frame->samples_per_channel_);
+    audio_frame->num_channels_ = sync_buffer_->Channels();
+    *muted_output = true;
+    return 0;
+  }
+
   int return_value = GetDecision(&operation, &packet_list, &dtmf_event,
                                  &play_dtmf);
   if (return_value != 0) {