From b223cb60e9974638d4401bd668ce87672eb8ed21 Mon Sep 17 00:00:00 2001 From: Tim Na Date: Fri, 20 Nov 2020 09:34:47 -0800 Subject: [PATCH] Defining API result types on VoIP API Bug: webrtc:12193 Change-Id: I6f5ffd82cc838e6982257781f225f9d8159e6b82 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/193720 Commit-Queue: Tim Na Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#32656} --- api/voip/voip_base.h | 69 ++++-- api/voip/voip_codec.h | 14 +- api/voip/voip_dtmf.h | 21 +- api/voip/voip_engine.h | 31 ++- api/voip/voip_network.h | 20 +- api/voip/voip_statistics.h | 10 +- api/voip/voip_volume_control.h | 25 +- audio/voip/test/voip_core_unittest.cc | 106 ++++----- audio/voip/voip_core.cc | 219 +++++++++++------- audio/voip/voip_core.h | 57 ++--- .../androidvoip/jni/android_voip_client.cc | 25 +- 11 files changed, 350 insertions(+), 247 deletions(-) diff --git a/api/voip/voip_base.h b/api/voip/voip_base.h index ef83b51ed8..c5f54aa9e9 100644 --- a/api/voip/voip_base.h +++ b/api/voip/voip_base.h @@ -35,6 +35,21 @@ class Transport; enum class ChannelId : int {}; +enum class VoipResult { + // kOk indicates the function was successfully invoked with no error. + kOk, + // kInvalidArgument indicates the caller specified an invalid argument, such + // as an invalid ChannelId. + kInvalidArgument, + // kFailedPrecondition indicates that the operation was failed due to not + // satisfying prerequisite such as not setting codec type before sending. + kFailedPrecondition, + // kInternal is used to indicate various internal failures that are not the + // caller's fault. Further detail is commented on each function that uses this + // return value. + kInternal, +}; + class VoipBase { public: // Creates a channel. @@ -46,40 +61,48 @@ class VoipBase { // and injection for incoming RTP from remote endpoint is handled via // VoipNetwork interface. |local_ssrc| is optional and when local_ssrc is not // set, some random value will be used by voip engine. - // Returns value is optional as to indicate the failure to create channel. - virtual absl::optional CreateChannel( - Transport* transport, - absl::optional local_ssrc) = 0; + // Returns a ChannelId created for caller to handle subsequent Channel + // operations. + virtual ChannelId CreateChannel(Transport* transport, + absl::optional local_ssrc) = 0; // Releases |channel_id| that no longer has any use. - virtual void ReleaseChannel(ChannelId channel_id) = 0; + // Returns following VoipResult; + // kOk - |channel_id| is released. + // kInvalidArgument - |channel_id| is invalid. + // kInternal - Fails to stop audio output device. + virtual VoipResult ReleaseChannel(ChannelId channel_id) = 0; - // Starts sending on |channel_id|. This will start microphone if not started - // yet. Returns false if initialization has failed on selected microphone - // device. API is subject to expand to reflect error condition to application - // later. - virtual bool StartSend(ChannelId channel_id) = 0; + // Starts sending on |channel_id|. This starts microphone if not started yet. + // Returns following VoipResult; + // kOk - Channel successfully started to send. + // kInvalidArgument - |channel_id| is invalid. + // kFailedPrecondition - Missing prerequisite on VoipCodec::SetSendCodec. + // kInternal - initialization has failed on selected microphone. + virtual VoipResult StartSend(ChannelId channel_id) = 0; // Stops sending on |channel_id|. If this is the last active channel, it will // stop microphone input from underlying audio platform layer. - // Returns false if termination logic has failed on selected microphone - // device. API is subject to expand to reflect error condition to application - // later. - virtual bool StopSend(ChannelId channel_id) = 0; + // Returns following VoipResult; + // kOk - Channel successfully stopped to send. + // kInvalidArgument - |channel_id| is invalid. + // kInternal - Failed to stop the active microphone device. + virtual VoipResult StopSend(ChannelId channel_id) = 0; // Starts playing on speaker device for |channel_id|. // This will start underlying platform speaker device if not started. - // Returns false if initialization has failed - // on selected speaker device. API is subject to expand to reflect error - // condition to application later. - virtual bool StartPlayout(ChannelId channel_id) = 0; + // Returns following VoipResult; + // kOk - Channel successfully started to play out. + // kInvalidArgument - |channel_id| is invalid. + // kFailedPrecondition - Missing prerequisite on VoipCodec::SetReceiveCodecs. + // kInternal - Failed to initializate the selected speaker device. + virtual VoipResult StartPlayout(ChannelId channel_id) = 0; // Stops playing on speaker device for |channel_id|. - // If this is the last active channel playing, then it will stop speaker - // from the platform layer. - // Returns false if termination logic has failed on selected speaker device. - // API is subject to expand to reflect error condition to application later. - virtual bool StopPlayout(ChannelId channel_id) = 0; + // Returns following VoipResult; + // kOk - Channel successfully stopped t play out. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult StopPlayout(ChannelId channel_id) = 0; protected: virtual ~VoipBase() = default; diff --git a/api/voip/voip_codec.h b/api/voip/voip_codec.h index eb42c449d9..fec3827dbe 100644 --- a/api/voip/voip_codec.h +++ b/api/voip/voip_codec.h @@ -29,15 +29,21 @@ namespace webrtc { class VoipCodec { public: // Set encoder type here along with its payload type to use. - virtual void SetSendCodec(ChannelId channel_id, - int payload_type, - const SdpAudioFormat& encoder_spec) = 0; + // Returns following VoipResult; + // kOk - sending codec is set as provided. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult SetSendCodec(ChannelId channel_id, + int payload_type, + const SdpAudioFormat& encoder_spec) = 0; // Set decoder payload type here. In typical offer and answer model, // this should be called after payload type has been agreed in media // session. Note that payload type can differ with same codec in each // direction. - virtual void SetReceiveCodecs( + // Returns following VoipResult; + // kOk - receiving codecs are set as provided. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult SetReceiveCodecs( ChannelId channel_id, const std::map& decoder_specs) = 0; diff --git a/api/voip/voip_dtmf.h b/api/voip/voip_dtmf.h index 56817bae50..a7367bed53 100644 --- a/api/voip/voip_dtmf.h +++ b/api/voip/voip_dtmf.h @@ -43,9 +43,12 @@ class VoipDtmf { // Register the payload type and sample rate for DTMF (RFC 4733) payload. // Must be called exactly once prior to calling SendDtmfEvent after payload // type has been negotiated with remote. - virtual void RegisterTelephoneEventType(ChannelId channel_id, - int rtp_payload_type, - int sample_rate_hz) = 0; + // Returns following VoipResult; + // kOk - telephone event type is registered as provided. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult RegisterTelephoneEventType(ChannelId channel_id, + int rtp_payload_type, + int sample_rate_hz) = 0; // Send DTMF named event as specified by // https://tools.ietf.org/html/rfc4733#section-3.2 @@ -53,10 +56,14 @@ class VoipDtmf { // in place of real RTP packets instead. // Must be called after RegisterTelephoneEventType and VoipBase::StartSend // have been called. - // Returns true if the requested DTMF event is successfully scheduled. - virtual bool SendDtmfEvent(ChannelId channel_id, - DtmfEvent dtmf_event, - int duration_ms) = 0; + // Returns following VoipResult; + // kOk - requested DTMF event is successfully scheduled. + // kInvalidArgument - |channel_id| is invalid. + // kFailedPrecondition - Missing prerequisite on RegisterTelephoneEventType + // or sending state. + virtual VoipResult SendDtmfEvent(ChannelId channel_id, + DtmfEvent dtmf_event, + int duration_ms) = 0; protected: virtual ~VoipDtmf() = default; diff --git a/api/voip/voip_engine.h b/api/voip/voip_engine.h index 69c0a8504f..d223f6ad6c 100644 --- a/api/voip/voip_engine.h +++ b/api/voip/voip_engine.h @@ -23,7 +23,7 @@ class VoipVolumeControl; // VoipEngine is the main interface serving as the entry point for all VoIP // APIs. A single instance of VoipEngine should suffice the most of the need for // typical VoIP applications as it handles multiple media sessions including a -// specialized session type like ad-hoc mesh conferencing. Below example code +// specialized session type like ad-hoc conference. Below example code // describes the typical sequence of API usage. Each API header contains more // description on what the methods are used for. // @@ -38,36 +38,35 @@ class VoipVolumeControl; // config.audio_processing = AudioProcessingBuilder().Create(); // // auto voip_engine = CreateVoipEngine(std::move(config)); -// if (!voip_engine) return some_failure; // // auto& voip_base = voip_engine->Base(); // auto& voip_codec = voip_engine->Codec(); // auto& voip_network = voip_engine->Network(); // -// absl::optional channel = -// voip_base.CreateChannel(&app_transport_); -// if (!channel) return some_failure; +// ChannelId channel = voip_base.CreateChannel(&app_transport_); // // // After SDP offer/answer, set payload type and codecs that have been // // decided through SDP negotiation. -// voip_codec.SetSendCodec(*channel, ...); -// voip_codec.SetReceiveCodecs(*channel, ...); +// // VoipResult handling omitted here. +// voip_codec.SetSendCodec(channel, ...); +// voip_codec.SetReceiveCodecs(channel, ...); // // // Start sending and playing RTP on voip channel. -// voip_base.StartSend(*channel); -// voip_base.StartPlayout(*channel); +// // VoipResult handling omitted here. +// voip_base.StartSend(channel); +// voip_base.StartPlayout(channel); // // // Inject received RTP/RTCP through VoipNetwork interface. -// voip_network.ReceivedRTPPacket(*channel, ...); -// voip_network.ReceivedRTCPPacket(*channel, ...); +// // VoipResult handling omitted here. +// voip_network.ReceivedRTPPacket(channel, ...); +// voip_network.ReceivedRTCPPacket(channel, ...); // // // Stop and release voip channel. -// voip_base.StopSend(*channel); -// voip_base.StopPlayout(*channel); -// voip_base.ReleaseChannel(*channel); +// // VoipResult handling omitted here. +// voip_base.StopSend(channel); +// voip_base.StopPlayout(channel); +// voip_base.ReleaseChannel(channel); // -// Current VoipEngine defines three sub-API classes and is subject to expand in -// near future. class VoipEngine { public: virtual ~VoipEngine() = default; diff --git a/api/voip/voip_network.h b/api/voip/voip_network.h index c49c7695b9..c820ca04a3 100644 --- a/api/voip/voip_network.h +++ b/api/voip/voip_network.h @@ -18,20 +18,22 @@ namespace webrtc { // VoipNetwork interface provides any network related interfaces such as // processing received RTP/RTCP packet from remote endpoint. This interface -// requires a ChannelId created via VoipBase interface. Note that using invalid -// (previously released) ChannelId will silently fail these API calls as it -// would have released underlying audio components. It's anticipated that caller -// may be using different thread for network I/O where released channel id is -// still used to input incoming RTP packets in which case we should silently -// ignore. The interface is subjected to expand as needed in near future. +// requires a ChannelId created via VoipBase interface. class VoipNetwork { public: // The data received from the network including RTP header is passed here. - virtual void ReceivedRTPPacket(ChannelId channel_id, - rtc::ArrayView rtp_packet) = 0; + // Returns following VoipResult; + // kOk - received RTP packet is processed. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult ReceivedRTPPacket( + ChannelId channel_id, + rtc::ArrayView rtp_packet) = 0; // The data received from the network including RTCP header is passed here. - virtual void ReceivedRTCPPacket( + // Returns following VoipResult; + // kOk - received RTCP packet is processed. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult ReceivedRTCPPacket( ChannelId channel_id, rtc::ArrayView rtcp_packet) = 0; diff --git a/api/voip/voip_statistics.h b/api/voip/voip_statistics.h index cf01e95e9e..08f4cb75a4 100644 --- a/api/voip/voip_statistics.h +++ b/api/voip/voip_statistics.h @@ -30,10 +30,12 @@ struct IngressStatistics { // the jitter buffer (NetEq) performance. class VoipStatistics { public: - // Gets the audio ingress statistics. Returns absl::nullopt when channel_id is - // invalid. - virtual absl::optional GetIngressStatistics( - ChannelId channel_id) = 0; + // Gets the audio ingress statistics by |ingress_stats| reference. + // Returns following VoipResult; + // kOk - successfully set provided IngressStatistics reference. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult GetIngressStatistics(ChannelId channel_id, + IngressStatistics& ingress_stats) = 0; protected: virtual ~VoipStatistics() = default; diff --git a/api/voip/voip_volume_control.h b/api/voip/voip_volume_control.h index 54e446715e..d91eabc5a9 100644 --- a/api/voip/voip_volume_control.h +++ b/api/voip/voip_volume_control.h @@ -36,17 +36,24 @@ class VoipVolumeControl { // Mute/unmutes the microphone input sample before encoding process. Note that // mute doesn't affect audio input level and energy values as input sample is // silenced after the measurement. - virtual void SetInputMuted(ChannelId channel_id, bool enable) = 0; + // Returns following VoipResult; + // kOk - input source muted or unmuted as provided by |enable|. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult SetInputMuted(ChannelId channel_id, bool enable) = 0; - // Gets the microphone volume info. - // Returns absl::nullopt if |channel_id| is invalid. - virtual absl::optional GetInputVolumeInfo( - ChannelId channel_id) = 0; + // Gets the microphone volume info via |volume_info| reference. + // Returns following VoipResult; + // kOk - successfully set provided input volume info. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult GetInputVolumeInfo(ChannelId channel_id, + VolumeInfo& volume_info) = 0; - // Gets the speaker volume info. - // Returns absl::nullopt if |channel_id| is invalid. - virtual absl::optional GetOutputVolumeInfo( - ChannelId channel_id) = 0; + // Gets the speaker volume info via |volume_info| reference. + // Returns following VoipResult; + // kOk - successfully set provided output volume info. + // kInvalidArgument - |channel_id| is invalid. + virtual VoipResult GetOutputVolumeInfo(ChannelId channel_id, + VolumeInfo& volume_info) = 0; protected: virtual ~VoipVolumeControl() = default; diff --git a/audio/voip/test/voip_core_unittest.cc b/audio/voip/test/voip_core_unittest.cc index 9763d588d5..8ab67b7360 100644 --- a/audio/voip/test/voip_core_unittest.cc +++ b/audio/voip/test/voip_core_unittest.cc @@ -69,19 +69,19 @@ TEST_F(VoipCoreTest, BasicVoipCoreOperation) { EXPECT_CALL(*audio_device_, StartPlayout()).WillOnce(Return(0)); auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de); - EXPECT_TRUE(channel); - voip_core_->SetSendCodec(*channel, kPcmuPayload, kPcmuFormat); - voip_core_->SetReceiveCodecs(*channel, {{kPcmuPayload, kPcmuFormat}}); + voip_core_->SetSendCodec(channel, kPcmuPayload, kPcmuFormat); + voip_core_->SetReceiveCodecs(channel, {{kPcmuPayload, kPcmuFormat}}); - EXPECT_TRUE(voip_core_->StartSend(*channel)); - EXPECT_TRUE(voip_core_->StartPlayout(*channel)); + EXPECT_EQ(voip_core_->StartSend(channel), VoipResult::kOk); + EXPECT_EQ(voip_core_->StartPlayout(channel), VoipResult::kOk); - voip_core_->RegisterTelephoneEventType(*channel, kPcmuPayload, + voip_core_->RegisterTelephoneEventType(channel, kPcmuPayload, kPcmuSampleRateHz); - EXPECT_TRUE(voip_core_->SendDtmfEvent(*channel, kDtmfEventCode, - kDtmfEventDurationMs)); + EXPECT_EQ( + voip_core_->SendDtmfEvent(channel, kDtmfEventCode, kDtmfEventDurationMs), + VoipResult::kOk); // Program mock as operational that is ready to be stopped. EXPECT_CALL(*audio_device_, Recording()).WillOnce(Return(true)); @@ -89,30 +89,32 @@ TEST_F(VoipCoreTest, BasicVoipCoreOperation) { EXPECT_CALL(*audio_device_, StopRecording()).WillOnce(Return(0)); EXPECT_CALL(*audio_device_, StopPlayout()).WillOnce(Return(0)); - EXPECT_TRUE(voip_core_->StopSend(*channel)); - EXPECT_TRUE(voip_core_->StopPlayout(*channel)); - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->StopSend(channel), VoipResult::kOk); + EXPECT_EQ(voip_core_->StopPlayout(channel), VoipResult::kOk); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); } TEST_F(VoipCoreTest, ExpectFailToUseReleasedChannelId) { auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de); - EXPECT_TRUE(channel); // Release right after creation. - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); // Now use released channel. - // These should be no-op. - voip_core_->SetSendCodec(*channel, kPcmuPayload, kPcmuFormat); - voip_core_->SetReceiveCodecs(*channel, {{kPcmuPayload, kPcmuFormat}}); - voip_core_->RegisterTelephoneEventType(*channel, kPcmuPayload, - kPcmuSampleRateHz); - - EXPECT_FALSE(voip_core_->StartSend(*channel)); - EXPECT_FALSE(voip_core_->StartPlayout(*channel)); - EXPECT_FALSE(voip_core_->SendDtmfEvent(*channel, kDtmfEventCode, - kDtmfEventDurationMs)); + EXPECT_EQ(voip_core_->SetSendCodec(channel, kPcmuPayload, kPcmuFormat), + VoipResult::kInvalidArgument); + EXPECT_EQ( + voip_core_->SetReceiveCodecs(channel, {{kPcmuPayload, kPcmuFormat}}), + VoipResult::kInvalidArgument); + EXPECT_EQ(voip_core_->RegisterTelephoneEventType(channel, kPcmuPayload, + kPcmuSampleRateHz), + VoipResult::kInvalidArgument); + EXPECT_EQ(voip_core_->StartSend(channel), VoipResult::kInvalidArgument); + EXPECT_EQ(voip_core_->StartPlayout(channel), VoipResult::kInvalidArgument); + EXPECT_EQ( + voip_core_->SendDtmfEvent(channel, kDtmfEventCode, kDtmfEventDurationMs), + VoipResult::kInvalidArgument); } TEST_F(VoipCoreTest, SendDtmfEventWithoutRegistering) { @@ -122,64 +124,65 @@ TEST_F(VoipCoreTest, SendDtmfEventWithoutRegistering) { EXPECT_CALL(*audio_device_, StartRecording()).WillOnce(Return(0)); auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de); - EXPECT_TRUE(channel); - voip_core_->SetSendCodec(*channel, kPcmuPayload, kPcmuFormat); + voip_core_->SetSendCodec(channel, kPcmuPayload, kPcmuFormat); - EXPECT_TRUE(voip_core_->StartSend(*channel)); + EXPECT_EQ(voip_core_->StartSend(channel), VoipResult::kOk); // Send Dtmf event without registering beforehand, thus payload - // type is not set and false is expected. - EXPECT_FALSE(voip_core_->SendDtmfEvent(*channel, kDtmfEventCode, - kDtmfEventDurationMs)); + // type is not set and kFailedPrecondition is expected. + EXPECT_EQ( + voip_core_->SendDtmfEvent(channel, kDtmfEventCode, kDtmfEventDurationMs), + VoipResult::kFailedPrecondition); // Program mock as sending and is ready to be stopped. EXPECT_CALL(*audio_device_, Recording()).WillOnce(Return(true)); EXPECT_CALL(*audio_device_, StopRecording()).WillOnce(Return(0)); - EXPECT_TRUE(voip_core_->StopSend(*channel)); - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->StopSend(channel), VoipResult::kOk); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); } TEST_F(VoipCoreTest, SendDtmfEventWithoutStartSend) { auto channel = voip_core_->CreateChannel(&transport_, 0xdeadc0de); - EXPECT_TRUE(channel); - voip_core_->RegisterTelephoneEventType(*channel, kPcmuPayload, + voip_core_->RegisterTelephoneEventType(channel, kPcmuPayload, kPcmuSampleRateHz); // Send Dtmf event without calling StartSend beforehand, thus - // Dtmf events cannot be sent and false is expected. - EXPECT_FALSE(voip_core_->SendDtmfEvent(*channel, kDtmfEventCode, - kDtmfEventDurationMs)); + // Dtmf events cannot be sent and kFailedPrecondition is expected. + EXPECT_EQ( + voip_core_->SendDtmfEvent(channel, kDtmfEventCode, kDtmfEventDurationMs), + VoipResult::kFailedPrecondition); - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); } 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)); + EXPECT_EQ(voip_core_->StartSend(channel), VoipResult::kFailedPrecondition); + EXPECT_EQ(voip_core_->StartPlayout(channel), VoipResult::kFailedPrecondition); - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); } 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}}); + EXPECT_EQ(voip_core_->SetSendCodec(channel, kPcmuPayload, kPcmuFormat), + VoipResult::kOk); + EXPECT_EQ( + voip_core_->SetReceiveCodecs(channel, {{kPcmuPayload, kPcmuFormat}}), + VoipResult::kOk); // 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)); + EXPECT_EQ(voip_core_->StopSend(channel), VoipResult::kOk); + EXPECT_EQ(voip_core_->StopPlayout(channel), VoipResult::kOk); - voip_core_->ReleaseChannel(*channel); + EXPECT_EQ(voip_core_->ReleaseChannel(channel), VoipResult::kOk); } // This tests correctness on ProcessThread usage where we expect the first/last @@ -190,25 +193,22 @@ TEST_F(VoipCoreTest, TestProcessThreadOperation) { auto channel_one = voip_core_->CreateChannel(&transport_, 0xdeadc0de); auto channel_two = voip_core_->CreateChannel(&transport_, 0xdeadbeef); - EXPECT_TRUE(channel_one); - EXPECT_TRUE(channel_two); EXPECT_CALL(*process_thread_, Stop); EXPECT_CALL(*process_thread_, DeRegisterModule).Times(2); - voip_core_->ReleaseChannel(*channel_one); - voip_core_->ReleaseChannel(*channel_two); + EXPECT_EQ(voip_core_->ReleaseChannel(channel_one), VoipResult::kOk); + EXPECT_EQ(voip_core_->ReleaseChannel(channel_two), VoipResult::kOk); EXPECT_CALL(*process_thread_, Start); EXPECT_CALL(*process_thread_, RegisterModule); auto channel_three = voip_core_->CreateChannel(&transport_, absl::nullopt); - EXPECT_TRUE(channel_three); EXPECT_CALL(*process_thread_, Stop); EXPECT_CALL(*process_thread_, DeRegisterModule); - voip_core_->ReleaseChannel(*channel_three); + EXPECT_EQ(voip_core_->ReleaseChannel(channel_three), VoipResult::kOk); } } // namespace diff --git a/audio/voip/voip_core.cc b/audio/voip/voip_core.cc index ac29fbf6d8..f65352c23f 100644 --- a/audio/voip/voip_core.cc +++ b/audio/voip/voip_core.cc @@ -127,10 +127,9 @@ bool VoipCore::InitializeIfNeeded() { return true; } -absl::optional VoipCore::CreateChannel( - Transport* transport, - absl::optional local_ssrc) { - absl::optional channel_id; +ChannelId VoipCore::CreateChannel(Transport* transport, + absl::optional local_ssrc) { + ChannelId channel_id; // Set local ssrc to random if not set by caller. if (!local_ssrc) { @@ -153,7 +152,7 @@ absl::optional VoipCore::CreateChannel( start_process_thread = channels_.empty(); channel_id = static_cast(next_channel_id_); - channels_[*channel_id] = channel; + channels_[channel_id] = channel; next_channel_id_++; if (next_channel_id_ >= kMaxChannelId) { next_channel_id_ = 0; @@ -161,7 +160,7 @@ absl::optional VoipCore::CreateChannel( } // Set ChannelId in audio channel for logging/debugging purpose. - channel->SetId(*channel_id); + channel->SetId(channel_id); if (start_process_thread) { process_thread_->Start(); @@ -170,7 +169,7 @@ absl::optional VoipCore::CreateChannel( return channel_id; } -void VoipCore::ReleaseChannel(ChannelId channel_id) { +VoipResult VoipCore::ReleaseChannel(ChannelId channel_id) { // Destroy channel outside of the lock. rtc::scoped_refptr channel; @@ -188,8 +187,10 @@ void VoipCore::ReleaseChannel(ChannelId channel_id) { no_channels_after_release = channels_.empty(); } + VoipResult status_code = VoipResult::kOk; if (!channel) { RTC_LOG(LS_WARNING) << "Channel " << channel_id << " not found"; + status_code = VoipResult::kInvalidArgument; } if (no_channels_after_release) { @@ -201,9 +202,12 @@ void VoipCore::ReleaseChannel(ChannelId channel_id) { if (audio_device_module_->Playing()) { if (audio_device_module_->StopPlayout() != 0) { RTC_LOG(LS_WARNING) << "StopPlayout failed"; + status_code = VoipResult::kInternal; } } } + + return status_code; } rtc::scoped_refptr VoipCore::GetChannel(ChannelId channel_id) { @@ -281,174 +285,219 @@ bool VoipCore::UpdateAudioTransportWithSenders() { return true; } -bool VoipCore::StartSend(ChannelId channel_id) { - rtc::scoped_refptr channel = GetChannel(channel_id); - - if (!channel || !channel->StartSend()) { - return false; - } - - return UpdateAudioTransportWithSenders(); -} - -bool VoipCore::StopSend(ChannelId channel_id) { +VoipResult VoipCore::StartSend(ChannelId channel_id) { rtc::scoped_refptr channel = GetChannel(channel_id); if (!channel) { - return false; + return VoipResult::kInvalidArgument; + } + + if (!channel->StartSend()) { + return VoipResult::kFailedPrecondition; + } + + return UpdateAudioTransportWithSenders() ? VoipResult::kOk + : VoipResult::kInternal; +} + +VoipResult VoipCore::StopSend(ChannelId channel_id) { + rtc::scoped_refptr channel = GetChannel(channel_id); + + if (!channel) { + return VoipResult::kInvalidArgument; } channel->StopSend(); - return UpdateAudioTransportWithSenders(); + return UpdateAudioTransportWithSenders() ? VoipResult::kOk + : VoipResult::kInternal; } -bool VoipCore::StartPlayout(ChannelId channel_id) { +VoipResult VoipCore::StartPlayout(ChannelId channel_id) { rtc::scoped_refptr channel = GetChannel(channel_id); if (!channel) { - return false; + return VoipResult::kInvalidArgument; } if (channel->IsPlaying()) { - return true; + return VoipResult::kOk; } if (!channel->StartPlay()) { - return false; + return VoipResult::kFailedPrecondition; } // Initialize audio device module and default device if needed. if (!InitializeIfNeeded()) { - return false; + return VoipResult::kInternal; } if (!audio_device_module_->Playing()) { if (audio_device_module_->InitPlayout() != 0) { RTC_LOG(LS_ERROR) << "InitPlayout failed"; - return false; + return VoipResult::kInternal; } if (audio_device_module_->StartPlayout() != 0) { RTC_LOG(LS_ERROR) << "StartPlayout failed"; - return false; + return VoipResult::kInternal; } } - return true; + + return VoipResult::kOk; } -bool VoipCore::StopPlayout(ChannelId channel_id) { +VoipResult VoipCore::StopPlayout(ChannelId channel_id) { rtc::scoped_refptr channel = GetChannel(channel_id); if (!channel) { - return false; + return VoipResult::kInvalidArgument; } channel->StopPlay(); - return true; + return VoipResult::kOk; } -void VoipCore::ReceivedRTPPacket(ChannelId channel_id, - rtc::ArrayView rtp_packet) { +VoipResult VoipCore::ReceivedRTPPacket( + ChannelId channel_id, + rtc::ArrayView rtp_packet) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - channel->ReceivedRTPPacket(rtp_packet); + if (!channel) { + return VoipResult::kInvalidArgument; } + + channel->ReceivedRTPPacket(rtp_packet); + + return VoipResult::kOk; } -void VoipCore::ReceivedRTCPPacket(ChannelId channel_id, - rtc::ArrayView rtcp_packet) { +VoipResult VoipCore::ReceivedRTCPPacket( + ChannelId channel_id, + rtc::ArrayView rtcp_packet) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - channel->ReceivedRTCPPacket(rtcp_packet); + if (!channel) { + return VoipResult::kInvalidArgument; } + + channel->ReceivedRTCPPacket(rtcp_packet); + + return VoipResult::kOk; } -void VoipCore::SetSendCodec(ChannelId channel_id, - int payload_type, - const SdpAudioFormat& encoder_format) { +VoipResult VoipCore::SetSendCodec(ChannelId channel_id, + int payload_type, + const SdpAudioFormat& encoder_format) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - auto encoder = encoder_factory_->MakeAudioEncoder( - payload_type, encoder_format, absl::nullopt); - channel->SetEncoder(payload_type, encoder_format, std::move(encoder)); + if (!channel) { + return VoipResult::kInvalidArgument; } + + auto encoder = encoder_factory_->MakeAudioEncoder( + payload_type, encoder_format, absl::nullopt); + channel->SetEncoder(payload_type, encoder_format, std::move(encoder)); + + return VoipResult::kOk; } -void VoipCore::SetReceiveCodecs( +VoipResult VoipCore::SetReceiveCodecs( ChannelId channel_id, const std::map& decoder_specs) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - channel->SetReceiveCodecs(decoder_specs); + if (!channel) { + return VoipResult::kInvalidArgument; } + + channel->SetReceiveCodecs(decoder_specs); + + return VoipResult::kOk; } -void VoipCore::RegisterTelephoneEventType(ChannelId channel_id, - int rtp_payload_type, - int sample_rate_hz) { +VoipResult VoipCore::RegisterTelephoneEventType(ChannelId channel_id, + int rtp_payload_type, + int sample_rate_hz) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - channel->RegisterTelephoneEventType(rtp_payload_type, sample_rate_hz); + if (!channel) { + return VoipResult::kInvalidArgument; } + + channel->RegisterTelephoneEventType(rtp_payload_type, sample_rate_hz); + + return VoipResult::kOk; } -bool VoipCore::SendDtmfEvent(ChannelId channel_id, - DtmfEvent dtmf_event, - int duration_ms) { +VoipResult VoipCore::SendDtmfEvent(ChannelId channel_id, + DtmfEvent dtmf_event, + int duration_ms) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - return channel->SendTelephoneEvent(static_cast(dtmf_event), - duration_ms); + if (!channel) { + return VoipResult::kInvalidArgument; } - return false; + + return (channel->SendTelephoneEvent(static_cast(dtmf_event), duration_ms) + ? VoipResult::kOk + : VoipResult::kFailedPrecondition); } -absl::optional VoipCore::GetIngressStatistics( - ChannelId channel_id) { +VoipResult VoipCore::GetIngressStatistics(ChannelId channel_id, + IngressStatistics& ingress_stats) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - return channel->GetIngressStatistics(); + if (!channel) { + return VoipResult::kInvalidArgument; } - return absl::nullopt; + + ingress_stats = channel->GetIngressStatistics(); + + return VoipResult::kOk; } -void VoipCore::SetInputMuted(ChannelId channel_id, bool enable) { +VoipResult VoipCore::SetInputMuted(ChannelId channel_id, bool enable) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - channel->SetMute(enable); + + if (!channel) { + return VoipResult::kInvalidArgument; } + + channel->SetMute(enable); + + return VoipResult::kOk; } -absl::optional VoipCore::GetInputVolumeInfo(ChannelId channel_id) { +VoipResult VoipCore::GetInputVolumeInfo(ChannelId channel_id, + VolumeInfo& input_volume) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - VolumeInfo input_volume; - input_volume.audio_level = channel->GetInputAudioLevel(); - input_volume.total_energy = channel->GetInputTotalEnergy(); - input_volume.total_duration = channel->GetInputTotalDuration(); - return input_volume; + + if (!channel) { + return VoipResult::kInvalidArgument; } - return absl::nullopt; + + input_volume.audio_level = channel->GetInputAudioLevel(); + input_volume.total_energy = channel->GetInputTotalEnergy(); + input_volume.total_duration = channel->GetInputTotalDuration(); + + return VoipResult::kOk; } -absl::optional VoipCore::GetOutputVolumeInfo(ChannelId channel_id) { +VoipResult VoipCore::GetOutputVolumeInfo(ChannelId channel_id, + VolumeInfo& output_volume) { rtc::scoped_refptr channel = GetChannel(channel_id); - if (channel) { - VolumeInfo output_volume; - output_volume.audio_level = channel->GetOutputAudioLevel(); - output_volume.total_energy = channel->GetOutputTotalEnergy(); - output_volume.total_duration = channel->GetOutputTotalDuration(); - return output_volume; + + if (!channel) { + return VoipResult::kInvalidArgument; } - return absl::nullopt; + + output_volume.audio_level = channel->GetOutputAudioLevel(); + output_volume.total_energy = channel->GetOutputTotalEnergy(); + output_volume.total_duration = channel->GetOutputTotalDuration(); + + return VoipResult::kOk; } } // namespace webrtc diff --git a/audio/voip/voip_core.h b/audio/voip/voip_core.h index 5ebf4381cc..194f8fbb67 100644 --- a/audio/voip/voip_core.h +++ b/audio/voip/voip_core.h @@ -74,45 +74,48 @@ class VoipCore : public VoipEngine, VoipVolumeControl& VolumeControl() override { return *this; } // Implements VoipBase interfaces. - absl::optional CreateChannel( - Transport* transport, - absl::optional local_ssrc) override; - void ReleaseChannel(ChannelId channel_id) override; - bool StartSend(ChannelId channel_id) override; - bool StopSend(ChannelId channel_id) override; - bool StartPlayout(ChannelId channel_id) override; - bool StopPlayout(ChannelId channel_id) override; + ChannelId CreateChannel(Transport* transport, + absl::optional local_ssrc) override; + VoipResult ReleaseChannel(ChannelId channel_id) override; + VoipResult StartSend(ChannelId channel_id) override; + VoipResult StopSend(ChannelId channel_id) override; + VoipResult StartPlayout(ChannelId channel_id) override; + VoipResult StopPlayout(ChannelId channel_id) override; // Implements VoipNetwork interfaces. - void ReceivedRTPPacket(ChannelId channel_id, - rtc::ArrayView rtp_packet) override; - void ReceivedRTCPPacket(ChannelId channel_id, - rtc::ArrayView rtcp_packet) override; + VoipResult ReceivedRTPPacket( + ChannelId channel_id, + rtc::ArrayView rtp_packet) override; + VoipResult ReceivedRTCPPacket( + ChannelId channel_id, + rtc::ArrayView rtcp_packet) override; // Implements VoipCodec interfaces. - void SetSendCodec(ChannelId channel_id, - int payload_type, - const SdpAudioFormat& encoder_format) override; - void SetReceiveCodecs( + VoipResult SetSendCodec(ChannelId channel_id, + int payload_type, + const SdpAudioFormat& encoder_format) override; + VoipResult SetReceiveCodecs( ChannelId channel_id, const std::map& decoder_specs) override; // Implements VoipDtmf interfaces. - void RegisterTelephoneEventType(ChannelId channel_id, - int rtp_payload_type, - int sample_rate_hz) override; - bool SendDtmfEvent(ChannelId channel_id, - DtmfEvent dtmf_event, - int duration_ms) override; + VoipResult RegisterTelephoneEventType(ChannelId channel_id, + int rtp_payload_type, + int sample_rate_hz) override; + VoipResult SendDtmfEvent(ChannelId channel_id, + DtmfEvent dtmf_event, + int duration_ms) override; // Implements VoipStatistics interfaces. - absl::optional GetIngressStatistics( - ChannelId channel_id) override; + VoipResult GetIngressStatistics(ChannelId channel_id, + IngressStatistics& ingress_stats) override; // Implements VoipVolumeControl interfaces. - void SetInputMuted(ChannelId channel_id, bool enable) override; - absl::optional GetInputVolumeInfo(ChannelId channel_id) override; - absl::optional GetOutputVolumeInfo(ChannelId channel_id) override; + VoipResult SetInputMuted(ChannelId channel_id, bool enable) override; + VoipResult GetInputVolumeInfo(ChannelId channel_id, + VolumeInfo& volume_info) override; + VoipResult GetOutputVolumeInfo(ChannelId channel_id, + VolumeInfo& volume_info) override; private: // Initialize ADM and default audio device if needed. diff --git a/examples/androidvoip/jni/android_voip_client.cc b/examples/androidvoip/jni/android_voip_client.cc index 2ad95bcf8d..d0763cdcc9 100644 --- a/examples/androidvoip/jni/android_voip_client.cc +++ b/examples/androidvoip/jni/android_voip_client.cc @@ -347,8 +347,8 @@ void AndroidVoipClient::StopSession(JNIEnv* env) { /*isSuccessful=*/false); return; } - if (!voip_engine_->Base().StopSend(*channel_) || - !voip_engine_->Base().StopPlayout(*channel_)) { + if (voip_engine_->Base().StopSend(*channel_) != webrtc::VoipResult::kOk || + voip_engine_->Base().StopPlayout(*channel_) != webrtc::VoipResult::kOk) { Java_VoipClient_onStopSessionCompleted(env_, j_voip_client_, /*isSuccessful=*/false); return; @@ -372,8 +372,9 @@ void AndroidVoipClient::StartSend(JNIEnv* env) { /*isSuccessful=*/false); return; } - Java_VoipClient_onStartSendCompleted( - env_, j_voip_client_, voip_engine_->Base().StartSend(*channel_)); + bool sending_started = + (voip_engine_->Base().StartSend(*channel_) == webrtc::VoipResult::kOk); + Java_VoipClient_onStartSendCompleted(env_, j_voip_client_, sending_started); } void AndroidVoipClient::StopSend(JNIEnv* env) { @@ -385,8 +386,9 @@ void AndroidVoipClient::StopSend(JNIEnv* env) { /*isSuccessful=*/false); return; } - Java_VoipClient_onStopSendCompleted(env_, j_voip_client_, - voip_engine_->Base().StopSend(*channel_)); + bool sending_stopped = + (voip_engine_->Base().StopSend(*channel_) == webrtc::VoipResult::kOk); + Java_VoipClient_onStopSendCompleted(env_, j_voip_client_, sending_stopped); } void AndroidVoipClient::StartPlayout(JNIEnv* env) { @@ -398,8 +400,10 @@ void AndroidVoipClient::StartPlayout(JNIEnv* env) { /*isSuccessful=*/false); return; } - Java_VoipClient_onStartPlayoutCompleted( - env_, j_voip_client_, voip_engine_->Base().StartPlayout(*channel_)); + bool playout_started = + (voip_engine_->Base().StartPlayout(*channel_) == webrtc::VoipResult::kOk); + Java_VoipClient_onStartPlayoutCompleted(env_, j_voip_client_, + playout_started); } void AndroidVoipClient::StopPlayout(JNIEnv* env) { @@ -411,8 +415,9 @@ void AndroidVoipClient::StopPlayout(JNIEnv* env) { /*isSuccessful=*/false); return; } - Java_VoipClient_onStopPlayoutCompleted( - env_, j_voip_client_, voip_engine_->Base().StopPlayout(*channel_)); + bool playout_stopped = + (voip_engine_->Base().StopPlayout(*channel_) == webrtc::VoipResult::kOk); + Java_VoipClient_onStopPlayoutCompleted(env_, j_voip_client_, playout_stopped); } void AndroidVoipClient::Delete(JNIEnv* env) {