admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add RTP sequence number to TransportFeedbackObserver::AddPacket()

Browse Source 
With this change, both the normal RTP and the transport-wide sequence
numbers are propagated with with AddPacket() call via a new
RtpPacketSendInfo struct, replacing the previous set of parameters.

The intent with this is that SendTimeHistory can hold a mapping from
transport-wide to rtp sequence numbers, and then via callbacks let the
RTP modules know when packets have been received by the remote end.

Bug: webrtc:8975
Change-Id: I6a24fc6282cbb041393752d39593c2867b242192
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133021
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27708}
This commit is contained in:
Erik Språng 2019-04-23 12:00:11 +02:00 committed by Commit Bot
parent d0e0ed82af
commit 30a276b5d7
17 changed files with 166 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
audio/channel_send.cc
View File

@ -314,14 +314,11 @@ class TransportFeedbackProxy : public TransportFeedbackObserver {
}
// Implements TransportFeedbackObserver.
void AddPacket(uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override {
void OnAddPacket(const RtpPacketSendInfo& packet_info) override {
RTC_DCHECK(pacer_thread_.IsCurrent());
rtc::CritScope lock(&crit_);
if (feedback_observer_)
feedback_observer_->AddPacket(ssrc, sequence_number, length, pacing_info);
feedback_observer_->OnAddPacket(packet_info);
}
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {

13
call/rtp_transport_controller_send.cc
View File

@ -408,15 +408,12 @@ void RtpTransportControllerSend::OnReceivedRtcpReceiverReport(
});
}
void RtpTransportControllerSend::AddPacket(uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) {
if (send_side_bwe_with_overhead_) {
length += transport_overhead_bytes_per_packet_;
}
void RtpTransportControllerSend::OnAddPacket(
const RtpPacketSendInfo& packet_info) {
transport_feedback_adapter_.AddPacket(
ssrc, sequence_number, length, pacing_info,
packet_info,
send_side_bwe_with_overhead_ ? transport_overhead_bytes_per_packet_.load()
: 0,
Timestamp::ms(clock_->TimeInMilliseconds()));
}

5
call/rtp_transport_controller_send.h
View File

@ -110,10 +110,7 @@ class RtpTransportControllerSend final
int64_t now_ms) override;
// Implements TransportFeedbackObserver interface
void AddPacket(uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override;
void OnAddPacket(const RtpPacketSendInfo& packet_info) override;
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;
private:

15
logging/rtc_event_log/rtc_event_log_parser.cc
View File

@ -1961,14 +1961,23 @@ std::vector<LoggedPacketInfo> ParsedRtcEventLog::GetPacketInfos(
logged.overhead = current_overhead;
if (rtp.header.extension.hasTransportSequenceNumber) {
logged.log_feedback_time = Timestamp::PlusInfinity();
RtpPacketSendInfo packet_info;
packet_info.ssrc = rtp.header.ssrc;
packet_info.transport_sequence_number =
rtp.header.extension.transportSequenceNumber;
packet_info.rtp_sequence_number = rtp.header.sequenceNumber;
packet_info.has_rtp_sequence_number = true;
packet_info.length = rtp.total_length;
feedback_adapter.AddPacket(packet_info,
0u, // Should this be current_overhead?
Timestamp::ms(rtp.log_time_ms()));
rtc::SentPacket sent_packet;
sent_packet.send_time_ms = rtp.log_time_ms();
sent_packet.info.packet_size_bytes = rtp.total_length;
sent_packet.info.included_in_feedback = true;
sent_packet.packet_id = rtp.header.extension.transportSequenceNumber;
feedback_adapter.AddPacket(rtp.header.ssrc, sent_packet.packet_id,
rtp.total_length, PacedPacketInfo(),
Timestamp::ms(rtp.log_time_ms()));
auto sent_packet_msg = feedback_adapter.ProcessSentPacket(sent_packet);
RTC_CHECK(sent_packet_msg);
indices[sent_packet_msg->sequence_number] = packets.size();

7
modules/congestion_controller/include/send_side_congestion_controller.h
View File

@ -43,7 +43,7 @@ class RateLimiter;
class RtcEventLog;
class CongestionWindowPushbackController;
// Deprecated, for somewhat similar funtionality GoogCcNetworkController can be
// Deprecated, for somewhat similar functionality GoogCcNetworkController can be
// used via GoogCcNetworkControllerFactory.
class DEPRECATED_SendSideCongestionController
: public SendSideCongestionControllerInterface {
@ -107,10 +107,7 @@ class DEPRECATED_SendSideCongestionController
void Process() override;
// Implements TransportFeedbackObserver.
void AddPacket(uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override;
void OnAddPacket(const RtpPacketSendInfo& packet_info) override;
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;
std::vector<PacketFeedback> GetTransportFeedbackVector() const;

27
modules/congestion_controller/rtp/transport_feedback_adapter.cc
View File

@ -83,18 +83,35 @@ void TransportFeedbackAdapter::AddPacket(uint32_t ssrc,
size_t length,
const PacedPacketInfo& pacing_info,
Timestamp creation_time) {
RtpPacketSendInfo packet_info;
packet_info.ssrc = ssrc;
packet_info.transport_sequence_number = sequence_number;
packet_info.length = length;
packet_info.pacing_info = pacing_info;
AddPacket(packet_info, 0u, creation_time);
}
void TransportFeedbackAdapter::AddPacket(const RtpPacketSendInfo& packet_info,
size_t overhead_bytes,
Timestamp creation_time) {
{
rtc::CritScope cs(&lock_);
send_time_history_.AddAndRemoveOld(
PacketFeedback(creation_time.ms(), sequence_number, length,
local_net_id_, remote_net_id_, pacing_info),
creation_time.ms());
PacketFeedback packet_feedback(
creation_time.ms(), packet_info.transport_sequence_number,
packet_info.length + overhead_bytes, local_net_id_, remote_net_id_,
packet_info.pacing_info);
if (packet_info.has_rtp_sequence_number) {
packet_feedback.ssrc = packet_info.ssrc;
packet_feedback.rtp_sequence_number = packet_info.rtp_sequence_number;
}
send_time_history_.AddAndRemoveOld(packet_feedback, creation_time.ms());
}
{
rtc::CritScope cs(&observers_lock_);
for (auto* observer : observers_) {
observer->OnPacketAdded(ssrc, sequence_number);
observer->OnPacketAdded(packet_info.ssrc,
packet_info.transport_sequence_number);
}
}
}

5
modules/congestion_controller/rtp/transport_feedback_adapter.h
View File

@ -24,6 +24,7 @@
namespace webrtc {
class PacketFeedbackObserver;
struct RtpPacketSendInfo;
namespace rtcp {
class TransportFeedback;
@ -37,12 +38,16 @@ class TransportFeedbackAdapter {
void RegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
void DeRegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
// TODO(webrtc:8975): Remove when downstream projects have been updated.
void AddPacket(uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info,
Timestamp creation_time);
void AddPacket(const RtpPacketSendInfo& packet_info,
size_t overhead_bytes,
Timestamp creation_time);
absl::optional<SentPacket> ProcessSentPacket(
const rtc::SentPacket& sent_packet);

11
modules/congestion_controller/rtp/transport_feedback_adapter_unittest.cc
View File

@ -64,9 +64,14 @@ class TransportFeedbackAdapterTest : public ::testing::Test {
int64_t now_ms) {}
void OnSentPacket(const PacketFeedback& packet_feedback) {
adapter_->AddPacket(kSsrc, packet_feedback.sequence_number,
packet_feedback.payload_size,
packet_feedback.pacing_info,
RtpPacketSendInfo packet_info;
packet_info.ssrc = kSsrc;
packet_info.transport_sequence_number = packet_feedback.sequence_number;
packet_info.rtp_sequence_number = 0;
packet_info.has_rtp_sequence_number = true;
packet_info.length = packet_feedback.payload_size;
packet_info.pacing_info = packet_feedback.pacing_info;
adapter_->AddPacket(RtpPacketSendInfo(packet_info), 0u,
Timestamp::ms(clock_.TimeInMilliseconds()));
adapter_->ProcessSentPacket(rtc::SentPacket(packet_feedback.sequence_number,
packet_feedback.send_time_ms,

15
modules/congestion_controller/send_side_congestion_controller.cc
View File

@ -365,17 +365,16 @@ void DEPRECATED_SendSideCongestionController::Process() {
MaybeTriggerOnNetworkChanged();
}
void DEPRECATED_SendSideCongestionController::AddPacket(
uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) {
void DEPRECATED_SendSideCongestionController::OnAddPacket(
const RtpPacketSendInfo& packet_info) {
size_t overhead_bytes = 0;
if (send_side_bwe_with_overhead_) {
rtc::CritScope cs(&bwe_lock_);
length += transport_overhead_bytes_per_packet_;
overhead_bytes = transport_overhead_bytes_per_packet_;
}
transport_feedback_adapter_.AddPacket(ssrc, sequence_number, length,
pacing_info);
transport_feedback_adapter_.AddPacket(
packet_info.ssrc, packet_info.transport_sequence_number,
packet_info.length + overhead_bytes, packet_info.pacing_info);
}
void DEPRECATED_SendSideCongestionController::OnTransportFeedback(

13
modules/congestion_controller/send_side_congestion_controller_unittest.cc
View File

@ -77,10 +77,15 @@ class LegacySendSideCongestionControllerTest : public ::testing::Test {
}
void OnSentPacket(const PacketFeedback& packet_feedback) {
constexpr uint32_t ssrc = 0;
controller_->AddPacket(ssrc, packet_feedback.sequence_number,
packet_feedback.payload_size,
packet_feedback.pacing_info);
RtpPacketSendInfo packet_info;
packet_info.ssrc = 0;
packet_info.transport_sequence_number = packet_feedback.sequence_number;
packet_info.rtp_sequence_number = 0;
packet_info.has_rtp_sequence_number = true;
packet_info.length = packet_feedback.payload_size;
packet_info.pacing_info = packet_feedback.pacing_info;
controller_->OnAddPacket(packet_info);
controller_->OnSentPacket(rtc::SentPacket(packet_feedback.sequence_number,
packet_feedback.send_time_ms));
}

5
modules/rtp_rtcp/include/rtp_rtcp_defines.cc
View File

@ -95,11 +95,14 @@ PacketFeedback::PacketFeedback(int64_t creation_time_ms,
arrival_time_ms(arrival_time_ms),
send_time_ms(send_time_ms),
sequence_number(sequence_number),
long_sequence_number(0),
payload_size(payload_size),
unacknowledged_data(0),
local_net_id(local_net_id),
remote_net_id(remote_net_id),
pacing_info(pacing_info) {}
pacing_info(pacing_info),
ssrc(0),
rtp_sequence_number(0) {}
PacketFeedback::PacketFeedback(const PacketFeedback&) = default;
PacketFeedback& PacketFeedback::operator=(const PacketFeedback&) = default;

36
modules/rtp_rtcp/include/rtp_rtcp_defines.h
View File

@ -274,6 +274,10 @@ struct PacketFeedback {
uint16_t remote_net_id;
// Pacing information about this packet.
PacedPacketInfo pacing_info;
// The SSRC and RTP sequence number of the packet this feedback refers to.
uint32_t ssrc;
uint16_t rtp_sequence_number;
};
class PacketFeedbackComparator {
@ -287,16 +291,42 @@ class PacketFeedbackComparator {
}
};
struct RtpPacketSendInfo {
public:
RtpPacketSendInfo() = default;
uint16_t transport_sequence_number = 0;
uint32_t ssrc = 0;
uint16_t rtp_sequence_number = 0;
// Get rid of this flag when all code paths populate |rtp_sequence_number|.
bool has_rtp_sequence_number = false;
size_t length = 0;
PacedPacketInfo pacing_info;
};
class TransportFeedbackObserver {
public:
TransportFeedbackObserver() {}
virtual ~TransportFeedbackObserver() {}
// Note: Transport-wide sequence number as sequence number.
// TODO(webrtc:8975): Remove when downstream projects have been updated.
virtual void AddPacket(uint32_t ssrc,
uint16_t sequence_number,
uint16_t sequence_number, // Transport-wide.
size_t length,
const PacedPacketInfo& pacing_info) = 0;
const PacedPacketInfo& pacing_info) {
RtpPacketSendInfo packet_info;
packet_info.ssrc = ssrc;
packet_info.transport_sequence_number = sequence_number;
packet_info.length = length;
packet_info.pacing_info = pacing_info;
OnAddPacket(packet_info);
}
virtual void OnAddPacket(const RtpPacketSendInfo& packet_info) {
// TODO(webrtc:8975): Remove when downstream projects have been updated.
AddPacket(packet_info.ssrc, packet_info.transport_sequence_number,
packet_info.length, packet_info.pacing_info);
}
virtual void OnTransportFeedback(const rtcp::TransportFeedback& feedback) = 0;
};

4
modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
View File

@ -84,9 +84,7 @@ class MockRtcpCallbackImpl : public RtcpStatisticsCallback {
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
MOCK_METHOD3(AddPacket, void(uint32_t, uint16_t, size_t));
MOCK_METHOD4(AddPacket,
void(uint32_t, uint16_t, size_t, const PacedPacketInfo&));
MOCK_METHOD1(OnAddPacket, void(const RtpPacketSendInfo&));
MOCK_METHOD1(OnTransportFeedback, void(const rtcp::TransportFeedback&));
MOCK_CONST_METHOD0(GetTransportFeedbackVector, std::vector<PacketFeedback>());
};

19
modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -1198,14 +1198,19 @@ void RTPSender::AddPacketToTransportFeedback(
uint16_t packet_id,
const RtpPacketToSend& packet,
const PacedPacketInfo& pacing_info) {
size_t packet_size = packet.payload_size() + packet.padding_size();
if (send_side_bwe_with_overhead_) {
packet_size = packet.size();
}
if (transport_feedback_observer_) {
transport_feedback_observer_->AddPacket(SSRC(), packet_id, packet_size,
pacing_info);
size_t packet_size = packet.payload_size() + packet.padding_size();
if (send_side_bwe_with_overhead_) {
packet_size = packet.size();
}
RtpPacketSendInfo packet_info;
packet_info.ssrc = SSRC();
packet_info.transport_sequence_number = packet_id;
packet_info.rtp_sequence_number = packet.SequenceNumber();
packet_info.length = packet_size;
packet_info.pacing_info = pacing_info;
transport_feedback_observer_->OnAddPacket(packet_info);
}
}

33
modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -69,8 +69,10 @@ const char kNoRid[] = "";
const char kNoMid[] = "";
using ::testing::_;
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
using ::testing::Field;
using ::testing::Invoke;
using ::testing::SizeIs;
@ -162,8 +164,7 @@ class MockSendPacketObserver : public SendPacketObserver {
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
MOCK_METHOD4(AddPacket,
void(uint32_t, uint16_t, size_t, const PacedPacketInfo&));
MOCK_METHOD1(OnAddPacket, void(const RtpPacketSendInfo&));
MOCK_METHOD1(OnTransportFeedback, void(const rtcp::TransportFeedback&));
MOCK_CONST_METHOD0(GetTransportFeedbackVector, std::vector<PacketFeedback>());
};
@ -396,8 +397,14 @@ TEST_P(RtpSenderTestWithoutPacer,
: sizeof(kPayloadData);
EXPECT_CALL(feedback_observer_,
AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber,
expected_bytes, PacedPacketInfo()))
OnAddPacket(AllOf(
Field(&RtpPacketSendInfo::ssrc, rtp_sender_->SSRC()),
Field(&RtpPacketSendInfo::transport_sequence_number,
kTransportSequenceNumber),
Field(&RtpPacketSendInfo::rtp_sequence_number,
rtp_sender_->SequenceNumber()),
Field(&RtpPacketSendInfo::length, expected_bytes),
Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo()))))
.Times(1);
EXPECT_CALL(mock_overhead_observer,
OnOverheadChanged(kRtpOverheadBytesPerPacket))
@ -424,8 +431,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) {
.Times(1);
EXPECT_CALL(feedback_observer_,
AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber, _,
PacedPacketInfo()))
OnAddPacket(AllOf(
Field(&RtpPacketSendInfo::ssrc, rtp_sender_->SSRC()),
Field(&RtpPacketSendInfo::transport_sequence_number,
kTransportSequenceNumber),
Field(&RtpPacketSendInfo::rtp_sequence_number,
rtp_sender_->SequenceNumber()),
Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo()))))
.Times(1);
SendGenericPacket();
@ -597,8 +609,13 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) {
OnSendPacket(kTransportSequenceNumber, _, _))
.Times(1);
EXPECT_CALL(feedback_observer_,
AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber, _,
PacedPacketInfo()))
OnAddPacket(AllOf(
Field(&RtpPacketSendInfo::ssrc, rtp_sender_->SSRC()),
Field(&RtpPacketSendInfo::transport_sequence_number,
kTransportSequenceNumber),
Field(&RtpPacketSendInfo::rtp_sequence_number,
rtp_sender_->SequenceNumber()),
Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo()))))
.Times(1);
SendGenericPacket();

12
rtc_tools/event_log_visualizer/analyzer.cc
View File

@ -1286,10 +1286,16 @@ void EventLogAnalyzer::CreateSendSideBweSimulationGraph(Plot* plot) {
const RtpPacketType& rtp_packet = *rtp_iterator->second;
if (rtp_packet.rtp.header.extension.hasTransportSequenceNumber) {
RTC_DCHECK(rtp_packet.rtp.header.extension.hasTransportSequenceNumber);
RtpPacketSendInfo packet_info;
packet_info.ssrc = rtp_packet.rtp.header.ssrc;
packet_info.rtp_sequence_number =
rtp_packet.rtp.header.extension.transportSequenceNumber;
packet_info.rtp_sequence_number = rtp_packet.rtp.header.sequenceNumber;
packet_info.has_rtp_sequence_number = true;
packet_info.length = rtp_packet.rtp.total_length;
transport_feedback.AddPacket(
rtp_packet.rtp.header.ssrc,
rtp_packet.rtp.header.extension.transportSequenceNumber,
rtp_packet.rtp.total_length, PacedPacketInfo(),
packet_info,
0u, // Per packet overhead bytes.
Timestamp::us(rtp_packet.rtp.log_time_us()));
rtc::SentPacket sent_packet(
rtp_packet.rtp.header.extension.transportSequenceNumber,

11
rtc_tools/event_log_visualizer/log_simulation.cc
View File

@ -80,8 +80,15 @@ void LogBasedNetworkControllerSimulation::OnPacketSent(
pending_probes_.pop_front();
}
}
transport_feedback_.AddPacket(packet.ssrc, packet.transport_seq_no,
packet.size + packet.overhead, probe_info,
RtpPacketSendInfo packet_info;
packet_info.ssrc = packet.ssrc;
packet_info.transport_sequence_number = packet.transport_seq_no;
packet_info.rtp_sequence_number = packet.stream_seq_no;
packet_info.has_rtp_sequence_number = true;
packet_info.length = packet.size;
packet_info.pacing_info = probe_info;
transport_feedback_.AddPacket(packet_info, packet.overhead,
packet.log_packet_time);
}
rtc::SentPacket sent_packet;