Add ability to set rtp header extensions without recreating streams.
Setting the rtp header extensions on the packet delivery thread
(currently worker, soon to be network), is now possible without
taking the hit of deleting and recreating the receive stream (and
rtp receiver and related state).
Bug: webrtc:11993
Change-Id: I9bbe306844a25d85d79cd216092ead66eaf68960
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223741
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34953}
diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h
index 1c2d11b..17691f7 100644
--- a/call/audio_receive_stream.h
+++ b/call/audio_receive_stream.h
@@ -166,10 +166,6 @@
int history_ms) = 0;
virtual void SetNonSenderRttMeasurement(bool enabled) = 0;
- // Set/change the rtp header extensions. Must be called on the packet
- // delivery thread.
- virtual void SetRtpExtensions(std::vector<RtpExtension> extensions) = 0;
-
// Returns true if the stream has been started.
virtual bool IsRunning() const = 0;
diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc
index 7230a4a..eda5c7f 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -14,7 +14,7 @@
#include <cstdint>
#include <string>
-#include <vector>
+#include <utility>
#include "api/array_view.h"
#include "api/call/transport.h"
@@ -184,6 +184,7 @@
}
void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
if (!receiver_)
return;
@@ -201,4 +202,10 @@
return FlexfecReceiveStream::Stats();
}
+void FlexfecReceiveStreamImpl::SetRtpExtensions(
+ std::vector<RtpExtension> extensions) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ config_.rtp.extensions = std::move(extensions);
+}
+
} // namespace webrtc
diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h
index 285a33f..c2407cd 100644
--- a/call/flexfec_receive_stream_impl.h
+++ b/call/flexfec_receive_stream_impl.h
@@ -12,6 +12,7 @@
#define CALL_FLEXFEC_RECEIVE_STREAM_IMPL_H_
#include <memory>
+#include <vector>
#include "call/flexfec_receive_stream.h"
#include "call/rtp_packet_sink_interface.h"
@@ -58,13 +59,14 @@
Stats GetStats() const override;
// ReceiveStream impl.
+ void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
const RtpConfig& rtp_config() const override { return config_.rtp; }
private:
RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_sequence_checker_;
- // Config.
- const Config config_;
+ // Config. Mostly const, header extensions may change.
+ Config config_ RTC_GUARDED_BY(packet_sequence_checker_);
// Erasure code interfacing.
const std::unique_ptr<FlexfecReceiver> receiver_;
diff --git a/call/receive_stream.h b/call/receive_stream.h
index 0f59b37..a6756fc 100644
--- a/call/receive_stream.h
+++ b/call/receive_stream.h
@@ -51,6 +51,10 @@
std::vector<RtpExtension> extensions;
};
+ // Set/change the rtp header extensions. Must be called on the packet
+ // delivery thread.
+ virtual void SetRtpExtensions(std::vector<RtpExtension> extensions) = 0;
+
// Called on the packet delivery thread since some members of the config may
// change mid-stream (e.g. the local ssrc). All mutation must also happen on
// the packet delivery thread. Return value can be assumed to
diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc
index f61aab0..62bcab9 100644
--- a/media/engine/fake_webrtc_call.cc
+++ b/media/engine/fake_webrtc_call.cc
@@ -375,6 +375,11 @@
return stats_;
}
+void FakeVideoReceiveStream::SetRtpExtensions(
+ std::vector<webrtc::RtpExtension> extensions) {
+ config_.rtp.extensions = std::move(extensions);
+}
+
void FakeVideoReceiveStream::Start() {
receiving_ = true;
}
@@ -392,6 +397,11 @@
const webrtc::FlexfecReceiveStream::Config& config)
: config_(config) {}
+void FakeFlexfecReceiveStream::SetRtpExtensions(
+ std::vector<webrtc::RtpExtension> extensions) {
+ config_.rtp.extensions = std::move(extensions);
+}
+
const webrtc::FlexfecReceiveStream::Config&
FakeFlexfecReceiveStream::GetConfig() const {
return config_;
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 5f6b864..e732379 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -264,9 +264,12 @@
private:
// webrtc::VideoReceiveStream implementation.
+ void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
+
const webrtc::ReceiveStream::RtpConfig& rtp_config() const override {
return config_.rtp;
}
+
void Start() override;
void Stop() override;
@@ -293,6 +296,8 @@
explicit FakeFlexfecReceiveStream(
const webrtc::FlexfecReceiveStream::Config& config);
+ void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
+
const webrtc::ReceiveStream::RtpConfig& rtp_config() const override {
return config_.rtp;
}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 72903c6..a630b45 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -3018,13 +3018,20 @@
if (params.rtp_header_extensions) {
if (config_.rtp.extensions != *params.rtp_header_extensions) {
config_.rtp.extensions = *params.rtp_header_extensions;
- video_needs_recreation = true;
+ if (stream_) {
+ stream_->SetRtpExtensions(config_.rtp.extensions);
+ } else {
+ video_needs_recreation = true;
+ }
}
if (flexfec_config_.rtp.extensions != *params.rtp_header_extensions) {
flexfec_config_.rtp.extensions = *params.rtp_header_extensions;
- if (flexfec_stream_ || flexfec_config_.IsCompleteAndEnabled())
+ if (flexfec_stream_) {
+ flexfec_stream_->SetRtpExtensions(flexfec_config_.rtp.extensions);
+ } else if (flexfec_config_.IsCompleteAndEnabled()) {
video_needs_recreation = true;
+ }
}
}
if (params.flexfec_payload_type) {
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 025a553..be431cb 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -3071,11 +3071,11 @@
EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams());
- // Setting different extensions should recreate the stream.
+ // Setting different extensions should not require the stream to be recreated.
recv_parameters_.extensions.resize(1);
EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_));
- EXPECT_EQ(2, fake_call_->GetNumCreatedReceiveStreams());
+ EXPECT_EQ(1, fake_call_->GetNumCreatedReceiveStreams());
}
TEST_F(WebRtcVideoChannelTest,
diff --git a/modules/rtp_rtcp/include/rtp_header_extension_map.h b/modules/rtp_rtcp/include/rtp_header_extension_map.h
index 72e5541..a746d8a 100644
--- a/modules/rtp_rtcp/include/rtp_header_extension_map.h
+++ b/modules/rtp_rtcp/include/rtp_header_extension_map.h
@@ -31,6 +31,8 @@
explicit RtpHeaderExtensionMap(bool extmap_allow_mixed);
explicit RtpHeaderExtensionMap(rtc::ArrayView<const RtpExtension> extensions);
+ void Reset(rtc::ArrayView<const RtpExtension> extensions);
+
template <typename Extension>
bool Register(int id) {
return Register(id, Extension::kId, Extension::kUri);
diff --git a/modules/rtp_rtcp/include/ulpfec_receiver.h b/modules/rtp_rtcp/include/ulpfec_receiver.h
index bf1c826..6cbae52 100644
--- a/modules/rtp_rtcp/include/ulpfec_receiver.h
+++ b/modules/rtp_rtcp/include/ulpfec_receiver.h
@@ -53,6 +53,9 @@
// Returns a counter describing the added and recovered packets.
virtual FecPacketCounter GetPacketCounter() const = 0;
+
+ virtual void SetRtpExtensions(
+ rtc::ArrayView<const RtpExtension> extensions) = 0;
};
} // namespace webrtc
#endif // MODULES_RTP_RTCP_INCLUDE_ULPFEC_RECEIVER_H_
diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc
index d1eee22..ddd5793 100644
--- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc
@@ -80,6 +80,14 @@
RegisterByUri(extension.id, extension.uri);
}
+void RtpHeaderExtensionMap::Reset(
+ rtc::ArrayView<const RtpExtension> extensions) {
+ for (auto& id : ids_)
+ id = kInvalidId;
+ for (const RtpExtension& extension : extensions)
+ RegisterByUri(extension.id, extension.uri);
+}
+
bool RtpHeaderExtensionMap::RegisterByType(int id, RTPExtensionType type) {
for (const ExtensionInfo& extension : kExtensions)
if (type == extension.type)
diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
index c993923..159e21f 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
@@ -47,6 +47,12 @@
return packet_counter_;
}
+void UlpfecReceiverImpl::SetRtpExtensions(
+ rtc::ArrayView<const RtpExtension> extensions) {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ extensions_.Reset(extensions);
+}
+
// 0 1 2 3
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
index f59251f..92e5153 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h
@@ -41,9 +41,11 @@
FecPacketCounter GetPacketCounter() const override;
+ void SetRtpExtensions(rtc::ArrayView<const RtpExtension> extensions) override;
+
private:
const uint32_t ssrc_;
- const RtpHeaderExtensionMap extensions_;
+ RtpHeaderExtensionMap extensions_ RTC_GUARDED_BY(&sequence_checker_);
RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_;
RecoveredPacketReceiver* const recovered_packet_callback_;
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc
index b4a139e..309d1588 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -907,6 +907,12 @@
frame_transformer_delegate_->Init();
}
+void RtpVideoStreamReceiver2::SetRtpExtensions(
+ const std::vector<RtpExtension>& extensions) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ rtp_header_extensions_.Reset(extensions);
+}
+
void RtpVideoStreamReceiver2::UpdateRtt(int64_t max_rtt_ms) {
RTC_DCHECK_RUN_ON(&worker_task_checker_);
if (nack_module_)
diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h
index 3a4f58e..ff9e43f 100644
--- a/video/rtp_video_stream_receiver2.h
+++ b/video/rtp_video_stream_receiver2.h
@@ -176,6 +176,10 @@
void SetDepacketizerToDecoderFrameTransformer(
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer);
+ // Updates the rtp header extensions at runtime. Must be called on the
+ // `packet_sequence_checker_` thread.
+ void SetRtpExtensions(const std::vector<RtpExtension>& extensions);
+
// Called by VideoReceiveStream when stats are updated.
void UpdateRtt(int64_t max_rtt_ms);
@@ -290,7 +294,8 @@
RemoteNtpTimeEstimator ntp_estimator_;
- RtpHeaderExtensionMap rtp_header_extensions_;
+ RtpHeaderExtensionMap rtp_header_extensions_
+ RTC_GUARDED_BY(packet_sequence_checker_);
// Set by the field trial WebRTC-ForcePlayoutDelay to override any playout
// delay that is specified in the received packets.
FieldTrialOptional<int> forced_playout_delay_max_ms_;
diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc
index e2e2bb8..6823a1e 100644
--- a/video/video_receive_stream.cc
+++ b/video/video_receive_stream.cc
@@ -456,6 +456,12 @@
rtp_video_stream_receiver_.RemoveSecondarySink(sink);
}
+void VideoReceiveStream::SetRtpExtensions(
+ std::vector<RtpExtension> extensions) {
+ // VideoReceiveStream is deprecated and this function not supported.
+ RTC_NOTREACHED();
+}
+
bool VideoReceiveStream::SetBaseMinimumPlayoutDelayMs(int delay_ms) {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
if (delay_ms < kMinBaseMinimumDelayMs || delay_ms > kMaxBaseMinimumDelayMs) {
diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h
index 637b91a..bbf0367 100644
--- a/video/video_receive_stream.h
+++ b/video/video_receive_stream.h
@@ -93,6 +93,7 @@
void AddSecondarySink(RtpPacketSinkInterface* sink) override;
void RemoveSecondarySink(const RtpPacketSinkInterface* sink) override;
+ void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
// SetBaseMinimumPlayoutDelayMs and GetBaseMinimumPlayoutDelayMs are called
// from webrtc/api level and requested by user code. For e.g. blink/js layer
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 3127069..725a9bb 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -219,8 +219,8 @@
this, // NackSender
nullptr, // Use default KeyFrameRequestSender
this, // OnCompleteFrameCallback
- config_.frame_decryptor,
- config_.frame_transformer),
+ std::move(config_.frame_decryptor),
+ std::move(config_.frame_transformer)),
rtp_stream_sync_(call->worker_thread(), this),
max_wait_for_keyframe_ms_(DetermineMaxWaitForFrame(config, true)),
max_wait_for_frame_ms_(DetermineMaxWaitForFrame(config, false)),
@@ -450,6 +450,25 @@
transport_adapter_.Disable();
}
+void VideoReceiveStream2::SetRtpExtensions(
+ std::vector<RtpExtension> extensions) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ rtp_video_stream_receiver_.SetRtpExtensions(extensions);
+ // TODO(tommi): We don't use the `c.rtp.extensions` member in the
+ // VideoReceiveStream2 class, so this const_cast<> is a temporary hack to keep
+ // things consistent between VideoReceiveStream2 and RtpVideoStreamReceiver2
+ // for debugging purposes. The `packet_sequence_checker_` gives us assurances
+ // that from a threading perspective, this is still safe. The accessors that
+ // give read access to this state, run behind the same check.
+ // The alternative to the const_cast<> would be to make `config_` non-const
+ // and guarded by `packet_sequence_checker_`. However the scope of that state
+ // is huge (the whole Config struct), and would require all methods that touch
+ // the struct to abide the needs of the `extensions` member.
+ VideoReceiveStream::Config& c =
+ const_cast<VideoReceiveStream::Config&>(config_);
+ c.rtp.extensions = std::move(extensions);
+}
+
void VideoReceiveStream2::CreateAndRegisterExternalDecoder(
const Decoder& decoder) {
TRACE_EVENT0("webrtc",
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index ace5de2..cf637f8 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -134,7 +134,9 @@
void Start() override;
void Stop() override;
- const RtpConfig& rtp_config() const override { return config_.rtp; }
+ void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
+
+ const RtpConfig& rtp_config() const override { return rtp(); }
webrtc::VideoReceiveStream::Stats GetStats() const override;
