diff --git a/api/rtpsenderinterface.h b/api/rtpsenderinterface.h index a7d7edc4b0..83e135f350 100644 --- a/api/rtpsenderinterface.h +++ b/api/rtpsenderinterface.h @@ -21,6 +21,7 @@ #include "api/mediastreaminterface.h" #include "api/mediatypes.h" #include "api/proxy.h" +#include "api/rtcerror.h" #include "api/rtpparameters.h" #include "rtc_base/refcount.h" #include "rtc_base/scoped_ref_ptr.h" @@ -54,7 +55,7 @@ class RtpSenderInterface : public rtc::RefCountInterface { virtual RtpParameters GetParameters() const = 0; // Note that only a subset of the parameters can currently be changed. See // rtpparameters.h - virtual bool SetParameters(const RtpParameters& parameters) = 0; + virtual RTCError SetParameters(const RtpParameters& parameters) = 0; // Returns null for a video sender. virtual rtc::scoped_refptr GetDtmfSender() const = 0; @@ -82,7 +83,7 @@ BEGIN_SIGNALING_PROXY_MAP(RtpSender) PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(std::vector, stream_ids) PROXY_CONSTMETHOD0(RtpParameters, GetParameters); - PROXY_METHOD1(bool, SetParameters, const RtpParameters&) + PROXY_METHOD1(RTCError, SetParameters, const RtpParameters&) PROXY_CONSTMETHOD0(rtc::scoped_refptr, GetDtmfSender); PROXY_CONSTMETHOD0(int, AttachmentId); END_PROXY_MAP() diff --git a/api/test/mock_rtpsender.h b/api/test/mock_rtpsender.h index 35d048c18a..728ceca1a8 100644 --- a/api/test/mock_rtpsender.h +++ b/api/test/mock_rtpsender.h @@ -28,7 +28,7 @@ class MockRtpSender : public rtc::RefCountedObject { MOCK_CONST_METHOD0(id, std::string()); MOCK_CONST_METHOD0(stream_ids, std::vector()); MOCK_CONST_METHOD0(GetParameters, RtpParameters()); - MOCK_METHOD1(SetParameters, bool(const RtpParameters&)); + MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&)); MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr()); MOCK_CONST_METHOD0(AttachmentId, int()); }; diff --git a/media/base/fakemediaengine.h b/media/base/fakemediaengine.h index 609b62f910..46cc3dab1c 100644 --- a/media/base/fakemediaengine.h +++ b/media/base/fakemediaengine.h @@ -145,16 +145,17 @@ template class RtpHelper : public Base { } return webrtc::RtpParameters(); } - virtual bool SetRtpSendParameters(uint32_t ssrc, - const webrtc::RtpParameters& parameters) { + virtual webrtc::RTCError SetRtpSendParameters( + uint32_t ssrc, + const webrtc::RtpParameters& parameters) { auto parameters_iterator = rtp_send_parameters_.find(ssrc); if (parameters_iterator != rtp_send_parameters_.end()) { parameters_iterator->second = parameters; - return true; + return webrtc::RTCError::OK(); } // Replicate the behavior of the real media channel: return false // when setting parameters for unknown SSRCs. - return false; + return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); } virtual webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const { diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h index 3b78144c03..af2434f121 100644 --- a/media/base/mediachannel.h +++ b/media/base/mediachannel.h @@ -20,6 +20,7 @@ #include "api/audio_codecs/audio_encoder.h" #include "api/audio_options.h" #include "api/optional.h" +#include "api/rtcerror.h" #include "api/rtpparameters.h" #include "api/rtpreceiverinterface.h" #include "api/video/video_content_type.h" @@ -647,7 +648,7 @@ class VoiceMediaChannel : public MediaChannel { virtual bool SetSendParameters(const AudioSendParameters& params) = 0; virtual bool SetRecvParameters(const AudioRecvParameters& params) = 0; virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0; - virtual bool SetRtpSendParameters( + virtual webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) = 0; // Get the receive parameters for the incoming stream identified by |ssrc|. @@ -720,7 +721,7 @@ class VideoMediaChannel : public MediaChannel { virtual bool SetSendParameters(const VideoSendParameters& params) = 0; virtual bool SetRecvParameters(const VideoRecvParameters& params) = 0; virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0; - virtual bool SetRtpSendParameters( + virtual webrtc::RTCError SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) = 0; // Get the receive parameters for the incoming stream identified by |ssrc|. diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 7bbb88b97a..2357bdfe8a 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -816,7 +816,7 @@ webrtc::RtpParameters WebRtcVideoChannel::GetRtpSendParameters( return rtp_params; } -bool WebRtcVideoChannel::SetRtpSendParameters( +webrtc::RTCError WebRtcVideoChannel::SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { TRACE_EVENT0("webrtc", "WebRtcVideoChannel::SetRtpSendParameters"); @@ -825,7 +825,7 @@ bool WebRtcVideoChannel::SetRtpSendParameters( if (it == send_streams_.end()) { RTC_LOG(LS_ERROR) << "Attempting to set RTP send parameters for stream " << "with ssrc " << ssrc << " which doesn't exist."; - return false; + return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); } // TODO(deadbeef): Handle setting parameters with a list of codecs in a @@ -834,7 +834,7 @@ bool WebRtcVideoChannel::SetRtpSendParameters( if (current_parameters.codecs != parameters.codecs) { RTC_LOG(LS_ERROR) << "Using SetParameters to change the set of codecs " << "is not currently supported."; - return false; + return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); } return it->second->SetRtpParameters(parameters); @@ -1784,11 +1784,12 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters( } } -bool WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( +webrtc::RTCError WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( const webrtc::RtpParameters& new_parameters) { RTC_DCHECK_RUN_ON(&thread_checker_); - if (!ValidateRtpParameters(new_parameters)) { - return false; + webrtc::RTCError error = ValidateRtpParameters(new_parameters); + if (!error.ok()) { + return error; } bool reconfigure_encoder = (new_parameters.encodings[0].max_bitrate_bps != @@ -1803,7 +1804,7 @@ bool WebRtcVideoChannel::WebRtcVideoSendStream::SetRtpParameters( } // Encoding may have been activated/deactivated. UpdateSendState(); - return true; + return webrtc::RTCError::OK(); } webrtc::RtpParameters @@ -1812,24 +1813,26 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetRtpParameters() const { return rtp_parameters_; } -bool WebRtcVideoChannel::WebRtcVideoSendStream::ValidateRtpParameters( +webrtc::RTCError +WebRtcVideoChannel::WebRtcVideoSendStream::ValidateRtpParameters( const webrtc::RtpParameters& rtp_parameters) { + using webrtc::RTCErrorType; RTC_DCHECK_RUN_ON(&thread_checker_); if (rtp_parameters.encodings.size() != rtp_parameters_.encodings.size()) { - RTC_LOG(LS_ERROR) - << "Attempted to set RtpParameters with different encoding count"; - return false; + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with different encoding count"); } if (rtp_parameters.encodings[0].ssrc != rtp_parameters_.encodings[0].ssrc) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC"; - return false; + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified SSRC"); } if (rtp_parameters.encodings[0].bitrate_priority <= 0) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters bitrate_priority to " - "an invalid number. bitrate_priority must be > 0."; - return false; + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, + "Attempted to set RtpParameters bitrate_priority to " + "an invalid number. bitrate_priority must be > 0."); } - return true; + return webrtc::RTCError::OK(); } void WebRtcVideoChannel::WebRtcVideoSendStream::UpdateSendState() { diff --git a/media/engine/webrtcvideoengine.h b/media/engine/webrtcvideoengine.h index 56c7586de9..c6f54128eb 100644 --- a/media/engine/webrtcvideoengine.h +++ b/media/engine/webrtcvideoengine.h @@ -137,8 +137,9 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { bool SetSendParameters(const VideoSendParameters& params) override; bool SetRecvParameters(const VideoRecvParameters& params) override; webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const override; - bool SetRtpSendParameters(uint32_t ssrc, - const webrtc::RtpParameters& parameters) override; + webrtc::RTCError SetRtpSendParameters( + uint32_t ssrc, + const webrtc::RtpParameters& parameters) override; webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const override; bool SetRtpReceiveParameters( uint32_t ssrc, @@ -264,7 +265,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { virtual ~WebRtcVideoSendStream(); void SetSendParameters(const ChangedSendParameters& send_params); - bool SetRtpParameters(const webrtc::RtpParameters& parameters); + webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters); webrtc::RtpParameters GetRtpParameters() const; // Implements rtc::VideoSourceInterface. @@ -315,7 +316,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport { webrtc::VideoEncoderConfig CreateVideoEncoderConfig( const VideoCodec& codec) const; void ReconfigureEncoder(); - bool ValidateRtpParameters(const webrtc::RtpParameters& parameters); + webrtc::RTCError ValidateRtpParameters( + const webrtc::RtpParameters& parameters); // Calls Start or Stop according to whether or not |sending_| is true, // and whether or not the encoding in |rtp_parameters_| is active. diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc index a896c43569..9accef0321 100644 --- a/media/engine/webrtcvideoengine_unittest.cc +++ b/media/engine/webrtcvideoengine_unittest.cc @@ -1449,7 +1449,7 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { channel_->GetRtpSendParameters(last_ssrc_); EXPECT_EQ(1UL, parameters.encodings.size()); parameters.encodings[0].max_bitrate_bps = stream_max; - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); // Read back the parameteres and verify they have the correct value parameters = channel_->GetRtpSendParameters(last_ssrc_); EXPECT_EQ(1UL, parameters.encodings.size()); @@ -4359,7 +4359,7 @@ TEST_F(WebRtcVideoChannelTest, CannotSetMaxBitrateForNonexistentStream) { nonexistent_parameters.encodings.push_back(webrtc::RtpEncodingParameters()); EXPECT_FALSE( - channel_->SetRtpSendParameters(last_ssrc_, nonexistent_parameters)); + channel_->SetRtpSendParameters(last_ssrc_, nonexistent_parameters).ok()); } TEST_F(WebRtcVideoChannelTest, @@ -4368,10 +4368,10 @@ TEST_F(WebRtcVideoChannelTest, webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); // Two or more encodings should result in failure. parameters.encodings.push_back(webrtc::RtpEncodingParameters()); - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); // Zero encodings should also fail. parameters.encodings.clear(); - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); } TEST_F(WebRtcVideoChannelTest, @@ -4384,10 +4384,10 @@ TEST_F(WebRtcVideoChannelTest, // Additional encodings should result in failure. parameters.encodings.push_back(webrtc::RtpEncodingParameters()); - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); // Zero encodings should also fail. parameters.encodings.clear(); - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); } // Changing the SSRC through RtpParameters is not allowed. @@ -4395,7 +4395,7 @@ TEST_F(WebRtcVideoChannelTest, CannotSetSsrcInRtpSendParameters) { AddSendStream(); webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_); parameters.encodings[0].ssrc = 0xdeadbeef; - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); } // Tests that when RTCRtpEncodingParameters.bitrate_priority gets set to @@ -4408,9 +4408,9 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersInvalidBitratePriority) { parameters.encodings[0].bitrate_priority); parameters.encodings[0].bitrate_priority = 0; - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); parameters.encodings[0].bitrate_priority = -2; - EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); } // Tests when the the RTCRtpEncodingParameters.bitrate_priority gets set @@ -4425,7 +4425,7 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersPriorityOneStream) { // Change the value and set it on the VideoChannel. double new_bitrate_priority = 2.0; parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); // Verify that the encoding parameters bitrate_priority is set for the // VideoChannel. @@ -4481,7 +4481,7 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersPrioritySimulcastStreams) { // Change the value and set it on the VideoChannel. double new_bitrate_priority = 2.0; parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(primary_ssrc, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(primary_ssrc, parameters).ok()); // Verify that the encoding parameters priority is set on the VideoChannel. parameters = channel_->GetRtpSendParameters(primary_ssrc); @@ -4528,12 +4528,12 @@ TEST_F(WebRtcVideoChannelTest, SetRtpSendParametersEncodingsActive) { ASSERT_EQ(1u, parameters.encodings.size()); ASSERT_TRUE(parameters.encodings[0].active); parameters.encodings[0].active = false; - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); EXPECT_FALSE(stream->IsSending()); // Now change it back to active and verify we resume sending. parameters.encodings[0].active = true; - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); EXPECT_TRUE(stream->IsSending()); } @@ -4558,7 +4558,7 @@ TEST_F(WebRtcVideoChannelTest, parameters.encodings[0].active = false; EXPECT_EQ(1u, GetFakeSendStreams().size()); EXPECT_EQ(1, fake_call_->GetNumCreatedSendStreams()); - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); EXPECT_FALSE(stream->IsSending()); // Reorder the codec list, causing the stream to be reconfigured. @@ -4617,7 +4617,7 @@ TEST_F(WebRtcVideoChannelTest, SetAndGetRtpSendParameters) { channel_->GetRtpSendParameters(last_ssrc_); // We should be able to set the params we just got. - EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, initial_params)); + EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, initial_params).ok()); // ... And this shouldn't change the params returned by GetRtpSendParameters. EXPECT_EQ(initial_params, channel_->GetRtpSendParameters(last_ssrc_)); diff --git a/media/engine/webrtcvoiceengine.cc b/media/engine/webrtcvoiceengine.cc index a1cbf9204f..91618f8c7c 100644 --- a/media/engine/webrtcvoiceengine.cc +++ b/media/engine/webrtcvoiceengine.cc @@ -939,27 +939,30 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream return rtp_parameters_; } - bool ValidateRtpParameters(const webrtc::RtpParameters& rtp_parameters) { - if (rtp_parameters.encodings.size() != 1) { - RTC_LOG(LS_ERROR) - << "Attempted to set RtpParameters without exactly one encoding"; - return false; + webrtc::RTCError ValidateRtpParameters( + const webrtc::RtpParameters& rtp_parameters) { + using webrtc::RTCErrorType; + if (rtp_parameters.encodings.size() != rtp_parameters_.encodings.size()) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with different encoding count"); } if (rtp_parameters.encodings[0].ssrc != rtp_parameters_.encodings[0].ssrc) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC"; - return false; + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, + "Attempted to set RtpParameters with modified SSRC"); } if (rtp_parameters.encodings[0].bitrate_priority <= 0) { - RTC_LOG(LS_ERROR) << "Attempted to set RtpParameters bitrate_priority to " - "an invalid number."; - return false; + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_RANGE, + "Attempted to set RtpParameters bitrate_priority to " + "an invalid number."); } - return true; + return webrtc::RTCError::OK(); } - bool SetRtpParameters(const webrtc::RtpParameters& parameters) { - if (!ValidateRtpParameters(parameters)) { - return false; + webrtc::RTCError SetRtpParameters(const webrtc::RtpParameters& parameters) { + webrtc::RTCError error = ValidateRtpParameters(parameters); + if (!error.ok()) { + return error; } rtc::Optional send_rate; @@ -968,7 +971,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream parameters.encodings[0].max_bitrate_bps, *audio_codec_spec_); if (!send_rate) { - return false; + return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); } } @@ -993,7 +996,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream } // parameters.encodings[0].active could have changed. UpdateSendState(); - return true; + return webrtc::RTCError::OK(); } private: @@ -1363,7 +1366,7 @@ webrtc::RtpParameters WebRtcVoiceMediaChannel::GetRtpSendParameters( return rtp_params; } -bool WebRtcVoiceMediaChannel::SetRtpSendParameters( +webrtc::RTCError WebRtcVoiceMediaChannel::SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); @@ -1371,7 +1374,7 @@ bool WebRtcVoiceMediaChannel::SetRtpSendParameters( if (it == send_streams_.end()) { RTC_LOG(LS_WARNING) << "Attempting to set RTP send parameters for stream " << "with ssrc " << ssrc << " which doesn't exist."; - return false; + return webrtc::RTCError(webrtc::RTCErrorType::INTERNAL_ERROR); } // TODO(deadbeef): Handle setting parameters with a list of codecs in a @@ -1380,7 +1383,7 @@ bool WebRtcVoiceMediaChannel::SetRtpSendParameters( if (current_parameters.codecs != parameters.codecs) { RTC_LOG(LS_ERROR) << "Using SetParameters to change the set of codecs " << "is not currently supported."; - return false; + return webrtc::RTCError(webrtc::RTCErrorType::UNSUPPORTED_PARAMETER); } // TODO(minyue): The following legacy actions go into diff --git a/media/engine/webrtcvoiceengine.h b/media/engine/webrtcvoiceengine.h index 47aba8a1e3..083c1eb45b 100644 --- a/media/engine/webrtcvoiceengine.h +++ b/media/engine/webrtcvoiceengine.h @@ -156,8 +156,9 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, bool SetSendParameters(const AudioSendParameters& params) override; bool SetRecvParameters(const AudioRecvParameters& params) override; webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const override; - bool SetRtpSendParameters(uint32_t ssrc, - const webrtc::RtpParameters& parameters) override; + webrtc::RTCError SetRtpSendParameters( + uint32_t ssrc, + const webrtc::RtpParameters& parameters) override; webrtc::RtpParameters GetRtpReceiveParameters(uint32_t ssrc) const override; bool SetRtpReceiveParameters( uint32_t ssrc, @@ -225,7 +226,8 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, int CreateVoEChannel(); bool DeleteVoEChannel(int channel); bool SetMaxSendBitrate(int bps); - bool ValidateRtpParameters(const webrtc::RtpParameters& parameters); + webrtc::RTCError ValidateRtpParameters( + const webrtc::RtpParameters& parameters); void SetupRecording(); // Check if 'ssrc' is an unsignaled stream, and if so mark it as not being // unsignaled anymore (i.e. it is now removed, or signaled), and return true. diff --git a/media/engine/webrtcvoiceengine_unittest.cc b/media/engine/webrtcvoiceengine_unittest.cc index 9b56dbf826..f5fc9a0e38 100644 --- a/media/engine/webrtcvoiceengine_unittest.cc +++ b/media/engine/webrtcvoiceengine_unittest.cc @@ -362,7 +362,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_EQ(1UL, parameters.encodings.size()); parameters.encodings[0].max_bitrate_bps = bitrate; - return channel_->SetRtpSendParameters(ssrc, parameters); + return channel_->SetRtpSendParameters(ssrc, parameters).ok(); } void SetGlobalMaxBitrate(const cricket::AudioCodec& codec, int bitrate) { @@ -1034,7 +1034,8 @@ TEST_F(WebRtcVoiceEngineTestFake, CannotSetMaxBitrateForNonexistentStream) { EXPECT_EQ(0, nonexistent_parameters.encodings.size()); nonexistent_parameters.encodings.push_back(webrtc::RtpEncodingParameters()); - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, nonexistent_parameters)); + EXPECT_FALSE( + channel_->SetRtpSendParameters(kSsrcX, nonexistent_parameters).ok()); } TEST_F(WebRtcVoiceEngineTestFake, @@ -1048,10 +1049,10 @@ TEST_F(WebRtcVoiceEngineTestFake, webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(kSsrcX); // Two or more encodings should result in failure. parameters.encodings.push_back(webrtc::RtpEncodingParameters()); - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok()); // Zero encodings should also fail. parameters.encodings.clear(); - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok()); } // Changing the SSRC through RtpParameters is not allowed. @@ -1059,7 +1060,7 @@ TEST_F(WebRtcVoiceEngineTestFake, CannotSetSsrcInRtpSendParameters) { EXPECT_TRUE(SetupSendStream()); webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(kSsrcX); parameters.encodings[0].ssrc = 0xdeadbeef; - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok()); } // Test that a stream will not be sending if its encoding is made @@ -1073,14 +1074,14 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpParametersEncodingsActive) { ASSERT_EQ(1u, parameters.encodings.size()); ASSERT_TRUE(parameters.encodings[0].active); parameters.encodings[0].active = false; - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok()); EXPECT_FALSE(GetSendStream(kSsrcX).IsSending()); // Now change it back to active and verify we resume sending. // This should occur even when other parameters are updated. parameters.encodings[0].active = true; parameters.encodings[0].max_bitrate_bps = rtc::Optional(6000); - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, parameters).ok()); EXPECT_TRUE(GetSendStream(kSsrcX).IsSending()); } @@ -1146,7 +1147,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAndGetRtpSendParameters) { webrtc::RtpParameters initial_params = channel_->GetRtpSendParameters(kSsrcX); // We should be able to set the params we just got. - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, initial_params)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, initial_params).ok()); // ... And this shouldn't change the params returned by GetRtpSendParameters. webrtc::RtpParameters new_params = channel_->GetRtpSendParameters(kSsrcX); @@ -1169,7 +1170,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterUpdatesMaxBitrate) { constexpr int kMaxBitrateBps = 6000; rtp_parameters.encodings[0].max_bitrate_bps = kMaxBitrateBps; - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok()); const int max_bitrate = GetSendStreamConfig(kSsrcX).max_bitrate_bps; EXPECT_EQ(max_bitrate, kMaxBitrateBps); @@ -1185,9 +1186,9 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterInvalidBitratePriority) { rtp_parameters.encodings[0].bitrate_priority); rtp_parameters.encodings[0].bitrate_priority = 0; - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok()); rtp_parameters.encodings[0].bitrate_priority = -1.0; - EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + EXPECT_FALSE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok()); } // Test that the bitrate_priority in the send stream config gets updated when @@ -1201,7 +1202,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRtpSendParameterUpdatesBitratePriority) { rtp_parameters.encodings[0].bitrate_priority); double new_bitrate_priority = 2.0; rtp_parameters.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok()); // The priority should get set for both the audio channel's rtp parameters // and the audio send stream's audio config. @@ -2344,7 +2345,7 @@ TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest, constexpr int kMaxBitrateBps = 6000; rtp_parameters.encodings[0].max_bitrate_bps = kMaxBitrateBps; - EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters)); + EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok()); const int max_bitrate = GetSendStreamConfig(kSsrcX).max_bitrate_bps; #if WEBRTC_OPUS_SUPPORT_120MS_PTIME diff --git a/pc/channel.h b/pc/channel.h index 00747eb2ed..3fb19972b9 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -480,7 +480,8 @@ class VoiceChannel : public BaseChannel { int GetOutputLevel_w(); void GetActiveStreams_w(AudioInfo::StreamList* actives); webrtc::RtpParameters GetRtpSendParameters_w(uint32_t ssrc) const; - bool SetRtpSendParameters_w(uint32_t ssrc, webrtc::RtpParameters parameters); + webrtc::RTCError SetRtpSendParameters_w(uint32_t ssrc, + webrtc::RtpParameters parameters); cricket::MediaType media_type() override { return cricket::MEDIA_TYPE_AUDIO; } private: diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc index ef8c650acc..6095418788 100644 --- a/pc/rtpsender.cc +++ b/pc/rtpsender.cc @@ -199,12 +199,12 @@ RtpParameters AudioRtpSender::GetParameters() const { }); } -bool AudioRtpSender::SetParameters(const RtpParameters& parameters) { +RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) { TRACE_EVENT0("webrtc", "AudioRtpSender::SetParameters"); if (!media_channel_ || stopped_) { - return false; + return RTCError(RTCErrorType::INVALID_STATE); } - return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { return media_channel_->SetRtpSendParameters(ssrc_, parameters); }); } @@ -391,12 +391,12 @@ RtpParameters VideoRtpSender::GetParameters() const { }); } -bool VideoRtpSender::SetParameters(const RtpParameters& parameters) { +RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) { TRACE_EVENT0("webrtc", "VideoRtpSender::SetParameters"); if (!media_channel_ || stopped_) { - return false; + return RTCError(RTCErrorType::INVALID_STATE); } - return worker_thread_->Invoke(RTC_FROM_HERE, [&] { + return worker_thread_->Invoke(RTC_FROM_HERE, [&] { return media_channel_->SetRtpSendParameters(ssrc_, parameters); }); } diff --git a/pc/rtpsender.h b/pc/rtpsender.h index 092efe7111..97e32e2638 100644 --- a/pc/rtpsender.h +++ b/pc/rtpsender.h @@ -120,7 +120,7 @@ class AudioRtpSender : public DtmfProviderInterface, std::vector stream_ids() const override { return stream_ids_; } RtpParameters GetParameters() const override; - bool SetParameters(const RtpParameters& parameters) override; + RTCError SetParameters(const RtpParameters& parameters) override; rtc::scoped_refptr GetDtmfSender() const override; @@ -211,7 +211,7 @@ class VideoRtpSender : public ObserverInterface, std::vector stream_ids() const override { return stream_ids_; } RtpParameters GetParameters() const override; - bool SetParameters(const RtpParameters& parameters) override; + RTCError SetParameters(const RtpParameters& parameters) override; rtc::scoped_refptr GetDtmfSender() const override; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index ff425e2f82..40ed3686f9 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -589,7 +589,7 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParameters) { RtpParameters params = audio_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); DestroyAudioRtpSender(); } @@ -602,7 +602,7 @@ TEST_F(RtpSenderReceiverTest, SetAudioMaxSendBitrate) { EXPECT_EQ(1, params.encodings.size()); EXPECT_FALSE(params.encodings[0].max_bitrate_bps); params.encodings[0].max_bitrate_bps = 1000; - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); // Read back the parameters and verify they have been changed. params = audio_rtp_sender_->GetParameters(); @@ -629,7 +629,7 @@ TEST_F(RtpSenderReceiverTest, SetAudioBitratePriority) { params.encodings[0].bitrate_priority); double new_bitrate_priority = 2.0; params.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(audio_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok()); params = audio_rtp_sender_->GetParameters(); EXPECT_EQ(1, params.encodings.size()); @@ -647,7 +647,7 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParameters) { RtpParameters params = video_rtp_sender_->GetParameters(); EXPECT_EQ(1u, params.encodings.size()); - EXPECT_TRUE(video_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); DestroyVideoRtpSender(); } @@ -660,7 +660,7 @@ TEST_F(RtpSenderReceiverTest, SetVideoMaxSendBitrate) { EXPECT_EQ(1, params.encodings.size()); EXPECT_FALSE(params.encodings[0].max_bitrate_bps); params.encodings[0].max_bitrate_bps = 1000; - EXPECT_TRUE(video_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); // Read back the parameters and verify they have been changed. params = video_rtp_sender_->GetParameters(); @@ -687,7 +687,7 @@ TEST_F(RtpSenderReceiverTest, SetVideoBitratePriority) { params.encodings[0].bitrate_priority); double new_bitrate_priority = 2.0; params.encodings[0].bitrate_priority = new_bitrate_priority; - EXPECT_TRUE(video_rtp_sender_->SetParameters(params)); + EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok()); params = video_rtp_sender_->GetParameters(); EXPECT_EQ(1, params.encodings.size()); diff --git a/sdk/android/src/jni/pc/rtpsender.cc b/sdk/android/src/jni/pc/rtpsender.cc index da59ad017a..58053f97aa 100644 --- a/sdk/android/src/jni/pc/rtpsender.cc +++ b/sdk/android/src/jni/pc/rtpsender.cc @@ -63,7 +63,8 @@ jboolean JNI_RtpSender_SetParameters( } RtpParameters parameters = JavaToNativeRtpParameters(jni, j_parameters); return reinterpret_cast(j_rtp_sender_pointer) - ->SetParameters(parameters); + ->SetParameters(parameters) + .ok(); } ScopedJavaLocalRef JNI_RtpSender_GetParameters( diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm index de3d69c958..ed007cf952 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCRtpSender.mm @@ -34,7 +34,7 @@ } - (void)setParameters:(RTCRtpParameters *)parameters { - if (!_nativeRtpSender->SetParameters(parameters.nativeParameters)) { + if (!_nativeRtpSender->SetParameters(parameters.nativeParameters).ok()) { RTCLogError(@"RTCRtpSender(%p): Failed to set parameters: %@", self, parameters); }