Provide RtpState at construction when restoring an RTP module When a suspended stream was recreated, its previous state was applied with SetRtpState() and SetRtxState() right after construction. This required state to be mutable that otherwise could be const, which in turn required a lock for every read on the encoder queue. * Add rtp_state and rtx_rtp_state to RtpRtcpInterface::Configuration. * Apply it when constructing RTPSender and ModuleRtpRtcpImpl2. * RtpVideoSender and AudioSendStream, via ChannelSend, now pass the suspended states at creation. * Remove SetStartTimestamp(), SetRtpState() and SetRtxState() from RtpRtcpInterface, ModuleRtpRtcpImpl2, as well as RTPSender::SetRtxRtpState(). * Deprecate RTPSender::SetTimestampOffset() and SetRtpState(). Once direct users of RTPSender have migrated, they can be removed and the timestamp offset made const. Bug: webrtc:42223727 Change-Id: Idad26fe1205c1288a3455865dfb79027ed7f83da Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/507320 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48831}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 4bc85b2..15ca876 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc
@@ -132,7 +132,6 @@ audio_state, rtp_transport, bitrate_allocator, - suspended_rtp_state, voe::CreateChannelSend(env, config.send_transport, rtcp_rtt_stats, @@ -141,6 +140,7 @@ config.rtp.extmap_allow_mixed, config.rtcp_report_interval_ms, config.rtp.ssrc, + suspended_rtp_state, config.frame_transformer, rtp_transport)) {} @@ -150,7 +150,6 @@ const scoped_refptr<webrtc::AudioState>& audio_state, RtpTransportControllerSendInterface* rtp_transport, BitrateAllocatorInterface* bitrate_allocator, - const std::optional<RtpState>& suspended_rtp_state, std::unique_ptr<voe::ChannelSendInterface> channel_send) : env_(env), allocate_audio_without_feedback_( @@ -167,8 +166,7 @@ !env_.field_trials().IsDisabled("WebRTC-Audio-PriorityBitrate")), bitrate_allocator_(bitrate_allocator), rtp_transport_(rtp_transport), - rtp_rtcp_module_(channel_send_->GetRtpRtcp()), - suspended_rtp_state_(suspended_rtp_state) { + rtp_rtcp_module_(channel_send_->GetRtpRtcp()) { RTC_LOG(LS_INFO) << "AudioSendStream: " << config.rtp.ssrc; RTC_DCHECK(audio_state_); RTC_DCHECK(channel_send_); @@ -243,9 +241,6 @@ RTC_DCHECK(first_time || old_config.send_transport == new_config.send_transport); RTC_DCHECK(first_time || old_config.rtp.ssrc == new_config.rtp.ssrc); - if (suspended_rtp_state_ && first_time) { - rtp_rtcp_module_->SetRtpState(*suspended_rtp_state_); - } if (first_time || old_config.rtp.c_name != new_config.rtp.c_name) { channel_send_->SetRTCP_CNAME(new_config.rtp.c_name); }
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index b7eca51..635d0de 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h
@@ -79,7 +79,6 @@ const scoped_refptr<webrtc::AudioState>& audio_state, RtpTransportControllerSendInterface* rtp_transport, BitrateAllocatorInterface* bitrate_allocator, - const std::optional<RtpState>& suspended_rtp_state, std::unique_ptr<voe::ChannelSendInterface> channel_send); AudioSendStream() = delete; @@ -190,7 +189,6 @@ RtpTransportControllerSendInterface* const rtp_transport_; RtpRtcpInterface* const rtp_rtcp_module_; - std::optional<RtpState> const suspended_rtp_state_; // RFC 8285: Each distinct extension MUST have a unique ID. // The ID is picked in the SDP offer/answer process; if no ID is
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 26559a0..dc103ec 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc
@@ -211,7 +211,6 @@ CreateEnvironment(&field_trials_, time_controller_.GetClock(), time_controller_.GetTaskQueueFactory()), stream_config_, audio_state_, &rtp_transport_, &bitrate_allocator_, - std::nullopt, std::unique_ptr<voe::ChannelSendInterface>(channel_send_)); }
diff --git a/audio/channel_send.cc b/audio/channel_send.cc index 0539bdc..10dccdd 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc
@@ -135,6 +135,7 @@ bool extmap_allow_mixed, int rtcp_report_interval_ms, uint32_t ssrc, + const std::optional<RtpState>& rtp_state, scoped_refptr<FrameTransformerInterface> frame_transformer, RtpTransportControllerSendInterface* transport_controller); @@ -512,6 +513,7 @@ bool extmap_allow_mixed, int rtcp_report_interval_ms, uint32_t ssrc, + const std::optional<RtpState>& rtp_state, scoped_refptr<FrameTransformerInterface> frame_transformer, RtpTransportControllerSendInterface* transport_controller) : env_(env), @@ -542,6 +544,7 @@ configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; configuration.rtcp_packet_type_counter_observer = this; configuration.local_media_ssrc = ssrc; + configuration.rtp_state = rtp_state; configuration.rtcp_mode = RtcpMode::kCompound; rtp_rtcp_ = ModuleRtpRtcpImpl2::CreateSendModule(env_, configuration); @@ -1017,11 +1020,12 @@ bool extmap_allow_mixed, int rtcp_report_interval_ms, uint32_t ssrc, + const std::optional<RtpState>& rtp_state, scoped_refptr<FrameTransformerInterface> frame_transformer, RtpTransportControllerSendInterface* transport_controller) { return std::make_unique<ChannelSend>( env, rtp_transport, rtcp_rtt_stats, frame_encryptor, crypto_options, - extmap_allow_mixed, rtcp_report_interval_ms, ssrc, + extmap_allow_mixed, rtcp_report_interval_ms, ssrc, rtp_state, std::move(frame_transformer), transport_controller); }
diff --git a/audio/channel_send.h b/audio/channel_send.h index 750d2a9..0b1e328 100644 --- a/audio/channel_send.h +++ b/audio/channel_send.h
@@ -123,6 +123,8 @@ virtual void RegisterPacketOverhead(int packet_byte_overhead) = 0; }; +// `rtp_state` is the initial state of the RTP stream, e.g. when a stream with +// the same SSRC is recreated. std::unique_ptr<ChannelSendInterface> CreateChannelSend( const Environment& env, Transport* rtp_transport, @@ -132,6 +134,7 @@ bool extmap_allow_mixed, int rtcp_report_interval_ms, uint32_t ssrc, + const std::optional<RtpState>& rtp_state, scoped_refptr<FrameTransformerInterface> frame_transformer, RtpTransportControllerSendInterface* transport_controller);
diff --git a/audio/channel_send_unittest.cc b/audio/channel_send_unittest.cc index df987e1..8f2282f 100644 --- a/audio/channel_send_unittest.cc +++ b/audio/channel_send_unittest.cc
@@ -91,7 +91,8 @@ .worker_thread = TaskQueueBase::Current()}) { channel_ = voe::CreateChannelSend(env_, &transport_, nullptr, nullptr, crypto_options_, false, kRtcpIntervalMs, - kSsrc, nullptr, &transport_controller_); + kSsrc, /*rtp_state=*/std::nullopt, + nullptr, &transport_controller_); encoder_factory_ = CreateBuiltinAudioEncoderFactory(); SdpAudioFormat opus = SdpAudioFormat("opus", kRtpRateHz, 2); std::unique_ptr<AudioEncoder> encoder =
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index eaf4d88..06d59d9 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc
@@ -222,15 +222,22 @@ return nullptr; } +// Returns the state that the stream with `ssrc` had when it was suspended, if +// any. +std::optional<RtpState> FindSuspendedRtpState( + const std::map<uint32_t, RtpState>& suspended_ssrcs, + uint32_t ssrc) { + auto it = suspended_ssrcs.find(ssrc); + if (it == suspended_ssrcs.end()) { + return std::nullopt; + } + return it->second; +} + // Configures RTX for the media stream at `simulcast_index`. void ConfigureRtx(const RtpConfig& rtp_config, - const std::map<uint32_t, RtpState>& suspended_ssrcs, size_t simulcast_index, RtpRtcpInterface& rtp_rtcp) { - auto it = suspended_ssrcs.find(rtp_config.rtx.ssrcs[simulcast_index]); - if (it != suspended_ssrcs.end()) - rtp_rtcp.SetRtxState(it->second); - // Configure RTX payload types. RTC_DCHECK_GE(rtp_config.rtx.payload_type, 0); RtpStreamConfig stream_config = rtp_config.GetStreamConfig(simulcast_index); @@ -247,7 +254,6 @@ // Configures the RTP module that sends the media stream at `simulcast_index`. void ConfigureRtpModule(const RtpConfig& rtp_config, - const std::map<uint32_t, RtpState>& suspended_ssrcs, size_t simulcast_index, RtpRtcpInterface& rtp_rtcp) { rtp_rtcp.SetSendingStatus(false); @@ -261,14 +267,9 @@ rtp_rtcp.RegisterRtpHeaderExtension(extension.uri, extension.id); } - // Restore RTP state if previous existed. - auto it = suspended_ssrcs.find(rtp_config.ssrcs[simulcast_index]); - if (it != suspended_ssrcs.end()) - rtp_rtcp.SetRtpState(it->second); - // Set up RTX if available. if (!rtp_config.rtx.ssrcs.empty()) - ConfigureRtx(rtp_config, suspended_ssrcs, simulcast_index, rtp_rtcp); + ConfigureRtx(rtp_config, simulcast_index, rtp_rtcp); if (!rtp_config.mid.empty()) rtp_rtcp.SetMid(rtp_config.mid); @@ -344,12 +345,21 @@ RTC_DCHECK_EQ(configuration.rtx_send_ssrc.has_value(), !rtp_config.rtx.ssrcs.empty()); + // Restore RTP state if previous existed. + configuration.rtp_state = + FindSuspendedRtpState(suspended_ssrcs, rtp_config.ssrcs[i]); + configuration.rtx_rtp_state = + configuration.rtx_send_ssrc.has_value() + ? FindSuspendedRtpState(suspended_ssrcs, + *configuration.rtx_send_ssrc) + : std::nullopt; + configuration.rid = (i < rtp_config.rids.size()) ? rtp_config.rids[i] : ""; configuration.need_rtp_packet_infos = rtp_config.lntf.enabled; auto rtp_rtcp = ModuleRtpRtcpImpl2::CreateSendModule(env, configuration); - ConfigureRtpModule(rtp_config, suspended_ssrcs, i, *rtp_rtcp); + ConfigureRtpModule(rtp_config, i, *rtp_rtcp); video_config.clock = &env.clock(); video_config.rtp_sender = rtp_rtcp->RtpSender();
diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 1dfea77..7e21ac4 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h
@@ -63,11 +63,8 @@ MOCK_METHOD(bool, SupportsPadding, (), (const, override)); MOCK_METHOD(bool, SupportsRtxPayloadPadding, (), (const, override)); MOCK_METHOD(uint32_t, StartTimestamp, (), (const, override)); - MOCK_METHOD(void, SetStartTimestamp, (uint32_t timestamp), (override)); MOCK_METHOD(uint16_t, SequenceNumber, (), (const, override)); MOCK_METHOD(void, SetSequenceNumber, (uint16_t seq), (override)); - MOCK_METHOD(void, SetRtpState, (const RtpState& rtp_state), (override)); - MOCK_METHOD(void, SetRtxState, (const RtpState& rtp_state), (override)); MOCK_METHOD(void, SetNonSenderRttMeasurement, (bool enabled), (override)); MOCK_METHOD(RtpState, GetRtpState, (), (const, override)); MOCK_METHOD(RtpState, GetRtxState, (), (const, override));
diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc index 8000e8b..1a71d86 100644 --- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
@@ -170,6 +170,7 @@ configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; configuration.local_media_ssrc = kTestSsrc; configuration.rtx_send_ssrc = kTestRtxSsrc; + configuration.rtp_state = RtpState{.start_timestamp = 111111}; configuration.rtcp_mode = RtcpMode::kCompound; rtp_rtcp_module_ = ModuleRtpRtcpImpl2::CreateSendModule(env_, configuration); @@ -181,7 +182,6 @@ rtp_rtcp_module_->SetStorePacketsStatus(true, 600); EXPECT_EQ(0, rtp_rtcp_module_->SetSendingStatus(true)); rtp_rtcp_module_->SetSequenceNumber(kTestSequenceNumber); - rtp_rtcp_module_->SetStartTimestamp(111111); // Used for NACK processing. rtp_rtcp_module_->SetRemoteSSRC(kTestSsrc);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index d28e855..ea23613 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -74,7 +74,16 @@ env, config, &packet_history, - config.paced_sender ? config.paced_sender : &non_paced_sender) {} + config.paced_sender ? config.paced_sender : &non_paced_sender) { + // `packet_generator` has already read the start timestamp and the ack state + // from the same configuration. + if (config.rtp_state.has_value()) { + sequencer.SetRtpState(*config.rtp_state); + } + if (config.rtx_rtp_state.has_value()) { + sequencer.set_rtx_sequence_number(config.rtx_rtp_state->sequence_number); + } +} ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2( const Environment& env, @@ -196,13 +205,6 @@ return rtp_sender_->packet_generator.TimestampOffset(); } -// Configure start timestamp, default is a random number. -void ModuleRtpRtcpImpl2::SetStartTimestamp(const uint32_t timestamp) { - rtcp_sender_.SetTimestampOffset(timestamp); - rtp_sender_->packet_generator.SetTimestampOffset(timestamp); - rtp_sender_->packet_sender.SetTimestampOffset(timestamp); -} - uint16_t ModuleRtpRtcpImpl2::SequenceNumber() const { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); return rtp_sender_->sequencer.media_sequence_number(); @@ -217,20 +219,6 @@ } } -void ModuleRtpRtcpImpl2::SetRtpState(const RtpState& rtp_state) { - RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - rtp_sender_->packet_generator.SetRtpState(rtp_state); - rtp_sender_->sequencer.SetRtpState(rtp_state); - rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp); - rtp_sender_->packet_sender.SetTimestampOffset(rtp_state.start_timestamp); -} - -void ModuleRtpRtcpImpl2::SetRtxState(const RtpState& rtp_state) { - RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); - rtp_sender_->packet_generator.SetRtxRtpState(rtp_state); - rtp_sender_->sequencer.set_rtx_sequence_number(rtp_state.sequence_number); -} - RtpState ModuleRtpRtcpImpl2::GetRtpState() const { RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); RtpState state = rtp_sender_->packet_generator.GetRtpState();
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index f962f0d..e55f35c 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h
@@ -120,16 +120,11 @@ // Get start timestamp. uint32_t StartTimestamp() const override; - // Configure start timestamp, default is a random number. - void SetStartTimestamp(uint32_t timestamp) override; - uint16_t SequenceNumber() const override; // Set SequenceNumber, default is a random number. void SetSequenceNumber(uint16_t seq) override; - void SetRtpState(const RtpState& rtp_state) override; - void SetRtxState(const RtpState& rtp_state) override; RtpState GetRtpState() const override; RtpState GetRtxState() const override;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 77d0ece..3474312 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -244,6 +244,15 @@ fec_generator_ = fec_generator; CreateModuleImpl(); } + // Recreates the module with the given initial RTP states, as when a + // suspended stream is resumed. + void ReinitWithRtpState( + const RtpState& rtp_state, + std::optional<RtpState> rtx_rtp_state = std::nullopt) { + rtp_state_ = rtp_state; + rtx_rtp_state_ = rtx_rtp_state; + CreateModuleImpl(); + } void CreateModuleImpl() { RtpRtcpInterface::Configuration config; @@ -256,6 +265,8 @@ config.local_media_ssrc = is_sender_ ? kSenderSsrc : kReceiverSsrc; config.rtx_send_ssrc = is_sender_ ? std::make_optional(kRtxSenderSsrc) : std::nullopt; + config.rtp_state = rtp_state_; + config.rtx_rtp_state = rtx_rtp_state_; config.need_rtp_packet_infos = true; config.non_sender_rtt_measurement = true; config.send_packet_observer = this; @@ -272,6 +283,8 @@ std::optional<SentPacket> last_sent_packet_; VideoFecGenerator* fec_generator_ = nullptr; TimeDelta rtcp_report_interval_ = kDefaultReportInterval; + std::optional<RtpState> rtp_state_; + std::optional<RtpState> rtx_rtp_state_; }; } // namespace @@ -699,11 +712,11 @@ RtpState saved_rtp_state = sender_.impl_->GetRtpState(); - // Change RTP timestamp offset. - sender_.impl_->SetStartTimestamp(2000); - - // Restores RtpState and make sure the old timestamp offset is in place. - sender_.impl_->SetRtpState(saved_rtp_state); + // Recreate the sender with the saved RtpState and make sure the old + // timestamp offset is in place. + sender_.ReinitWithRtpState(saved_rtp_state); + SetUp(); + EXPECT_EQ(sender_.impl_->StartTimestamp(), saved_rtp_state.start_timestamp); seqno = sender_.impl_->GetRtpState().sequence_number; media_rtp_ts = 1031; rtp_ts = media_rtp_ts + sender_.impl_->StartTimestamp(); @@ -717,8 +730,8 @@ TEST_F(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { const uint32_t kStartTimestamp = 1u; + sender_.ReinitWithRtpState(RtpState{.start_timestamp = kStartTimestamp}); SetUp(); - sender_.impl_->SetStartTimestamp(kStartTimestamp); sender_.impl_->SetSequenceNumber(1); @@ -1081,8 +1094,9 @@ const Timestamp capture_time = time; const uint32_t timestamp = capture_time.ms() * kCaptureTimeMsToRtpTimestamp; + sender_.ReinitWithRtpState(RtpState{.start_timestamp = kStartTimestamp}); + SetUp(); sender_.impl_->SetSequenceNumber(kSeq - 1); - sender_.impl_->SetStartTimestamp(kStartTimestamp); EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); // Simulate an RTCP receiver report in order to populate `ssrc_has_acked`. @@ -1099,11 +1113,10 @@ EXPECT_EQ(state.last_timestamp_time, time); EXPECT_EQ(state.ssrc_has_acked, true); - // Reset sender, advance time, restore state. Directly observing state + // Advance time, recreate the sender with the state. Directly observing state // is not feasible, so just verify returned state matches what we set. - sender_.CreateModuleImpl(); time_controller_.AdvanceTime(TimeDelta::Millis(10)); - sender_.impl_->SetRtpState(state); + sender_.ReinitWithRtpState(state); state = sender_.impl_->GetRtpState(); EXPECT_EQ(state.sequence_number, kSeq); @@ -1115,15 +1128,16 @@ } TEST_F(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { + // `start_timestamp` is the only timestamp populate in the RTX state. + const uint32_t kStartTimestamp = 3456; + sender_.ReinitWithRtpState(RtpState{.start_timestamp = kStartTimestamp}); + SetUp(); + // Enable RTX. sender_.impl_->SetStorePacketsStatus(/*enable=*/true, /*number_to_store=*/10); sender_.impl_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); sender_.impl_->SetRtxSendStatus(kRtxRetransmitted | kRtxRedundantPayloads); - // `start_timestamp` is the only timestamp populate in the RTX state. - const uint32_t kStartTimestamp = 3456; - sender_.impl_->SetStartTimestamp(kStartTimestamp); - // Send a frame and ask for a retransmit of the last packet. Capture the RTX // packet in order to verify RTX sequence number. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); @@ -1145,13 +1159,11 @@ EXPECT_EQ(rtx_state.ssrc_has_acked, true); EXPECT_EQ(rtx_state.sequence_number, rtx_packet.SequenceNumber() + 1); - // Reset sender, advance time, restore state. Directly observing state - // is not feasible, so just verify returned state matches what we set. - // Needs SetRtpState() too in order to propagate start timestamp. - sender_.CreateModuleImpl(); + // Advance time, recreate the sender with the states. Directly observing + // state is not feasible, so just verify returned state matches what we set. + // Needs `rtp_state` too in order to propagate start timestamp. time_controller_.AdvanceTime(TimeDelta::Millis(10)); - sender_.impl_->SetRtpState(rtp_state); - sender_.impl_->SetRtxState(rtx_state); + sender_.ReinitWithRtpState(rtp_state, rtx_state); rtx_state = sender_.impl_->GetRtxState(); EXPECT_EQ(rtx_state.start_timestamp, kStartTimestamp);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 1d7f923..70be760 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h
@@ -127,6 +127,15 @@ std::optional<uint32_t> rtx_send_ssrc; std::optional<uint32_t> remote_ssrc; + // Initial state of the media and RTX streams, e.g. when a stream is + // recreated after being suspended. A stream without a state starts with a + // random sequence number, and the media stream with a random start + // timestamp. Only `sequence_number` and `ssrc_has_acked` are used from + // `rtx_rtp_state`. `RTPSender` only uses `start_timestamp` and + // `ssrc_has_acked`. + std::optional<RtpState> rtp_state; + std::optional<RtpState> rtx_rtp_state; + bool need_rtp_packet_infos = false; // Estimate RTT as non-sender as described in @@ -226,18 +235,12 @@ // Returns start timestamp. virtual uint32_t StartTimestamp() const = 0; - // Sets start timestamp. Start timestamp is set to a random value if this - // function is never called. - virtual void SetStartTimestamp(uint32_t timestamp) = 0; - // Returns SequenceNumber. virtual uint16_t SequenceNumber() const = 0; // Sets SequenceNumber, default is a random number. virtual void SetSequenceNumber(uint16_t seq) = 0; - virtual void SetRtpState(const RtpState& rtp_state) = 0; - virtual void SetRtxState(const RtpState& rtp_state) = 0; virtual RtpState GetRtpState() const = 0; virtual RtpState GetRtxState() const = 0; @@ -361,7 +364,7 @@ // Access to packet state (e.g. sequence numbering) must only be access by // one thread at a time. It may be only one thread, or a construction thread - // that calls SetRtpState() - handing over to a pacer thread that calls + // that sets the initial state - handing over to a pacer thread that calls // TrySendPacket() - and at teardown ownership is handed to a destruciton // thread that calls GetRtpState(). // This method is used to signal that "ownership" of the rtp state is being
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index d6bb3b5..d5d94fe 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -190,11 +190,16 @@ // RTP variables // This random initialization is not intended to be cryptographically // strong. - timestamp_offset_(Random(clock_->TimeInMicroseconds()).Rand<uint32_t>()), + timestamp_offset_( + config.rtp_state.has_value() + ? config.rtp_state->start_timestamp + : Random(clock_->TimeInMicroseconds()).Rand<uint32_t>()), rid_(config.rid), always_send_mid_and_rid_(config.always_send_mid_and_rid), - ssrc_has_acked_(false), - rtx_ssrc_has_acked_(false), + ssrc_has_acked_(config.rtp_state.has_value() && + config.rtp_state->ssrc_has_acked), + rtx_ssrc_has_acked_(config.rtx_rtp_state.has_value() && + config.rtx_rtp_state->ssrc_has_acked), rtx_(kRtxOff), supports_bwe_extension_(false), retransmission_rate_limiter_(config.retransmission_rate_limiter) { @@ -714,11 +719,6 @@ return state; } -void RTPSender::SetRtxRtpState(const RtpState& rtp_state) { - MutexLock lock(&send_mutex_); - rtx_ssrc_has_acked_ = rtp_state.ssrc_has_acked; -} - RtpState RTPSender::GetRtxRtpState() const { MutexLock lock(&send_mutex_);
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 166b266..82409fe 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -78,6 +78,7 @@ bool IsAudioConfigured() const { return audio_configured_; } uint32_t TimestampOffset() const RTC_LOCKS_EXCLUDED(send_mutex_); + [[deprecated("Set RtpRtcpInterface::Configuration::rtp_state instead.")]] void SetTimestampOffset(uint32_t timestamp) RTC_LOCKS_EXCLUDED(send_mutex_); void SetMid(absl::string_view mid) RTC_LOCKS_EXCLUDED(send_mutex_); @@ -159,10 +160,9 @@ void EnqueuePackets(std::vector<std::unique_ptr<RtpPacketToSend>> packets) RTC_LOCKS_EXCLUDED(send_mutex_); + [[deprecated("Set RtpRtcpInterface::Configuration::rtp_state instead.")]] void SetRtpState(const RtpState& rtp_state) RTC_LOCKS_EXCLUDED(send_mutex_); RtpState GetRtpState() const RTC_LOCKS_EXCLUDED(send_mutex_); - void SetRtxRtpState(const RtpState& rtp_state) - RTC_LOCKS_EXCLUDED(send_mutex_); RtpState GetRtxRtpState() const RTC_LOCKS_EXCLUDED(send_mutex_); private:
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index ab67860..6618144 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -142,6 +142,8 @@ // Configure rid unconditionally, it has effect only if // corresponding header extension is enabled. config.rid = std::string(kRid); + // Use a fixed start timestamp instead of a random one. + config.rtp_state = RtpState{.start_timestamp = 0}; return config; } @@ -154,7 +156,6 @@ rtp_sender_ = std::make_unique<RTPSender>( env_, config, packet_history_.get(), config.paced_sender); sequencer_->set_media_sequence_number(kSeqNum); - rtp_sender_->SetTimestampOffset(0); } GlobalSimulatedTimeController time_controller_; @@ -773,13 +774,14 @@ // Test that if the RtpState indicates an ACK has been received on that SSRC // then neither the MID nor RID header extensions will be sent. TEST_F(RtpSenderTest, MidAndRidNotIncludedOnSentPacketsAfterRtpStateRestored) { - EnableMidSending(kMid); - EnableRidSending(); - RtpState state = rtp_sender_->GetRtpState(); EXPECT_FALSE(state.ssrc_has_acked); state.ssrc_has_acked = true; - rtp_sender_->SetRtpState(state); + RtpRtcpInterface::Configuration config = GetDefaultConfig(); + config.rtp_state = state; + CreateSender(config); + EnableMidSending(kMid); + EnableRidSending(); EXPECT_CALL( mock_paced_sender_, @@ -793,14 +795,15 @@ // RTX SSRC then neither the MID nor RRID header extensions will be sent on // RTX packets. TEST_F(RtpSenderTest, MidAndRridNotIncludedOnRtxPacketsAfterRtpStateRestored) { - EnableRtx(); - EnableMidSending(kMid); - EnableRidSending(); - RtpState rtx_state = rtp_sender_->GetRtxRtpState(); EXPECT_FALSE(rtx_state.ssrc_has_acked); rtx_state.ssrc_has_acked = true; - rtp_sender_->SetRtxRtpState(rtx_state); + RtpRtcpInterface::Configuration config = GetDefaultConfig(); + config.rtx_rtp_state = rtx_state; + CreateSender(config); + EnableRtx(); + EnableMidSending(kMid); + EnableRidSending(); EXPECT_CALL(mock_paced_sender_, EnqueuePackets(SizeIs(1))) .WillOnce([&](std::vector<std::unique_ptr<RtpPacketToSend>> packets) { @@ -817,6 +820,21 @@ ASSERT_LT(0, rtp_sender_->ReSendPacket(built_packet->SequenceNumber())); } +TEST_F(RtpSenderTest, UsesInitialRtpStatesFromConfig) { + constexpr uint32_t kStartTimestamp = 0x1234'5678; + RtpRtcpInterface::Configuration config = GetDefaultConfig(); + config.rtp_state = + RtpState{.start_timestamp = kStartTimestamp, .ssrc_has_acked = true}; + config.rtx_rtp_state = RtpState{.ssrc_has_acked = true}; + CreateSender(config); + + EXPECT_EQ(rtp_sender_->TimestampOffset(), kStartTimestamp); + RtpState state = rtp_sender_->GetRtpState(); + EXPECT_EQ(state.start_timestamp, kStartTimestamp); + EXPECT_TRUE(state.ssrc_has_acked); + EXPECT_TRUE(rtp_sender_->GetRtxRtpState().ssrc_has_acked); +} + TEST_F(RtpSenderTest, RespectsNackBitrateLimit) { const int32_t kPacketSize = 1400; const int32_t kNumPackets = 30;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 0e4612c..2ad2770 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -211,6 +211,7 @@ .retransmission_rate_limiter = &retransmission_rate_limiter_, .local_media_ssrc = kSsrc, .rtx_send_ssrc = kRtxSsrc, + .rtp_state = RtpState{.start_timestamp = 0}, .rid = "rid"})), rtp_sender_video_( std::make_unique<TestRtpSenderVideo>(&fake_clock_, @@ -218,7 +219,6 @@ env_.field_trials(), raw_packetization)) { rtp_module_->SetSequenceNumber(kSeqNum); - rtp_module_->SetStartTimestamp(0); } void UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed( @@ -1878,9 +1878,9 @@ {.outgoing_transport = &transport_, .retransmission_rate_limiter = &retransmission_rate_limiter_, .local_media_ssrc = kSsrc, + .rtp_state = RtpState{.start_timestamp = 0}, .rid = "myrid"})) { rtp_module_->SetSequenceNumber(kSeqNum); - rtp_module_->SetStartTimestamp(0); } std::unique_ptr<RTPSenderVideo> CreateSenderWithFrameTransformer(