Add SetPayloadType to FlexfecReceiveStream.
This reduces the number of times we recreate video receive streams
and prepares for not having to do that for flexfec streams and avoid
having to recreate a video receive stream when flexfec config changes.
Bug: webrtc:11993
Change-Id: I11134b6a72eb162623ebbc12521d409da8616107
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261941
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37641}
diff --git a/call/flexfec_receive_stream.h b/call/flexfec_receive_stream.h
index 7c8cd2e..4f6fe44 100644
--- a/call/flexfec_receive_stream.h
+++ b/call/flexfec_receive_stream.h
@@ -65,6 +65,10 @@
// 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 b962f67..9e00078 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -135,6 +135,7 @@
: 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 @@
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::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<RtpExtension> 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 21aef79..9cb383a 100644
--- a/call/flexfec_receive_stream_impl.h
+++ b/call/flexfec_receive_stream_impl.h
@@ -55,6 +55,10 @@
// RtpPacketSinkInterface.
void OnRtpPacket(const RtpPacketReceived& packet) override;
+
+ void SetPayloadType(int payload_type) override;
+ int payload_type() const override;
+
// ReceiveStreamInterface impl.
void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
RtpHeaderExtensionMap GetRtpExtensionMap() const override;
@@ -88,6 +92,10 @@
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<FlexfecReceiver> receiver_;
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 5a7fddc..c8d56cb 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -323,6 +323,11 @@
}
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 89bf1f5..f05202a 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -3035,10 +3035,46 @@
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 @@
}
}
}
- 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 e080eda..bce5e05 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -505,6 +505,12 @@
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
