From 1c5f317e178abd4334903e6beb37b03cc139f80c Mon Sep 17 00:00:00 2001 From: Tommi Date: Sat, 13 Aug 2022 10:43:59 +0200 Subject: [PATCH] Update the red_payload_type without recreating video receive streams. A follow-up change will combine the setters for ulpfec and red payload types, since they're entwined. Bug: webrtc:11993 Change-Id: Ifea7fe9f4ebc7ac88a62db6cd6748f4d3c20db4c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271482 Reviewed-by: Rasmus Brandt Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#37785} --- call/video_receive_stream.h | 2 ++ media/engine/fake_webrtc_call.h | 4 ++++ media/engine/webrtc_video_engine.cc | 18 ++++++++++------- video/rtp_video_stream_receiver2.cc | 30 +++++++++++++++++++++++++---- video/rtp_video_stream_receiver2.h | 4 ++++ video/video_receive_stream2.cc | 5 +++++ video/video_receive_stream2.h | 1 + 7 files changed, 53 insertions(+), 11 deletions(-) diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index 79edf9d5fd..82ad8cfc43 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -305,6 +305,8 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface { virtual void SetUlpfecPayloadType(int ulpfec_payload_type) = 0; + virtual void SetRedPayloadType(int red_payload_type) = 0; + virtual void SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) = 0; protected: diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index f34bb5b1a6..26be7801bf 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -302,6 +302,10 @@ class FakeVideoReceiveStream final config_.rtp.ulpfec_payload_type = ulpfec_payload_type; } + void SetRedPayloadType(int red_payload_type) override { + config_.rtp.red_payload_type = red_payload_type; + } + void SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) override { config_.rtp.rtcp_xr = rtcp_xr; } diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index b02fa4dc51..774368cec4 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2973,16 +2973,18 @@ bool WebRtcVideoChannel::WebRtcVideoReceiveStream::ReconfigureCodecs( raw_payload_types, decoders); const auto& codec = recv_codecs.front(); - if (config_.rtp.ulpfec_payload_type != codec.ulpfec.ulpfec_payload_type) { - config_.rtp.ulpfec_payload_type = codec.ulpfec.ulpfec_payload_type; - stream_->SetUlpfecPayloadType(config_.rtp.ulpfec_payload_type); - } - - bool recreate_needed = false; if (config_.rtp.red_payload_type != codec.ulpfec.red_payload_type) { config_.rtp.red_payload_type = codec.ulpfec.red_payload_type; - recreate_needed = true; + stream_->SetRedPayloadType(codec.ulpfec.red_payload_type); + } + + // Check and optionally set `ulpfec_payload_type` _after_ checking the + // `red_payload_type` due to encapsulation. See RtpVideoStreamReceiver2 + // for more details. + if (config_.rtp.ulpfec_payload_type != codec.ulpfec.ulpfec_payload_type) { + config_.rtp.ulpfec_payload_type = codec.ulpfec.ulpfec_payload_type; + stream_->SetUlpfecPayloadType(config_.rtp.ulpfec_payload_type); } const bool has_lntf = HasLntf(codec.codec); @@ -3020,6 +3022,8 @@ bool WebRtcVideoChannel::WebRtcVideoReceiveStream::ReconfigureCodecs( codec.ulpfec.red_payload_type; } + bool recreate_needed = false; + if (config_.rtp.rtx_associated_payload_types != rtx_associated_payload_types) { rtx_associated_payload_types.swap(config_.rtp.rtx_associated_payload_types); diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 2b765d9d1c..9809ddfebc 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -266,6 +266,7 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( config->rtp.extensions, this, clock_)), + red_payload_type_(config_.rtp.red_payload_type), packet_sink_(config->rtp.packet_sink_), receiving_(false), last_packet_log_ms_(-1), @@ -667,7 +668,7 @@ void RtpVideoStreamReceiver2::OnRecoveredPacket(const uint8_t* rtp_packet, RtpPacketReceived packet; if (!packet.Parse(rtp_packet, rtp_packet_length)) return; - if (packet.PayloadType() == config_.rtp.red_payload_type) { + if (packet.PayloadType() == red_payload_type_) { RTC_LOG(LS_WARNING) << "Discarding recovered packet with RED encapsulation"; return; } @@ -1005,7 +1006,28 @@ int RtpVideoStreamReceiver2::ulpfec_payload_type() const { void RtpVideoStreamReceiver2::set_ulpfec_payload_type(int payload_type) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); ulpfec_receiver_ = MaybeConstructUlpfecReceiver( - config_.rtp.remote_ssrc, config_.rtp.red_payload_type, payload_type, + config_.rtp.remote_ssrc, red_payload_type_, payload_type, + config_.rtp.extensions, this, clock_); +} + +int RtpVideoStreamReceiver2::red_payload_type() const { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + return red_payload_type_; +} + +void RtpVideoStreamReceiver2::set_red_payload_type(int payload_type) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + RTC_DCHECK_GE(payload_type, -1); + RTC_DCHECK_LE(payload_type, 0x7f); + // TODO(tommi, brandtr): It would be less error prone to require both + // red and ulpfec payload ids to be set at the same time. + + // Stash away the currently configured ulpfec payload id to carry over to the + // potentially new `ulpfec_receiver_` instance. + int ulpfec_type = ulpfec_payload_type(); + red_payload_type_ = payload_type; + ulpfec_receiver_ = MaybeConstructUlpfecReceiver( + config_.rtp.remote_ssrc, red_payload_type_, ulpfec_type, config_.rtp.extensions, this, clock_); } @@ -1043,7 +1065,7 @@ void RtpVideoStreamReceiver2::ReceivePacket(const RtpPacketReceived& packet) { NotifyReceiverOfEmptyPacket(packet.SequenceNumber()); return; } - if (packet.PayloadType() == config_.rtp.red_payload_type) { + if (packet.PayloadType() == red_payload_type_) { ParseAndHandleEncapsulatingHeader(packet); return; } @@ -1066,7 +1088,7 @@ void RtpVideoStreamReceiver2::ReceivePacket(const RtpPacketReceived& packet) { void RtpVideoStreamReceiver2::ParseAndHandleEncapsulatingHeader( const RtpPacketReceived& packet) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); - RTC_DCHECK_EQ(packet.PayloadType(), config_.rtp.red_payload_type); + RTC_DCHECK_EQ(packet.PayloadType(), red_payload_type_); if (!ulpfec_receiver_ || packet.payload_size() == 0U) return; diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index ab84007113..8d54d5ceb9 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -203,6 +203,9 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, int ulpfec_payload_type() const; void set_ulpfec_payload_type(int payload_type); + int red_payload_type() const; + void set_red_payload_type(int payload_type); + absl::optional LastReceivedPacketMs() const; absl::optional LastReceivedKeyframePacketMs() const; @@ -327,6 +330,7 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, ReceiveStatistics* const rtp_receive_statistics_; std::unique_ptr ulpfec_receiver_ RTC_GUARDED_BY(packet_sequence_checker_); + int red_payload_type_ RTC_GUARDED_BY(packet_sequence_checker_); RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_task_checker_; // TODO(bugs.webrtc.org/11993): This checker conceptually represents diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 1b8c5194f5..5da6ea95df 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -556,6 +556,11 @@ void VideoReceiveStream2::SetUlpfecPayloadType(int payload_type) { rtp_video_stream_receiver_.set_ulpfec_payload_type(payload_type); } +void VideoReceiveStream2::SetRedPayloadType(int payload_type) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + rtp_video_stream_receiver_.set_red_payload_type(payload_type); +} + void VideoReceiveStream2::SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); rtp_video_stream_receiver_.SetReferenceTimeReport( diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 01cbf73c57..907a7d43cb 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -152,6 +152,7 @@ class VideoReceiveStream2 void SetLossNotificationEnabled(bool enabled) override; void SetNackHistory(TimeDelta history) override; void SetUlpfecPayloadType(int payload_type) override; + void SetRedPayloadType(int payload_type) override; void SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) override; webrtc::VideoReceiveStreamInterface::Stats GetStats() const override;