diff --git a/call/flexfec_receive_stream.h b/call/flexfec_receive_stream.h index 7c8cd2ecbe..4f6fe44afa 100644 --- a/call/flexfec_receive_stream.h +++ b/call/flexfec_receive_stream.h @@ -65,6 +65,10 @@ class FlexfecReceiveStream : public RtpPacketSinkInterface, // Perhaps this should be in ReceiveStreamInterface and apply to audio streams // as well (although there's no logic that would use it at present). virtual void SetRtcpMode(RtcpMode mode) = 0; + + // Called to change the payload type after initialization. + virtual void SetPayloadType(int payload_type) = 0; + virtual int payload_type() const = 0; }; } // namespace webrtc diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index b962f670ae..9e00078f96 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -135,6 +135,7 @@ FlexfecReceiveStreamImpl::FlexfecReceiveStreamImpl( : extension_map_(std::move(config.rtp.extensions)), remote_ssrc_(config.rtp.remote_ssrc), transport_cc_(config.rtp.transport_cc), + payload_type_(config.payload_type), receiver_( MaybeCreateFlexfecReceiver(clock, config, recovered_packet_receiver)), rtp_receive_statistics_(ReceiveStatistics::Create(clock)), @@ -143,6 +144,7 @@ FlexfecReceiveStreamImpl::FlexfecReceiveStreamImpl( config, rtt_stats)) { RTC_LOG(LS_INFO) << "FlexfecReceiveStreamImpl: " << config.ToString(); + RTC_DCHECK_GE(payload_type_, -1); packet_sequence_checker_.Detach(); @@ -188,6 +190,17 @@ void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) { } } +void FlexfecReceiveStreamImpl::SetPayloadType(int payload_type) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + RTC_DCHECK_GE(payload_type, -1); + payload_type_ = payload_type; +} + +int FlexfecReceiveStreamImpl::payload_type() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return payload_type_; +} + void FlexfecReceiveStreamImpl::SetRtpExtensions( std::vector extensions) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h index 21aef795fa..9cb383afee 100644 --- a/call/flexfec_receive_stream_impl.h +++ b/call/flexfec_receive_stream_impl.h @@ -55,6 +55,10 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { // RtpPacketSinkInterface. void OnRtpPacket(const RtpPacketReceived& packet) override; + + void SetPayloadType(int payload_type) override; + int payload_type() const override; + // ReceiveStreamInterface impl. void SetRtpExtensions(std::vector extensions) override; RtpHeaderExtensionMap GetRtpExtensionMap() const override; @@ -88,6 +92,10 @@ class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { const uint32_t remote_ssrc_; bool transport_cc_ RTC_GUARDED_BY(packet_sequence_checker_); + // `payload_type_` is initially set to -1, indicating that FlexFec is + // disabled. + int payload_type_ RTC_GUARDED_BY(packet_sequence_checker_) = -1; + // Erasure code interfacing. const std::unique_ptr receiver_; diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 5a7fddc041..c8d56cbcdc 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -323,6 +323,11 @@ class FakeFlexfecReceiveStream final : public webrtc::FlexfecReceiveStream { } void SetRtcpMode(webrtc::RtcpMode mode) override { config_.rtcp_mode = mode; } + int payload_type() const override { return config_.payload_type; } + void SetPayloadType(int payload_type) override { + config_.payload_type = payload_type; + } + const webrtc::FlexfecReceiveStream::Config& GetConfig() const; uint32_t remote_ssrc() const { return config_.rtp.remote_ssrc; } diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 89bf1f57ad..f05202ac6d 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3035,10 +3035,46 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFeedbackParameters( RecreateReceiveStream(); } +void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFlexFecPayload( + int payload_type, + bool& flexfec_needs_recreation) { + // TODO(bugs.webrtc.org/11993, tommi): See if it is better to always have a + // flexfec stream object around and instead of recreating the video stream, + // reconfigure the flexfec object from within the rtp callback (soon to be on + // the network thread). + if (flexfec_stream_) { + if (flexfec_stream_->payload_type() == payload_type) { + RTC_DCHECK_EQ(flexfec_config_.payload_type, payload_type); + return; + } + + flexfec_config_.payload_type = payload_type; + flexfec_stream_->SetPayloadType(payload_type); + + if (payload_type == -1) { + // TODO(tommi): Delete `flexfec_stream_` and clear references to it from + // `stream_` without having to recreate `stream_`. + flexfec_needs_recreation = true; + } + } else if (payload_type != -1) { + flexfec_config_.payload_type = payload_type; + if (flexfec_config_.IsCompleteAndEnabled()) { + // TODO(tommi): Construct new `flexfec_stream_` configure `stream_` + // without having to recreate `stream_`. + flexfec_needs_recreation = true; + } + } else { + // Noop. No flexfec stream exists and "new" payload_type == -1. + RTC_DCHECK(!flexfec_config_.IsCompleteAndEnabled()); + flexfec_config_.payload_type = payload_type; + } +} + void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters( const ChangedRecvParameters& params) { RTC_DCHECK(stream_); bool video_needs_recreation = false; + bool flexfec_needs_recreation = false; if (params.codec_settings) { video_needs_recreation = ConfigureCodecs(*params.codec_settings); } @@ -3056,17 +3092,17 @@ void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters( } } } - if (params.flexfec_payload_type) { - flexfec_config_.payload_type = *params.flexfec_payload_type; - // TODO(tommi): See if it is better to always have a flexfec stream object - // configured and instead of recreating the video stream, reconfigure the - // flexfec object from within the rtp callback (soon to be on the network - // thread). - if (flexfec_stream_ || flexfec_config_.IsCompleteAndEnabled()) - video_needs_recreation = true; - } - if (video_needs_recreation) { + + if (params.flexfec_payload_type) + SetFlexFecPayload(*params.flexfec_payload_type, flexfec_needs_recreation); + + // TODO(tommi): When `flexfec_needs_recreation` is `true` and + // `video_needs_recreation` is `false`, recreate only the flexfec stream and + // reconfigure the existing `stream_`. + if (video_needs_recreation || flexfec_needs_recreation) { RecreateReceiveStream(); + } else { + RTC_DLOG_F(LS_INFO) << "No receive stream recreate needed."; } } diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index e080edafb3..bce5e055e6 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -505,6 +505,12 @@ class WebRtcVideoChannel : public VideoMediaChannel, void SetLocalSsrc(uint32_t local_ssrc); private: + // Attempts to reconfigure an already existing `flexfec_stream_` or sets + // `flexfec_needs_recreation` to `true` if a new instance needs to be + // created by calling `RecreateReceiveStream()`. In all cases + // `SetFlexFecPayload()` will update `flexfec_config_`. + void SetFlexFecPayload(int payload_type, bool& flexfec_needs_recreation); + void RecreateReceiveStream(); // Applies a new receive codecs configration to `config_`. Returns true