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 <brandtr@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37785}
diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h
index 79edf9d..82ad8cf 100644
--- a/call/video_receive_stream.h
+++ b/call/video_receive_stream.h
@@ -305,6 +305,8 @@
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 f34bb5b..26be780 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -302,6 +302,10 @@
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 b02fa4d..774368c 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2973,16 +2973,18 @@
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 @@
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 2b765d9..9809ddf 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -266,6 +266,7 @@
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 @@
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 @@
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 @@
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::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 ab84007..8d54d5c 100644
--- a/video/rtp_video_stream_receiver2.h
+++ b/video/rtp_video_stream_receiver2.h
@@ -203,6 +203,9 @@
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<int64_t> LastReceivedPacketMs() const;
absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
@@ -327,6 +330,7 @@
ReceiveStatistics* const rtp_receive_statistics_;
std::unique_ptr<UlpfecReceiver> 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 1b8c519..5da6ea9 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -556,6 +556,11 @@
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 01cbf73..907a7d4 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -152,6 +152,7 @@
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;
