From 8b85de2ba1a8885b70bf9fe8beadc54c5c405335 Mon Sep 17 00:00:00 2001 From: solenberg Date: Mon, 16 Nov 2015 09:48:04 -0800 Subject: [PATCH] Converted a bunch of error checking in Audio[Receive|Send]Stream to RTC_CHECKs instead. They should never fail. BUG=webrtc:4690 Review URL: https://codereview.webrtc.org/1442483003 Cr-Commit-Position: refs/heads/master@{#10654} --- webrtc/audio/audio_receive_stream.cc | 57 +++++++------ webrtc/audio/audio_receive_stream_unittest.cc | 2 - webrtc/audio/audio_send_stream.cc | 81 +++++++++---------- webrtc/audio/audio_send_stream_unittest.cc | 2 - 4 files changed, 65 insertions(+), 77 deletions(-) diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc index 8caac6f87e..bdccea2c37 100644 --- a/webrtc/audio/audio_receive_stream.cc +++ b/webrtc/audio/audio_receive_stream.cc @@ -109,13 +109,12 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const { ScopedVoEInterface rtp(voice_engine); ScopedVoEInterface sync(voice_engine); ScopedVoEInterface volume(voice_engine); - unsigned int ssrc = 0; + webrtc::CallStatistics call_stats = {0}; + int error = rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats); + RTC_DCHECK_EQ(0, error); webrtc::CodecInst codec_inst = {0}; - // Only collect stats if we have seen some traffic with the SSRC. - if (rtp->GetRemoteSSRC(config_.voe_channel_id, ssrc) == -1 || - rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats) == -1 || - codec->GetRecCodec(config_.voe_channel_id, codec_inst) == -1) { + if (codec->GetRecCodec(config_.voe_channel_id, codec_inst) == -1) { return stats; } @@ -123,6 +122,7 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const { stats.packets_rcvd = call_stats.packetsReceived; stats.packets_lost = call_stats.cumulativeLost; stats.fraction_lost = Q8ToFloat(call_stats.fractionLost); + stats.capture_start_ntp_time_ms = call_stats.capture_start_ntp_time_ms_; if (codec_inst.pltype != -1) { stats.codec_name = codec_inst.plname; } @@ -139,35 +139,33 @@ webrtc::AudioReceiveStream::Stats AudioReceiveStream::GetStats() const { } { unsigned int level = 0; - if (volume->GetSpeechOutputLevelFullRange(config_.voe_channel_id, level) != - -1) { - stats.audio_level = static_cast(level); - } + error = volume->GetSpeechOutputLevelFullRange(config_.voe_channel_id, + level); + RTC_DCHECK_EQ(0, error); + stats.audio_level = static_cast(level); } + // Get jitter buffer and total delay (alg + jitter + playout) stats. webrtc::NetworkStatistics ns = {0}; - if (neteq->GetNetworkStatistics(config_.voe_channel_id, ns) != -1) { - // Get jitter buffer and total delay (alg + jitter + playout) stats. - stats.jitter_buffer_ms = ns.currentBufferSize; - stats.jitter_buffer_preferred_ms = ns.preferredBufferSize; - stats.expand_rate = Q14ToFloat(ns.currentExpandRate); - stats.speech_expand_rate = Q14ToFloat(ns.currentSpeechExpandRate); - stats.secondary_decoded_rate = Q14ToFloat(ns.currentSecondaryDecodedRate); - stats.accelerate_rate = Q14ToFloat(ns.currentAccelerateRate); - stats.preemptive_expand_rate = Q14ToFloat(ns.currentPreemptiveRate); - } + error = neteq->GetNetworkStatistics(config_.voe_channel_id, ns); + RTC_DCHECK_EQ(0, error); + stats.jitter_buffer_ms = ns.currentBufferSize; + stats.jitter_buffer_preferred_ms = ns.preferredBufferSize; + stats.expand_rate = Q14ToFloat(ns.currentExpandRate); + stats.speech_expand_rate = Q14ToFloat(ns.currentSpeechExpandRate); + stats.secondary_decoded_rate = Q14ToFloat(ns.currentSecondaryDecodedRate); + stats.accelerate_rate = Q14ToFloat(ns.currentAccelerateRate); + stats.preemptive_expand_rate = Q14ToFloat(ns.currentPreemptiveRate); webrtc::AudioDecodingCallStats ds; - if (neteq->GetDecodingCallStatistics(config_.voe_channel_id, &ds) != -1) { - stats.decoding_calls_to_silence_generator = ds.calls_to_silence_generator; - stats.decoding_calls_to_neteq = ds.calls_to_neteq; - stats.decoding_normal = ds.decoded_normal; - stats.decoding_plc = ds.decoded_plc; - stats.decoding_cng = ds.decoded_cng; - stats.decoding_plc_cng = ds.decoded_plc_cng; - } - - stats.capture_start_ntp_time_ms = call_stats.capture_start_ntp_time_ms_; + error = neteq->GetDecodingCallStatistics(config_.voe_channel_id, &ds); + RTC_DCHECK_EQ(0, error); + stats.decoding_calls_to_silence_generator = ds.calls_to_silence_generator; + stats.decoding_calls_to_neteq = ds.calls_to_neteq; + stats.decoding_normal = ds.decoded_normal; + stats.decoding_plc = ds.decoded_plc; + stats.decoding_cng = ds.decoded_cng; + stats.decoding_plc_cng = ds.decoded_plc_cng; return stats; } @@ -205,7 +203,6 @@ bool AudioReceiveStream::DeliverRtp(const uint8_t* packet, // thread. Then this check can be enabled. // RTC_DCHECK(!thread_checker_.CalledOnValidThread()); RTPHeader header; - if (!rtp_header_parser_->Parse(packet, length, &header)) { return false; } diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index edd804fe51..715b52a592 100644 --- a/webrtc/audio/audio_receive_stream_unittest.cc +++ b/webrtc/audio/audio_receive_stream_unittest.cc @@ -75,8 +75,6 @@ struct ConfigHelper { using testing::DoAll; using testing::SetArgPointee; using testing::SetArgReferee; - EXPECT_CALL(voice_engine_, GetRemoteSSRC(kChannelId, _)) - .WillOnce(DoAll(SetArgReferee<1>(0), Return(0))); EXPECT_CALL(voice_engine_, GetRTCPStatistics(kChannelId, _)) .WillOnce(DoAll(SetArgReferee<1>(kCallStats), Return(0))); EXPECT_CALL(voice_engine_, GetRecCodec(kChannelId, _)) diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 14112deb9a..a7b98c7670 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -119,17 +119,20 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const { ScopedVoEInterface codec(voice_engine()); ScopedVoEInterface rtp(voice_engine()); ScopedVoEInterface 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; - } + webrtc::CallStatistics call_stats = {0}; + int error = rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats); + RTC_DCHECK_EQ(0, error); 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(solenberg): [was ajm]: Re-enable this metric once we have a reliable + // implementation. + stats.aec_quality_min = -1; webrtc::CodecInst codec_inst = {0}; if (codec->GetSendCodec(config_.voe_channel_id, codec_inst) != -1) { @@ -138,53 +141,45 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const { // Get data from the last remote RTCP report. std::vector 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; + error = rtp->GetRemoteRTCPReportBlocks(config_.voe_channel_id, &blocks); + RTC_DCHECK_EQ(0, error); + 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(level); - } + error = volume->GetSpeechInputLevelFullRange(level); + RTC_DCHECK_EQ(0, error); + stats.audio_level = static_cast(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) { + error = processing->GetEcMetricsStatus(echo_metrics_on); + RTC_DCHECK_EQ(0, error); + 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; - } + error = processing->GetEcDelayMetrics(median, std, dummy); + RTC_DCHECK_EQ(0, error); + 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. @@ -192,10 +187,10 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const { 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; - } + error = processing->GetEchoMetrics(erl, erle, dummy1, dummy2); + RTC_DCHECK_EQ(0, error); + stats.echo_return_loss = erl; + stats.echo_return_loss_enhancement = erle; } internal::AudioState* audio_state = diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 1801e9df11..27d40297fe 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -86,8 +86,6 @@ struct ConfigHelper { block.fraction_lost = 0; report_blocks.push_back(block); // Duplicate SSRC, bad fraction_lost. - EXPECT_CALL(voice_engine_, GetLocalSSRC(kChannelId, _)) - .WillRepeatedly(DoAll(SetArgReferee<1>(0), Return(0))); EXPECT_CALL(voice_engine_, GetRTCPStatistics(kChannelId, _)) .WillRepeatedly(DoAll(SetArgReferee<1>(kCallStats), Return(0))); EXPECT_CALL(voice_engine_, GetSendCodec(kChannelId, _))