Correctly handle retransmissions/padding in early loss detection. This CL makes sure we don't cull packets from the history based on incorrect ack mapping, just like it's predecessor: https://webrtc-review.googlesource.com/c/src/+/218000 It also changes the logic to make sure retransmits counts towards history pruning - and properly ignores padding/fec. Bug: webrtc:12713 Change-Id: I7835d10e483687e960a9cce41d4e2f1a6c3189b4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221863 Reviewed-by: Björn Terelius <terelius@webrtc.org> Reviewed-by: Philip Eliasson <philipel@webrtc.org> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Commit-Queue: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/heads/master@{#34293}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 110e28b..e20ba32 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc
@@ -932,43 +932,45 @@ // Map from SSRC to all acked packets for that RTP module. std::map<uint32_t, std::vector<uint16_t>> acked_packets_per_ssrc; for (const StreamPacketInfo& packet : packet_feedback_vector) { - if (packet.received) { - acked_packets_per_ssrc[packet.ssrc].push_back(packet.rtp_sequence_number); + if (packet.received && packet.ssrc) { + acked_packets_per_ssrc[*packet.ssrc].push_back( + packet.rtp_sequence_number); } } - // Map from SSRC to vector of RTP sequence numbers that are indicated as - // lost by feedback, without being trailed by any received packets. - std::map<uint32_t, std::vector<uint16_t>> early_loss_detected_per_ssrc; + // Map from SSRC to vector of RTP sequence numbers that are indicated as + // lost by feedback, without being trailed by any received packets. + std::map<uint32_t, std::vector<uint16_t>> early_loss_detected_per_ssrc; - for (const StreamPacketInfo& packet : packet_feedback_vector) { - if (!packet.received) { - // Last known lost packet, might not be detectable as lost by remote - // jitter buffer. - early_loss_detected_per_ssrc[packet.ssrc].push_back( - packet.rtp_sequence_number); - } else { - // Packet received, so any loss prior to this is already detectable. - early_loss_detected_per_ssrc.erase(packet.ssrc); - } + for (const StreamPacketInfo& packet : packet_feedback_vector) { + // Only include new media packets, not retransmissions/padding/fec. + if (!packet.received && packet.ssrc && !packet.is_retransmission) { + // Last known lost packet, might not be detectable as lost by remote + // jitter buffer. + early_loss_detected_per_ssrc[*packet.ssrc].push_back( + packet.rtp_sequence_number); + } else { + // Packet received, so any loss prior to this is already detectable. + early_loss_detected_per_ssrc.erase(*packet.ssrc); } + } - for (const auto& kv : early_loss_detected_per_ssrc) { - const uint32_t ssrc = kv.first; - auto it = ssrc_to_rtp_module_.find(ssrc); - RTC_DCHECK(it != ssrc_to_rtp_module_.end()); - RTPSender* rtp_sender = it->second->RtpSender(); - for (uint16_t sequence_number : kv.second) { - rtp_sender->ReSendPacket(sequence_number); - } + for (const auto& kv : early_loss_detected_per_ssrc) { + const uint32_t ssrc = kv.first; + auto it = ssrc_to_rtp_module_.find(ssrc); + RTC_CHECK(it != ssrc_to_rtp_module_.end()); + RTPSender* rtp_sender = it->second->RtpSender(); + for (uint16_t sequence_number : kv.second) { + rtp_sender->ReSendPacket(sequence_number); } + } for (const auto& kv : acked_packets_per_ssrc) { const uint32_t ssrc = kv.first; auto it = ssrc_to_rtp_module_.find(ssrc); if (it == ssrc_to_rtp_module_.end()) { - // Packets not for a media SSRC, so likely RTX or FEC. If so, ignore - // since there's no RTP history to clean up anyway. + // No media, likely FEC or padding. Ignore since there's no RTP history to + // clean up anyway. continue; } rtc::ArrayView<const uint16_t> rtp_sequence_numbers(kv.second);
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index 85934cc..334d97c 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc
@@ -462,11 +462,13 @@ lost_packet_feedback.rtp_sequence_number = rtp_sequence_numbers[0]; lost_packet_feedback.ssrc = kSsrc1; lost_packet_feedback.received = false; + lost_packet_feedback.is_retransmission = false; StreamFeedbackObserver::StreamPacketInfo received_packet_feedback; received_packet_feedback.rtp_sequence_number = rtp_sequence_numbers[1]; received_packet_feedback.ssrc = kSsrc1; received_packet_feedback.received = true; + lost_packet_feedback.is_retransmission = false; test.router()->OnPacketFeedbackVector( {lost_packet_feedback, received_packet_feedback}); @@ -638,11 +640,13 @@ first_packet_feedback.rtp_sequence_number = frame1_rtp_sequence_number; first_packet_feedback.ssrc = kSsrc1; first_packet_feedback.received = false; + first_packet_feedback.is_retransmission = false; StreamFeedbackObserver::StreamPacketInfo second_packet_feedback; second_packet_feedback.rtp_sequence_number = frame2_rtp_sequence_number; second_packet_feedback.ssrc = kSsrc2; second_packet_feedback.received = true; + first_packet_feedback.is_retransmission = false; test.router()->OnPacketFeedbackVector( {first_packet_feedback, second_packet_feedback});
diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc index 138cdb6..933abd9 100644 --- a/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc +++ b/modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc
@@ -27,9 +27,9 @@ using ::testing::Invoke; namespace webrtc { -namespace webrtc_cc { namespace { +constexpr uint32_t kSsrc = 8492; const PacedPacketInfo kPacingInfo0(0, 5, 2000); const PacedPacketInfo kPacingInfo1(1, 8, 4000); const PacedPacketInfo kPacingInfo2(2, 14, 7000); @@ -77,10 +77,6 @@ return res; } -} // namespace - -namespace test { - class MockStreamFeedbackObserver : public webrtc::StreamFeedbackObserver { public: MOCK_METHOD(void, @@ -89,6 +85,8 @@ (override)); }; +} // namespace + class TransportFeedbackAdapterTest : public ::testing::Test { public: TransportFeedbackAdapterTest() : clock_(0) {} @@ -108,7 +106,7 @@ void OnSentPacket(const PacketResult& packet_feedback) { RtpPacketSendInfo packet_info; - packet_info.ssrc = kSsrc; + packet_info.media_ssrc = kSsrc; packet_info.transport_sequence_number = packet_feedback.sent_packet.sequence_number; packet_info.rtp_sequence_number = 0; @@ -122,8 +120,6 @@ packet_feedback.sent_packet.send_time.ms(), rtc::PacketInfo())); } - static constexpr uint32_t kSsrc = 8492; - SimulatedClock clock_; std::unique_ptr<TransportFeedbackAdapter> adapter_; }; @@ -393,7 +389,7 @@ // Add a packet and then mark it as sent. RtpPacketSendInfo packet_info; - packet_info.ssrc = kSsrc; + packet_info.media_ssrc = kSsrc; packet_info.transport_sequence_number = packet.sent_packet.sequence_number; packet_info.length = packet.sent_packet.size.bytes(); packet_info.pacing_info = packet.sent_packet.pacing_info; @@ -412,6 +408,4 @@ EXPECT_FALSE(duplicate_packet.has_value()); } -} // namespace test -} // namespace webrtc_cc } // namespace webrtc
diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc index c958a1c..6ab3ad8 100644 --- a/modules/congestion_controller/rtp/transport_feedback_demuxer.cc +++ b/modules/congestion_controller/rtp/transport_feedback_demuxer.cc
@@ -38,15 +38,16 @@ void TransportFeedbackDemuxer::AddPacket(const RtpPacketSendInfo& packet_info) { MutexLock lock(&lock_); - if (packet_info.ssrc != 0) { - StreamFeedbackObserver::StreamPacketInfo info; - info.ssrc = packet_info.ssrc; - info.rtp_sequence_number = packet_info.rtp_sequence_number; - info.received = false; - history_.insert( - {seq_num_unwrapper_.Unwrap(packet_info.transport_sequence_number), - info}); - } + + StreamFeedbackObserver::StreamPacketInfo info; + info.ssrc = packet_info.media_ssrc; + info.rtp_sequence_number = packet_info.rtp_sequence_number; + info.received = false; + info.is_retransmission = + packet_info.packet_type == RtpPacketMediaType::kRetransmission; + history_.insert( + {seq_num_unwrapper_.Unwrap(packet_info.transport_sequence_number), info}); + while (history_.size() > kMaxPacketsInHistory) { history_.erase(history_.begin()); }
diff --git a/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc b/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc index 6514a4e..482f58d 100644 --- a/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc +++ b/modules/congestion_controller/rtp/transport_feedback_demuxer_unittest.cc
@@ -16,7 +16,11 @@ namespace webrtc { namespace { -using ::testing::_; +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Field; +using PacketInfo = StreamFeedbackObserver::StreamPacketInfo; + static constexpr uint32_t kSsrc = 8492; class MockStreamFeedbackObserver : public webrtc::StreamFeedbackObserver { @@ -28,41 +32,65 @@ }; RtpPacketSendInfo CreatePacket(uint32_t ssrc, - int16_t rtp_sequence_number, - int64_t transport_sequence_number) { + uint16_t rtp_sequence_number, + int64_t transport_sequence_number, + bool is_retransmission) { RtpPacketSendInfo res; - res.ssrc = ssrc; + res.media_ssrc = ssrc; res.transport_sequence_number = transport_sequence_number; res.rtp_sequence_number = rtp_sequence_number; + res.packet_type = is_retransmission ? RtpPacketMediaType::kRetransmission + : RtpPacketMediaType::kVideo; return res; } } // namespace + TEST(TransportFeedbackDemuxerTest, ObserverSanity) { TransportFeedbackDemuxer demuxer; MockStreamFeedbackObserver mock; demuxer.RegisterStreamFeedbackObserver({kSsrc}, &mock); - demuxer.AddPacket(CreatePacket(kSsrc, 55, 1)); - demuxer.AddPacket(CreatePacket(kSsrc, 56, 2)); - demuxer.AddPacket(CreatePacket(kSsrc, 57, 3)); + const uint16_t kRtpStartSeq = 55; + const int64_t kTransportStartSeq = 1; + demuxer.AddPacket(CreatePacket(kSsrc, kRtpStartSeq, kTransportStartSeq, + /*is_retransmit=*/false)); + demuxer.AddPacket(CreatePacket(kSsrc, kRtpStartSeq + 1, + kTransportStartSeq + 1, + /*is_retransmit=*/false)); + demuxer.AddPacket(CreatePacket( + kSsrc, kRtpStartSeq + 2, kTransportStartSeq + 2, /*is_retransmit=*/true)); rtcp::TransportFeedback feedback; - feedback.SetBase(1, 1000); - ASSERT_TRUE(feedback.AddReceivedPacket(1, 1000)); - ASSERT_TRUE(feedback.AddReceivedPacket(2, 2000)); - ASSERT_TRUE(feedback.AddReceivedPacket(3, 3000)); + feedback.SetBase(kTransportStartSeq, 1000); + ASSERT_TRUE(feedback.AddReceivedPacket(kTransportStartSeq, 1000)); + // Drop middle packet. + ASSERT_TRUE(feedback.AddReceivedPacket(kTransportStartSeq + 2, 3000)); - EXPECT_CALL(mock, OnPacketFeedbackVector(_)).Times(1); + EXPECT_CALL( + mock, OnPacketFeedbackVector(ElementsAre( + AllOf(Field(&PacketInfo::received, true), + Field(&PacketInfo::ssrc, kSsrc), + Field(&PacketInfo::rtp_sequence_number, kRtpStartSeq), + Field(&PacketInfo::is_retransmission, false)), + AllOf(Field(&PacketInfo::received, false), + Field(&PacketInfo::ssrc, kSsrc), + Field(&PacketInfo::rtp_sequence_number, kRtpStartSeq + 1), + Field(&PacketInfo::is_retransmission, false)), + AllOf(Field(&PacketInfo::received, true), + Field(&PacketInfo::ssrc, kSsrc), + Field(&PacketInfo::rtp_sequence_number, kRtpStartSeq + 2), + Field(&PacketInfo::is_retransmission, true))))); demuxer.OnTransportFeedback(feedback); demuxer.DeRegisterStreamFeedbackObserver(&mock); - demuxer.AddPacket(CreatePacket(kSsrc, 58, 4)); + demuxer.AddPacket( + CreatePacket(kSsrc, kRtpStartSeq + 3, kTransportStartSeq + 3, false)); rtcp::TransportFeedback second_feedback; - second_feedback.SetBase(4, 4000); - ASSERT_TRUE(second_feedback.AddReceivedPacket(4, 4000)); + second_feedback.SetBase(kTransportStartSeq + 3, 4000); + ASSERT_TRUE(second_feedback.AddReceivedPacket(kTransportStartSeq + 3, 4000)); - EXPECT_CALL(mock, OnPacketFeedbackVector(_)).Times(0); + EXPECT_CALL(mock, OnPacketFeedbackVector).Times(0); demuxer.OnTransportFeedback(second_feedback); } } // namespace webrtc
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 59c0f29..ca12adf 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -228,8 +228,10 @@ RtpPacketSendInfo() = default; uint16_t transport_sequence_number = 0; + // TODO(bugs.webrtc.org/12713): Remove once downstream usage is gone. uint32_t ssrc = 0; - uint16_t rtp_sequence_number = 0; + absl::optional<uint32_t> media_ssrc; + uint16_t rtp_sequence_number = 0; // Only valid if |ssrc| is set. uint32_t rtp_timestamp = 0; size_t length = 0; absl::optional<RtpPacketMediaType> packet_type; @@ -267,9 +269,13 @@ class StreamFeedbackObserver { public: struct StreamPacketInfo { - uint32_t ssrc; - uint16_t rtp_sequence_number; bool received; + + // |rtp_sequence_number| and |is_retransmission| are only valid if |ssrc| + // is populated. + absl::optional<uint32_t> ssrc; + uint16_t rtp_sequence_number; + bool is_retransmission; }; virtual ~StreamFeedbackObserver() = default;
diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc index 3f7d22c..33a1b8e 100644 --- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
@@ -313,7 +313,7 @@ } RtpPacketSendInfo packet_info; - packet_info.ssrc = ssrc_; + packet_info.media_ssrc = ssrc_; packet_info.transport_sequence_number = packet_id; packet_info.rtp_sequence_number = packet.SequenceNumber(); packet_info.length = packet_size;
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 55dd9ff..126b89c 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc
@@ -142,6 +142,9 @@ RTC_DCHECK(packet->packet_type().has_value()); RTC_DCHECK(HasCorrectSsrc(*packet)); + if (packet->packet_type() == RtpPacketMediaType::kRetransmission) { + RTC_DCHECK(packet->retransmitted_sequence_number().has_value()); + } const uint32_t packet_ssrc = packet->Ssrc(); const int64_t now_ms = clock_->TimeInMilliseconds(); @@ -409,13 +412,34 @@ } RtpPacketSendInfo packet_info; - packet_info.ssrc = ssrc_; packet_info.transport_sequence_number = packet_id; - packet_info.rtp_sequence_number = packet.SequenceNumber(); packet_info.rtp_timestamp = packet.Timestamp(); packet_info.length = packet_size; packet_info.pacing_info = pacing_info; packet_info.packet_type = packet.packet_type(); + + switch (*packet_info.packet_type) { + case RtpPacketMediaType::kAudio: + case RtpPacketMediaType::kVideo: + packet_info.media_ssrc = ssrc_; + packet_info.rtp_sequence_number = packet.SequenceNumber(); + break; + case RtpPacketMediaType::kRetransmission: + // For retransmissions, we're want to remove the original media packet + // if the rentrasmit arrives - so populate that in the packet info. + packet_info.media_ssrc = ssrc_; + packet_info.rtp_sequence_number = + *packet.retransmitted_sequence_number(); + break; + case RtpPacketMediaType::kPadding: + case RtpPacketMediaType::kForwardErrorCorrection: + // We're not interested in feedback about these packets being received + // or lost. + break; + } + // TODO(bugs.webrtc.org/12713): Remove once downstream usage is gone. + packet_info.ssrc = packet_info.media_ssrc.value_or(0); + transport_feedback_observer_->OnAddPacket(packet_info); } }
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index 663638f..4f3990c 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
@@ -221,7 +221,7 @@ EXPECT_CALL( feedback_observer_, OnAddPacket(AllOf( - Field(&RtpPacketSendInfo::ssrc, kSsrc), + Field(&RtpPacketSendInfo::media_ssrc, kSsrc), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber), Field(&RtpPacketSendInfo::rtp_sequence_number, kStartSequenceNumber), @@ -246,6 +246,8 @@ std::unique_ptr<RtpPacketToSend> retransmission = BuildRtpPacket(); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); + retransmission->set_retransmitted_sequence_number( + media_packet->SequenceNumber()); sender->SendPacket(retransmission.get(), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } @@ -407,6 +409,7 @@ std::unique_ptr<RtpPacketToSend> packet = BuildRtpPacket(); packet->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber); packet->set_packet_type(RtpPacketMediaType::kRetransmission); + packet->set_retransmitted_sequence_number(packet->SequenceNumber()); sender->SendPacket(packet.get(), PacedPacketInfo()); } @@ -465,6 +468,7 @@ // Mark all packets as retransmissions - will cause total and retransmission // rates to be equal. packet->set_packet_type(RtpPacketMediaType::kRetransmission); + packet->set_retransmitted_sequence_number(packet->SequenceNumber()); total_data_sent += DataSize::Bytes(packet->size()); EXPECT_CALL(observer, Notify(_, _, kSsrc)) @@ -520,6 +524,8 @@ std::unique_ptr<RtpPacketToSend> retransmission = BuildRtpPacket(); retransmission->set_allow_retransmission(true); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); + retransmission->set_retransmitted_sequence_number( + retransmission->SequenceNumber()); sender->SendPacket(retransmission.get(), PacedPacketInfo()); EXPECT_FALSE(packet_history_.GetPacketState(retransmission->SequenceNumber()) .has_value()); @@ -600,6 +606,8 @@ // and retransmitted packet statistics. std::unique_ptr<RtpPacketToSend> retransmission_packet = BuildRtpPacket(); retransmission_packet->set_packet_type(RtpPacketMediaType::kRetransmission); + retransmission_packet->set_retransmitted_sequence_number( + retransmission_packet->SequenceNumber()); media_packet->SetPayloadSize(7); expected_transmitted_counter.packets += 1; expected_transmitted_counter.payload_bytes += @@ -710,6 +718,7 @@ rtx_packet->set_packet_type(RtpPacketMediaType::kRetransmission); rtx_packet->SetSsrc(kRtxSsrc); rtx_packet->SetPayloadSize(7); + rtx_packet->set_retransmitted_sequence_number(media_packet->SequenceNumber()); sender->SendPacket(rtx_packet.get(), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); @@ -785,6 +794,7 @@ std::unique_ptr<RtpPacketToSend> retransmission = BuildRtpPacket(); retransmission->SetExtension<TransportSequenceNumber>(kPacketId + 1); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); + retransmission->set_retransmitted_sequence_number(packet->SequenceNumber()); sender->SendPacket(retransmission.get(), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } @@ -815,6 +825,7 @@ std::unique_ptr<RtpPacketToSend> rtx_packet = BuildRtpPacket(); rtx_packet->SetSsrc(kRtxSsrc); rtx_packet->set_packet_type(RtpPacketMediaType::kRetransmission); + rtx_packet->set_retransmitted_sequence_number(video_packet->SequenceNumber()); rtx_packet->SetPayloadSize(kPayloadSize); rtx_packet->SetExtension<TransportSequenceNumber>(2); @@ -854,6 +865,115 @@ EXPECT_EQ(rtx_stats.retransmitted.packets, 1u); } +TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) { + const uint16_t kTransportSequenceNumber = 17; + header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, + TransportSequenceNumber::kUri); + std::unique_ptr<RtpPacketToSend> retransmission = BuildRtpPacket(); + retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); + retransmission->SetExtension<TransportSequenceNumber>( + kTransportSequenceNumber); + uint16_t retransmitted_seq = retransmission->SequenceNumber() - 2; + retransmission->set_retransmitted_sequence_number(retransmitted_seq); + + std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress(); + EXPECT_CALL( + feedback_observer_, + OnAddPacket(AllOf( + Field(&RtpPacketSendInfo::media_ssrc, kSsrc), + Field(&RtpPacketSendInfo::rtp_sequence_number, retransmitted_seq), + Field(&RtpPacketSendInfo::transport_sequence_number, + kTransportSequenceNumber)))); + sender->SendPacket(retransmission.get(), PacedPacketInfo()); +} + +TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { + const uint16_t kTransportSequenceNumber = 17; + header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, + TransportSequenceNumber::kUri); + + std::unique_ptr<RtpPacketToSend> rtx_retransmission = BuildRtpPacket(); + rtx_retransmission->SetSsrc(kRtxSsrc); + rtx_retransmission->SetExtension<TransportSequenceNumber>( + kTransportSequenceNumber); + rtx_retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); + uint16_t rtx_retransmitted_seq = rtx_retransmission->SequenceNumber() - 2; + rtx_retransmission->set_retransmitted_sequence_number(rtx_retransmitted_seq); + + std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress(); + EXPECT_CALL( + feedback_observer_, + OnAddPacket(AllOf( + Field(&RtpPacketSendInfo::media_ssrc, kSsrc), + Field(&RtpPacketSendInfo::rtp_sequence_number, rtx_retransmitted_seq), + Field(&RtpPacketSendInfo::transport_sequence_number, + kTransportSequenceNumber)))); + sender->SendPacket(rtx_retransmission.get(), PacedPacketInfo()); +} + +TEST_P(RtpSenderEgressTest, TransportFeedbackObserverPadding) { + const uint16_t kTransportSequenceNumber = 17; + header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, + TransportSequenceNumber::kUri); + std::unique_ptr<RtpPacketToSend> padding = BuildRtpPacket(); + padding->SetPadding(224); + padding->set_packet_type(RtpPacketMediaType::kPadding); + padding->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber); + + std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress(); + EXPECT_CALL( + feedback_observer_, + OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), + Field(&RtpPacketSendInfo::transport_sequence_number, + kTransportSequenceNumber)))); + sender->SendPacket(padding.get(), PacedPacketInfo()); +} + +TEST_P(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { + const uint16_t kTransportSequenceNumber = 17; + header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, + TransportSequenceNumber::kUri); + + std::unique_ptr<RtpPacketToSend> rtx_padding = BuildRtpPacket(); + rtx_padding->SetPadding(224); + rtx_padding->SetSsrc(kRtxSsrc); + rtx_padding->set_packet_type(RtpPacketMediaType::kPadding); + rtx_padding->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber); + + std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress(); + EXPECT_CALL( + feedback_observer_, + OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), + Field(&RtpPacketSendInfo::transport_sequence_number, + kTransportSequenceNumber)))); + sender->SendPacket(rtx_padding.get(), PacedPacketInfo()); +} + +TEST_P(RtpSenderEgressTest, TransportFeedbackObserverFec) { + const uint16_t kTransportSequenceNumber = 17; + header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, + TransportSequenceNumber::kUri); + + std::unique_ptr<RtpPacketToSend> fec_packet = BuildRtpPacket(); + fec_packet->SetSsrc(kFlexFecSsrc); + fec_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection); + fec_packet->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber); + + const rtc::ArrayView<const RtpExtensionSize> kNoRtpHeaderExtensionSizes; + FlexfecSender flexfec(kFlexfectPayloadType, kFlexFecSsrc, kSsrc, /*mid=*/"", + /*header_extensions=*/{}, kNoRtpHeaderExtensionSizes, + /*rtp_state=*/nullptr, time_controller_.GetClock()); + RtpRtcpInterface::Configuration config = DefaultConfig(); + config.fec_generator = &flexfec; + auto sender = std::make_unique<RtpSenderEgress>(config, &packet_history_); + EXPECT_CALL( + feedback_observer_, + OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), + Field(&RtpPacketSendInfo::transport_sequence_number, + kTransportSequenceNumber)))); + sender->SendPacket(fec_packet.get(), PacedPacketInfo()); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderEgressTest, ::testing::Values(TestConfig(false),
diff --git a/rtc_tools/rtc_event_log_visualizer/analyzer.cc b/rtc_tools/rtc_event_log_visualizer/analyzer.cc index 7690d7d..0f727f2 100644 --- a/rtc_tools/rtc_event_log_visualizer/analyzer.cc +++ b/rtc_tools/rtc_event_log_visualizer/analyzer.cc
@@ -1267,7 +1267,7 @@ const RtpPacketType& rtp_packet = *rtp_iterator->second; if (rtp_packet.rtp.header.extension.hasTransportSequenceNumber) { RtpPacketSendInfo packet_info; - packet_info.ssrc = rtp_packet.rtp.header.ssrc; + packet_info.media_ssrc = rtp_packet.rtp.header.ssrc; packet_info.transport_sequence_number = rtp_packet.rtp.header.extension.transportSequenceNumber; packet_info.rtp_sequence_number = rtp_packet.rtp.header.sequenceNumber;
diff --git a/rtc_tools/rtc_event_log_visualizer/log_simulation.cc b/rtc_tools/rtc_event_log_visualizer/log_simulation.cc index 4dbaf0b..c0b418d 100644 --- a/rtc_tools/rtc_event_log_visualizer/log_simulation.cc +++ b/rtc_tools/rtc_event_log_visualizer/log_simulation.cc
@@ -84,7 +84,7 @@ } RtpPacketSendInfo packet_info; - packet_info.ssrc = packet.ssrc; + packet_info.media_ssrc = packet.ssrc; packet_info.transport_sequence_number = packet.transport_seq_no; packet_info.rtp_sequence_number = packet.stream_seq_no; packet_info.length = packet.size;