Added Error Checking in Ingress/Egress and Extra Unit Tests
Added error checking in AudioIngress and AudioEgress to detect situations where codecs have not been set; added additional unit tests for VoipCore
Bug: webrtc:11251
Change-Id: Ibd57e518892c76e2865b844ba866e380a565dd6b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180229
Commit-Queue: Tim Na <natim@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Tim Na <natim@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31874}
diff --git a/audio/voip/audio_channel.cc b/audio/voip/audio_channel.cc
index d9c89fc..43d4d0f 100644
--- a/audio/voip/audio_channel.cc
+++ b/audio/voip/audio_channel.cc
@@ -82,44 +82,50 @@
process_thread_->DeRegisterModule(rtp_rtcp_.get());
}
-void AudioChannel::StartSend() {
- egress_->StartSend();
+bool AudioChannel::StartSend() {
+ // If encoder has not been set, return false.
+ if (!egress_->StartSend()) {
+ return false;
+ }
// Start sending with RTP stack if it has not been sending yet.
- if (!rtp_rtcp_->Sending() && rtp_rtcp_->SetSendingStatus(true) != 0) {
- RTC_DLOG(LS_ERROR) << "StartSend() RTP/RTCP failed to start sending";
+ if (!rtp_rtcp_->Sending()) {
+ rtp_rtcp_->SetSendingStatus(true);
}
+ return true;
}
void AudioChannel::StopSend() {
egress_->StopSend();
- // If the channel is not playing and RTP stack is active then deactivate RTP
- // stack. SetSendingStatus(false) triggers the transmission of RTCP BYE
+ // Deactivate RTP stack when both sending and receiving are stopped.
+ // SetSendingStatus(false) triggers the transmission of RTCP BYE
// message to remote endpoint.
- if (!IsPlaying() && rtp_rtcp_->Sending() &&
- rtp_rtcp_->SetSendingStatus(false) != 0) {
- RTC_DLOG(LS_ERROR) << "StopSend() RTP/RTCP failed to stop sending";
+ if (!ingress_->IsPlaying() && rtp_rtcp_->Sending()) {
+ rtp_rtcp_->SetSendingStatus(false);
}
}
-void AudioChannel::StartPlay() {
- ingress_->StartPlay();
+bool AudioChannel::StartPlay() {
+ // If decoders have not been set, return false.
+ if (!ingress_->StartPlay()) {
+ return false;
+ }
// If RTP stack is not sending then start sending as in recv-only mode, RTCP
// receiver report is expected.
- if (!rtp_rtcp_->Sending() && rtp_rtcp_->SetSendingStatus(true) != 0) {
- RTC_DLOG(LS_ERROR) << "StartPlay() RTP/RTCP failed to start sending";
+ if (!rtp_rtcp_->Sending()) {
+ rtp_rtcp_->SetSendingStatus(true);
}
+ return true;
}
void AudioChannel::StopPlay() {
ingress_->StopPlay();
// Deactivate RTP stack only when both sending and receiving are stopped.
- if (!IsSendingMedia() && rtp_rtcp_->Sending() &&
- rtp_rtcp_->SetSendingStatus(false) != 0) {
- RTC_DLOG(LS_ERROR) << "StopPlay() RTP/RTCP failed to stop sending";
+ if (!rtp_rtcp_->SendingMedia() && rtp_rtcp_->Sending()) {
+ rtp_rtcp_->SetSendingStatus(false);
}
}
diff --git a/audio/voip/audio_channel.h b/audio/voip/audio_channel.h
index 659e990..12138ee 100644
--- a/audio/voip/audio_channel.h
+++ b/audio/voip/audio_channel.h
@@ -45,9 +45,11 @@
ChannelId GetId() const { return id_; }
// APIs to start/stop audio channel on each direction.
- void StartSend();
+ // StartSend/StartPlay returns false if encoder/decoders
+ // have not been set, respectively.
+ bool StartSend();
void StopSend();
- void StartPlay();
+ bool StartPlay();
void StopPlay();
// APIs relayed to AudioEgress.
diff --git a/audio/voip/audio_egress.cc b/audio/voip/audio_egress.cc
index 305f712..90e069e 100644
--- a/audio/voip/audio_egress.cc
+++ b/audio/voip/audio_egress.cc
@@ -56,8 +56,13 @@
audio_coding_->SetEncoder(std::move(encoder));
}
-void AudioEgress::StartSend() {
+bool AudioEgress::StartSend() {
+ if (!GetEncoderFormat()) {
+ RTC_DLOG(LS_WARNING) << "Send codec has not been set yet";
+ return false;
+ }
rtp_rtcp_->SetSendingMediaStatus(true);
+ return true;
}
void AudioEgress::StopSend() {
diff --git a/audio/voip/audio_egress.h b/audio/voip/audio_egress.h
index 8ec048f..6b2d374 100644
--- a/audio/voip/audio_egress.h
+++ b/audio/voip/audio_egress.h
@@ -59,8 +59,9 @@
// Start or stop sending operation of AudioEgress. This will start/stop
// the RTP stack also causes encoder queue thread to start/stop
- // processing input audio samples.
- void StartSend();
+ // processing input audio samples. StartSend will return false if
+ // a send codec has not been set.
+ bool StartSend();
void StopSend();
// Query the state of the RTP stack. This returns true if StartSend()
diff --git a/audio/voip/audio_ingress.cc b/audio/voip/audio_ingress.cc
index 560055d..0bddb42 100644
--- a/audio/voip/audio_ingress.cc
+++ b/audio/voip/audio_ingress.cc
@@ -103,6 +103,18 @@
: AudioMixer::Source::AudioFrameInfo::kNormal;
}
+bool AudioIngress::StartPlay() {
+ {
+ MutexLock lock(&lock_);
+ if (receive_codec_info_.empty()) {
+ RTC_DLOG(LS_WARNING) << "Receive codecs have not been set yet";
+ return false;
+ }
+ }
+ playing_ = true;
+ return true;
+}
+
void AudioIngress::SetReceiveCodecs(
const std::map<int, SdpAudioFormat>& codecs) {
{
diff --git a/audio/voip/audio_ingress.h b/audio/voip/audio_ingress.h
index 5a8df21..d09de60 100644
--- a/audio/voip/audio_ingress.h
+++ b/audio/voip/audio_ingress.h
@@ -51,7 +51,7 @@
~AudioIngress() override;
// Start or stop receiving operation of AudioIngress.
- void StartPlay() { playing_ = true; }
+ bool StartPlay();
void StopPlay() {
playing_ = false;
output_audio_level_.ResetLevelFullRange();
diff --git a/audio/voip/test/voip_core_unittest.cc b/audio/voip/test/voip_core_unittest.cc
index c1969d6..b97b637 100644
--- a/audio/voip/test/voip_core_unittest.cc
+++ b/audio/voip/test/voip_core_unittest.cc
@@ -96,5 +96,33 @@
EXPECT_FALSE(voip_core_->StartPlayout(*channel));
}
+TEST_F(VoipCoreTest, StartSendAndPlayoutWithoutSettingCodec) {
+ auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de);
+ EXPECT_TRUE(channel);
+
+ // Call StartSend and StartPlayout without setting send/receive
+ // codec. Code should see that codecs aren't set and return false.
+ EXPECT_FALSE(voip_core_->StartSend(*channel));
+ EXPECT_FALSE(voip_core_->StartPlayout(*channel));
+
+ voip_core_->ReleaseChannel(*channel);
+}
+
+TEST_F(VoipCoreTest, StopSendAndPlayoutWithoutStarting) {
+ auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de);
+ EXPECT_TRUE(channel);
+
+ voip_core_->SetSendCodec(*channel, kPcmuPayload, kPcmuFormat);
+ voip_core_->SetReceiveCodecs(*channel, {{kPcmuPayload, kPcmuFormat}});
+
+ // Call StopSend and StopPlayout without starting them in
+ // the first place. Should see that it is already in the
+ // stopped state and return true.
+ EXPECT_TRUE(voip_core_->StopSend(*channel));
+ EXPECT_TRUE(voip_core_->StopPlayout(*channel));
+
+ voip_core_->ReleaseChannel(*channel);
+}
+
} // namespace
} // namespace webrtc
diff --git a/audio/voip/voip_core.cc b/audio/voip/voip_core.cc
index 7292644..6390223 100644
--- a/audio/voip/voip_core.cc
+++ b/audio/voip/voip_core.cc
@@ -238,12 +238,10 @@
bool VoipCore::StartSend(ChannelId channel) {
auto audio_channel = GetChannel(channel);
- if (!audio_channel) {
+ if (!audio_channel || !audio_channel->StartSend()) {
return false;
}
- audio_channel->StartSend();
-
return UpdateAudioTransportWithSenders();
}
@@ -260,12 +258,10 @@
bool VoipCore::StartPlayout(ChannelId channel) {
auto audio_channel = GetChannel(channel);
- if (!audio_channel) {
+ if (!audio_channel || !audio_channel->StartPlay()) {
return false;
}
- audio_channel->StartPlay();
-
if (!audio_device_module_->Playing()) {
if (audio_device_module_->InitPlayout() != 0) {
RTC_LOG(LS_ERROR) << "InitPlayout failed";
