Converted a bunch of error checking in Audio[Receive|Send]Stream to RTC_CHECKs instead. They should…

webrtc/audio/audio_send_stream.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 96 matching lines @@
 }
 
 webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   webrtc::AudioSendStream::Stats stats;
   stats.local_ssrc = config_.rtp.ssrc;
   ScopedVoEInterface<VoEAudioProcessing> processing(voice_engine());
   ScopedVoEInterface<VoECodec> codec(voice_engine());
   ScopedVoEInterface<VoERTP_RTCP> rtp(voice_engine());
   ScopedVoEInterface<VoEVolumeControl> volume(voice_engine());
-  unsigned int ssrc = 0;
+
   webrtc::CallStatistics call_stats = {0};
-  // TODO(solenberg): Change error code checking to RTC_CHECK_EQ(..., -1), if
-  //                  possible...
-  if (rtp->GetLocalSSRC(config_.voe_channel_id, ssrc) == -1 ||
-      rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats) == -1) {
-    return stats;
-  }
-
+  RTC_CHECK_EQ(0, rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats));
   stats.bytes_sent = call_stats.bytesSent;
   stats.packets_sent = call_stats.packetsSent;
+  // RTT isn't known until a RTCP report is received. Until then, VoiceEngine
+  // returns 0 to indicate an error value.
+  if (call_stats.rttMs > 0) {
+    stats.rtt_ms = call_stats.rttMs;
+  }
+  // TODO(ajm): Re-enable this metric once we have a reliable implementation.

[tommi, 2015/11/12 16:24:43]

assign this to peah? (or yourself?)

[the sun, 2015/11/12 16:38:17]

Done.

+  stats.aec_quality_min = -1;
 
   webrtc::CodecInst codec_inst = {0};
   if (codec->GetSendCodec(config_.voe_channel_id, codec_inst) != -1) {
     RTC_DCHECK_NE(codec_inst.pltype, -1);
     stats.codec_name = codec_inst.plname;
 
     // Get data from the last remote RTCP report.
     std::vector<webrtc::ReportBlock> blocks;
-    if (rtp->GetRemoteRTCPReportBlocks(config_.voe_channel_id, &blocks) != -1) {
-      for (const webrtc::ReportBlock& block : blocks) {
-        // Lookup report for send ssrc only.
-        if (block.source_SSRC == stats.local_ssrc) {
-          stats.packets_lost = block.cumulative_num_packets_lost;
-          stats.fraction_lost = Q8ToFloat(block.fraction_lost);
-          stats.ext_seqnum = block.extended_highest_sequence_number;
-          // Convert samples to milliseconds.
-          if (codec_inst.plfreq / 1000 > 0) {
-            stats.jitter_ms =
-                block.interarrival_jitter / (codec_inst.plfreq / 1000);
-          }
-          break;
+    RTC_CHECK_EQ(0, rtp->GetRemoteRTCPReportBlocks(config_.voe_channel_id,

[tommi, 2015/11/12 16:24:43]

is it preferable to do this at every call site where we know that this should
never fail, or should this be done in the implementation itself? (and thus not
have to worry about the call sites and eventually change the return type to
void?)

[the sun, 2015/11/12 16:38:17]

It's a public API, used by 3rd parties. I can guarantee it will always work here
because of how the other VoE APIs are being called in WVoE/MC and the rest of
Call. We cannot make such assumptions about other users and thus can't put the
check in the implementation itself.

Soon this will be history...

+                                                   &blocks));
+    for (const webrtc::ReportBlock& block : blocks) {
+      // Lookup report for send ssrc only.
+      if (block.source_SSRC == stats.local_ssrc) {
+        stats.packets_lost = block.cumulative_num_packets_lost;
+        stats.fraction_lost = Q8ToFloat(block.fraction_lost);
+        stats.ext_seqnum = block.extended_highest_sequence_number;
+        // Convert samples to milliseconds.
+        if (codec_inst.plfreq / 1000 > 0) {
+          stats.jitter_ms =
+              block.interarrival_jitter / (codec_inst.plfreq / 1000);
         }
+        break;
       }
     }
   }
 
-  // RTT isn't known until a RTCP report is received. Until then, VoiceEngine
-  // returns 0 to indicate an error value.
-  if (call_stats.rttMs > 0) {
-    stats.rtt_ms = call_stats.rttMs;
-  }
-
   // Local speech level.
   {
     unsigned int level = 0;
-    if (volume->GetSpeechInputLevelFullRange(level) != -1) {
-      stats.audio_level = static_cast<int32_t>(level);
-    }
+    RTC_CHECK_EQ(0, volume->GetSpeechInputLevelFullRange(level));

[tommi, 2015/11/12 16:24:43]

same question here and I guess other places.

[the sun, 2015/11/12 16:38:17]

Acknowledged.

+    stats.audio_level = static_cast<int32_t>(level);
   }
 
-  // TODO(ajm): Re-enable this metric once we have a reliable implementation.
-  stats.aec_quality_min = -1;
-
   bool echo_metrics_on = false;
-  if (processing->GetEcMetricsStatus(echo_metrics_on) != -1 &&
-      echo_metrics_on) {
+  RTC_CHECK_EQ(0, processing->GetEcMetricsStatus(echo_metrics_on));
+  if (echo_metrics_on) {
     // These can also be negative, but in practice -1 is only used to signal
     // insufficient data, since the resolution is limited to multiples of 4 ms.
     int median = -1;
     int std = -1;
     float dummy = 0.0f;
-    if (processing->GetEcDelayMetrics(median, std, dummy) != -1) {
-      stats.echo_delay_median_ms = median;
-      stats.echo_delay_std_ms = std;
-    }
+    RTC_CHECK_EQ(0, processing->GetEcDelayMetrics(median, std, dummy));
+    stats.echo_delay_median_ms = median;
+    stats.echo_delay_std_ms = std;
 
     // These can take on valid negative values, so use the lowest possible level
     // as default rather than -1.
     int erl = -100;
     int erle = -100;
     int dummy1 = 0;
     int dummy2 = 0;
-    if (processing->GetEchoMetrics(erl, erle, dummy1, dummy2) != -1) {
-      stats.echo_return_loss = erl;
-      stats.echo_return_loss_enhancement = erle;
-    }
+    RTC_CHECK_EQ(0, processing->GetEchoMetrics(erl, erle, dummy1, dummy2));
+    stats.echo_return_loss = erl;
+    stats.echo_return_loss_enhancement = erle;
   }
 
   internal::AudioState* audio_state =
       static_cast<internal::AudioState*>(audio_state_.get());
   stats.typing_noise_detected = audio_state->typing_noise_detected();
 
   return stats;
 }
 
 const webrtc::AudioSendStream::Config& AudioSendStream::config() const {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   return config_;
 }
 
 VoiceEngine* AudioSendStream::voice_engine() const {
   internal::AudioState* audio_state =
       static_cast<internal::AudioState*>(audio_state_.get());
   VoiceEngine* voice_engine = audio_state->voice_engine();
   RTC_DCHECK(voice_engine);
   return voice_engine;
 }
 }  // namespace internal
 }  // namespace webrtc