From 16cec3be2c96d7db74e5cb6a9d9837e98d09944e Mon Sep 17 00:00:00 2001 From: philipel Date: Fri, 25 Oct 2019 12:23:02 +0200 Subject: [PATCH] Added allow_codec_switching parameter to RTCConfig. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10795 Change-Id: I5507f1d801e262223bd18198c685b5fffa644b0b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157891 Commit-Queue: Philip Eliasson Reviewed-by: Per Kjellander Reviewed-by: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#29612} --- api/peer_connection_interface.h | 3 +++ media/base/media_channel.cc | 2 ++ media/base/media_channel.h | 3 +++ media/engine/webrtc_video_engine.cc | 13 +++++++++++++ media/engine/webrtc_video_engine.h | 4 ++++ media/engine/webrtc_video_engine_unittest.cc | 2 ++ pc/peer_connection.cc | 10 +++++++++- sdk/android/api/org/webrtc/PeerConnection.java | 12 ++++++++++++ sdk/android/src/jni/pc/peer_connection.cc | 3 +++ 9 files changed, 51 insertions(+), 1 deletion(-) diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 2b7da8369d..cc2fa46c99 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -673,6 +673,9 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // Added to be able to control rollout of this feature. bool enable_implicit_rollback = false; + // Whether network condition based codec switching is allowed. + absl::optional allow_codec_switching; + // // Don't forget to update operator== if adding something. // diff --git a/media/base/media_channel.cc b/media/base/media_channel.cc index 1bd20dc297..579cbc6b6f 100644 --- a/media/base/media_channel.cc +++ b/media/base/media_channel.cc @@ -47,6 +47,8 @@ void MediaChannel::SetFrameDecryptor( // Placeholder should be pure virtual once internal supports it. } +void MediaChannel::SetVideoCodecSwitchingEnabled(bool enabled) {} + MediaSenderInfo::MediaSenderInfo() = default; MediaSenderInfo::~MediaSenderInfo() = default; diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 582d29c385..c49f2ec069 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -243,6 +243,9 @@ class MediaChannel : public sigslot::has_slots<> { uint32_t ssrc, rtc::scoped_refptr frame_decryptor); + // Enable network condition based codec switching. + virtual void SetVideoCodecSwitchingEnabled(bool enabled); + // Base method to send packet using NetworkInterface. bool SendPacket(rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options) { diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 9ea80cc062..cab4e122e8 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -739,6 +739,12 @@ void WebRtcVideoChannel::RequestEncoderSwitch( invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, conf] { RTC_DCHECK_RUN_ON(&thread_checker_); + if (!allow_codec_switching_) { + RTC_LOG(LS_INFO) << "Encoder switch requested but codec switching has" + << " not been enabled."; + return; + } + for (VideoCodecSettings codec_setting : negotiated_codecs_) { if (codec_setting.codec.name == conf.codec_name) { if (conf.param) { @@ -1678,6 +1684,13 @@ void WebRtcVideoChannel::SetFrameEncryptor( } } +void WebRtcVideoChannel::SetVideoCodecSwitchingEnabled(bool enabled) { + invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [this, enabled] { + RTC_DCHECK_RUN_ON(&thread_checker_); + allow_codec_switching_ = enabled; + }); +} + bool WebRtcVideoChannel::SetBaseMinimumPlayoutDelayMs(uint32_t ssrc, int delay_ms) { RTC_DCHECK_RUN_ON(&thread_checker_); diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 5e5ab6e4f3..2493edb6a0 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -172,6 +172,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, rtc::scoped_refptr frame_encryptor) override; + void SetVideoCodecSwitchingEnabled(bool enabled) override; + bool SetBaseMinimumPlayoutDelayMs(uint32_t ssrc, int delay_ms) override; absl::optional GetBaseMinimumPlayoutDelayMs( @@ -569,6 +571,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, std::unique_ptr unknown_ssrc_packet_buffer_ RTC_GUARDED_BY(thread_checker_); + bool allow_codec_switching_ = false; + // In order for the |invoker_| to protect other members from being destructed // as they are used in asynchronous tasks it has to be destructed first. rtc::AsyncInvoker invoker_; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 62bbf245f8..b60ab953e9 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -2151,6 +2151,7 @@ TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderSwitchWithConfig) { parameters.codecs.push_back(vp8); EXPECT_TRUE(channel_->SetSendParameters(parameters)); + channel_->SetVideoCodecSwitchingEnabled(true); VideoCodec codec; ASSERT_TRUE(channel_->GetSendCodec(&codec)); @@ -2188,6 +2189,7 @@ TEST_F(WebRtcVideoChannelBaseTest, RequestEncoderSwitchIncorrectParam) { parameters.codecs.push_back(vp8); EXPECT_TRUE(channel_->SetSendParameters(parameters)); + channel_->SetVideoCodecSwitchingEnabled(true); VideoCodec codec; ASSERT_TRUE(channel_->GetSendCodec(&codec)); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 494a649e8f..08fbe41949 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -784,6 +784,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( bool offer_extmap_allow_mixed; std::string turn_logging_id; bool enable_implicit_rollback; + absl::optional allow_codec_switching; }; static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this), "Did you add something to RTCConfiguration and forget to " @@ -851,7 +852,8 @@ bool PeerConnectionInterface::RTCConfiguration::operator==( crypto_options == o.crypto_options && offer_extmap_allow_mixed == o.offer_extmap_allow_mixed && turn_logging_id == o.turn_logging_id && - enable_implicit_rollback == o.enable_implicit_rollback; + enable_implicit_rollback == o.enable_implicit_rollback && + allow_codec_switching == o.allow_codec_switching; } bool PeerConnectionInterface::RTCConfiguration::operator!=( @@ -3704,6 +3706,7 @@ RTCError PeerConnection::SetConfiguration( modified_config.use_datagram_transport_for_data_channels_receive_only = configuration.use_datagram_transport_for_data_channels_receive_only; modified_config.turn_logging_id = configuration.turn_logging_id; + modified_config.allow_codec_switching = configuration.allow_codec_switching; if (configuration != modified_config) { LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_MODIFICATION, "Modifying the configuration in an unsupported way."); @@ -3791,6 +3794,11 @@ RTCError PeerConnection::SetConfiguration( modified_config.active_reset_srtp_params); } + if (modified_config.allow_codec_switching.has_value()) { + video_media_channel()->SetVideoCodecSwitchingEnabled( + *modified_config.allow_codec_switching); + } + configuration_ = modified_config; use_media_transport_ = configuration.use_media_transport; return RTCError::OK(); diff --git a/sdk/android/api/org/webrtc/PeerConnection.java b/sdk/android/api/org/webrtc/PeerConnection.java index b981520746..e675d13c75 100644 --- a/sdk/android/api/org/webrtc/PeerConnection.java +++ b/sdk/android/api/org/webrtc/PeerConnection.java @@ -548,6 +548,11 @@ public class PeerConnection { // every offer/answer negotiation.This is only intended to be a workaround for crbug.com/835958 public boolean activeResetSrtpParams; + // Whether this client is allowed to switch encoding codec mid-stream. This is a workaround for + // a WebRTC bug where the receiver could get confussed if a codec switch happened mid-call. + // Null indicates no change to currently configured value. + @Nullable public Boolean allowCodecSwitching; + /* * Experimental flag that enables a use of media transport. If this is true, the media transport * factory MUST be provided to the PeerConnectionFactory. @@ -619,6 +624,7 @@ public class PeerConnection { useMediaTransportForDataChannels = false; cryptoOptions = null; turnLoggingId = null; + allowCodecSwitching = null; } @CalledByNative("RTCConfiguration") @@ -828,6 +834,12 @@ public class PeerConnection { return activeResetSrtpParams; } + @Nullable + @CalledByNative("RTCConfiguration") + Boolean getAllowCodecSwitching() { + return allowCodecSwitching; + } + @CalledByNative("RTCConfiguration") boolean getUseMediaTransport() { return useMediaTransport; diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index f40a7bff3a..9b1cce6155 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -279,6 +279,9 @@ void JavaToNativeRTCConfiguration( rtc_config->crypto_options = JavaToNativeOptionalCryptoOptions(jni, j_crypto_options); + rtc_config->allow_codec_switching = JavaToNativeOptionalBool( + jni, Java_RTCConfiguration_getAllowCodecSwitching(jni, j_rtc_config)); + ScopedJavaLocalRef j_turn_logging_id = Java_RTCConfiguration_getTurnLoggingId(jni, j_rtc_config); if (!IsNull(jni, j_turn_logging_id)) {