Propagate OnSendPacket even if transport sequence number is not registered

To allow to calculate send delay with that callback

Bug: None
Change-Id: I0fe1ffd42b62c99bd01670e583584511c34277db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319563
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40731}
This commit is contained in:
Danil Chapovalov 2023-09-11 11:34:57 +02:00 committed by WebRTC LUCI CQ
parent 030c6ff43f
commit 378fb28621
6 changed files with 35 additions and 15 deletions

View File

@ -422,7 +422,7 @@ class SendSideDelayObserver {
class SendPacketObserver {
public:
virtual ~SendPacketObserver() = default;
virtual void OnSendPacket(uint16_t packet_id,
virtual void OnSendPacket(absl::optional<uint16_t> packet_id,
Timestamp capture_time,
uint32_t ssrc) = 0;
};

View File

@ -196,10 +196,12 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver,
counter_map_[ssrc] = packet_counter;
}
void OnSendPacket(uint16_t packet_id,
void OnSendPacket(absl::optional<uint16_t> packet_id,
Timestamp capture_time,
uint32_t ssrc) override {
last_sent_packet_.emplace(packet_id, capture_time, ssrc);
if (packet_id.has_value()) {
last_sent_packet_.emplace(*packet_id, capture_time, ssrc);
}
}
absl::optional<SentPacket> last_sent_packet() const {

View File

@ -252,7 +252,9 @@ void RtpSenderEgress::CompleteSendPacket(const Packet& compound_packet,
// Downstream code actually uses this flag to distinguish between media and
// everything else.
options.is_retransmit = !is_media;
if (auto packet_id = packet->GetExtension<TransportSequenceNumber>()) {
absl::optional<uint16_t> packet_id =
packet->GetExtension<TransportSequenceNumber>();
if (packet_id.has_value()) {
options.packet_id = *packet_id;
options.included_in_feedback = true;
options.included_in_allocation = true;
@ -265,7 +267,7 @@ void RtpSenderEgress::CompleteSendPacket(const Packet& compound_packet,
if (packet->packet_type() != RtpPacketMediaType::kPadding &&
packet->packet_type() != RtpPacketMediaType::kRetransmission) {
UpdateDelayStatistics(packet->capture_time(), now, packet_ssrc);
UpdateOnSendPacket(options.packet_id, packet->capture_time(), packet_ssrc);
UpdateOnSendPacket(packet_id, packet->capture_time(), packet_ssrc);
}
options.batchable = enable_send_packet_batching_ && !is_audio_;
options.last_packet_in_batch = last_in_batch;
@ -506,10 +508,10 @@ void RtpSenderEgress::RecomputeMaxSendDelay() {
}
}
void RtpSenderEgress::UpdateOnSendPacket(int packet_id,
void RtpSenderEgress::UpdateOnSendPacket(absl::optional<uint16_t> packet_id,
Timestamp capture_time,
uint32_t ssrc) {
if (!send_packet_observer_ || capture_time.IsInfinite() || packet_id == -1) {
if (!send_packet_observer_ || capture_time.IsInfinite()) {
return;
}

View File

@ -117,7 +117,9 @@ class RtpSenderEgress {
Timestamp now,
uint32_t ssrc);
void RecomputeMaxSendDelay();
void UpdateOnSendPacket(int packet_id, Timestamp capture_time, uint32_t ssrc);
void UpdateOnSendPacket(absl::optional<uint16_t> packet_id,
Timestamp capture_time,
uint32_t ssrc);
// Sends packet on to `transport_`, leaving the RTP module.
bool SendPacketToNetwork(const RtpPacketToSend& packet,
const PacketOptions& options,

View File

@ -36,6 +36,7 @@ namespace {
using ::testing::_;
using ::testing::AllOf;
using ::testing::Eq;
using ::testing::Field;
using ::testing::InSequence;
using ::testing::NiceMock;
@ -57,7 +58,10 @@ enum : int {
class MockSendPacketObserver : public SendPacketObserver {
public:
MOCK_METHOD(void, OnSendPacket, (uint16_t, Timestamp, uint32_t), (override));
MOCK_METHOD(void,
OnSendPacket,
(absl::optional<uint16_t>, Timestamp, uint32_t),
(override));
};
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
@ -421,12 +425,20 @@ TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) {
const uint16_t kTransportSequenceNumber = 1;
EXPECT_CALL(
send_packet_observer_,
OnSendPacket(kTransportSequenceNumber, clock_->CurrentTime(), kSsrc));
OnSendPacket(Eq(kTransportSequenceNumber), clock_->CurrentTime(), kSsrc));
std::unique_ptr<RtpPacketToSend> packet = BuildRtpPacket();
packet->SetExtension<TransportSequenceNumber>(kTransportSequenceNumber);
sender->SendPacket(std::move(packet), PacedPacketInfo());
}
TEST_F(RtpSenderEgressTest, OnSendPacketUpdatedWithoutTransportSequenceNumber) {
std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
EXPECT_CALL(send_packet_observer_,
OnSendPacket(Eq(absl::nullopt), clock_->CurrentTime(), kSsrc));
sender->SendPacket(BuildRtpPacket(), PacedPacketInfo());
}
TEST_F(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) {
std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
@ -882,16 +894,16 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) {
EXPECT_CALL(send_side_delay_observer,
SendSideDelayUpdated(kDiffMs, kDiffMs, kFlexFecSsrc));
EXPECT_CALL(send_packet_observer_, OnSendPacket(1, capture_time, kSsrc));
EXPECT_CALL(send_packet_observer_, OnSendPacket(Eq(1), capture_time, kSsrc));
sender->SendPacket(std::move(video_packet), PacedPacketInfo());
// Send packet observer not called for padding/retransmissions.
EXPECT_CALL(send_packet_observer_, OnSendPacket(2, _, _)).Times(0);
EXPECT_CALL(send_packet_observer_, OnSendPacket(Eq(2), _, _)).Times(0);
sender->SendPacket(std::move(rtx_packet), PacedPacketInfo());
EXPECT_CALL(send_packet_observer_,
OnSendPacket(3, capture_time, kFlexFecSsrc));
OnSendPacket(Eq(3), capture_time, kFlexFecSsrc));
sender->SendPacket(std::move(fec_packet), PacedPacketInfo());
time_controller_.AdvanceTime(TimeDelta::Zero());

View File

@ -105,11 +105,13 @@ class VideoSendStream : public webrtc::VideoSendStream {
SendDelayStats* send_delay_stats)
: stats_proxy_(*stats_proxy), send_delay_stats_(*send_delay_stats) {}
void OnSendPacket(uint16_t packet_id,
void OnSendPacket(absl::optional<uint16_t> packet_id,
Timestamp capture_time,
uint32_t ssrc) override {
stats_proxy_.OnSendPacket(ssrc, capture_time);
send_delay_stats_.OnSendPacket(packet_id, capture_time, ssrc);
if (packet_id.has_value()) {
send_delay_stats_.OnSendPacket(*packet_id, capture_time, ssrc);
}
}
private: