Attach TransportFeedbackPacketLossTracker to ANA (PLR only)
This CL is one in a series. To finish the work, the following CLs will be added:
1. CL for connecting RPLR as well
2. CL for RPLR-based FecController
3. CL for allowing experiment-driven configuration of the above (through both field-trials and protobuf)
BUG=webrtc:7058
Review-Url: https://codereview.webrtc.org/2638083002
Cr-Commit-Position: refs/heads/master@{#17365}
diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc
index 6364202..06c660e 100644
--- a/webrtc/audio/audio_send_stream.cc
+++ b/webrtc/audio/audio_send_stream.cc
@@ -19,6 +19,7 @@
#include "webrtc/base/event.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/task_queue.h"
+#include "webrtc/base/timeutils.h"
#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
#include "webrtc/modules/congestion_controller/include/send_side_congestion_controller.h"
#include "webrtc/modules/pacing/paced_sender.h"
@@ -40,6 +41,11 @@
} // namespace
namespace internal {
+// TODO(elad.alon): Subsequent CL will make these values experiment-dependent.
+constexpr size_t kPacketLossTrackerMaxWindowSizeMs = 15000;
+constexpr size_t kPacketLossRateMinNumAckedPackets = 50;
+constexpr size_t kRecoverablePacketLossRateMinNumAckedPairs = 40;
+
AudioSendStream::AudioSendStream(
const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
@@ -53,7 +59,10 @@
config_(config),
audio_state_(audio_state),
bitrate_allocator_(bitrate_allocator),
- send_side_cc_(send_side_cc) {
+ send_side_cc_(send_side_cc),
+ packet_loss_tracker_(kPacketLossTrackerMaxWindowSizeMs,
+ kPacketLossRateMinNumAckedPackets,
+ kRecoverablePacketLossRateMinNumAckedPairs) {
LOG(LS_INFO) << "AudioSendStream: " << config_.ToString();
RTC_DCHECK_NE(config_.voe_channel_id, -1);
RTC_DCHECK(audio_state_.get());
@@ -72,6 +81,7 @@
config_.rtp.nack.rtp_history_ms / 20);
channel_proxy_->RegisterExternalTransport(config.send_transport);
+ send_side_cc_->RegisterPacketFeedbackObserver(this);
for (const auto& extension : config.rtp.extensions) {
if (extension.uri == RtpExtension::kAudioLevelUri) {
@@ -91,11 +101,14 @@
if (!SetupSendCodec()) {
LOG(LS_ERROR) << "Failed to set up send codec state.";
}
+
+ pacer_thread_checker_.DetachFromThread();
}
AudioSendStream::~AudioSendStream() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
LOG(LS_INFO) << "~AudioSendStream: " << config_.ToString();
+ send_side_cc_->DeRegisterPacketFeedbackObserver(this);
channel_proxy_->DeRegisterExternalTransport();
channel_proxy_->ResetCongestionControlObjects();
channel_proxy_->SetRtcEventLog(nullptr);
@@ -103,7 +116,7 @@
}
void AudioSendStream::Start() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (config_.min_bitrate_bps != -1 && config_.max_bitrate_bps != -1) {
RTC_DCHECK_GE(config_.max_bitrate_bps, config_.min_bitrate_bps);
rtc::Event thread_sync_event(false /* manual_reset */, false);
@@ -123,7 +136,7 @@
}
void AudioSendStream::Stop() {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
rtc::Event thread_sync_event(false /* manual_reset */, false);
worker_queue_->PostTask([this, &thread_sync_event] {
bitrate_allocator_->RemoveObserver(this);
@@ -141,19 +154,19 @@
bool AudioSendStream::SendTelephoneEvent(int payload_type,
int payload_frequency, int event,
int duration_ms) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
return channel_proxy_->SetSendTelephoneEventPayloadType(payload_type,
payload_frequency) &&
channel_proxy_->SendTelephoneEventOutband(event, duration_ms);
}
void AudioSendStream::SetMuted(bool muted) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
channel_proxy_->SetInputMute(muted);
}
webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
webrtc::AudioSendStream::Stats stats;
stats.local_ssrc = config_.rtp.ssrc;
@@ -217,14 +230,14 @@
}
void AudioSendStream::SignalNetworkState(NetworkState state) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
}
bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
// TODO(solenberg): Tests call this function on a network thread, libjingle
// calls on the worker thread. We should move towards always using a network
// thread. Then this check can be enabled.
- // RTC_DCHECK(!thread_checker_.CalledOnValidThread());
+ // RTC_DCHECK(!worker_thread_checker_.CalledOnValidThread());
return channel_proxy_->ReceivedRTCPPacket(packet, length);
}
@@ -247,13 +260,43 @@
return 0;
}
+void AudioSendStream::OnPacketAdded(uint32_t ssrc, uint16_t seq_num) {
+ RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread());
+ // Only packets that belong to this stream are of interest.
+ if (ssrc == config_.rtp.ssrc) {
+ rtc::CritScope lock(&packet_loss_tracker_cs_);
+ // TODO(elad.alon): This function call could potentially reset the window,
+ // setting both PLR and RPLR to unknown. Consider (during upcoming
+ // refactoring) passing an indication of such an event.
+ packet_loss_tracker_.OnPacketAdded(seq_num, rtc::TimeMillis());
+ }
+}
+
+void AudioSendStream::OnPacketFeedbackVector(
+ const std::vector<PacketFeedback>& packet_feedback_vector) {
+ // TODO(elad.alon): This fails in UT; fix and uncomment.
+ // RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ rtc::Optional<float> plr;
+ {
+ rtc::CritScope lock(&packet_loss_tracker_cs_);
+ packet_loss_tracker_.OnPacketFeedbackVector(packet_feedback_vector);
+ plr = packet_loss_tracker_.GetPacketLossRate();
+ }
+ // TODO(elad.alon): If PLR goes back to unknown, no indication is given that
+ // the previously sent value is no longer relevant. This will be taken care
+ // of with some refactoring which is now being done.
+ if (plr) {
+ channel_proxy_->OnTwccBasedUplinkPacketLossRate(*plr);
+ }
+}
+
const webrtc::AudioSendStream::Config& AudioSendStream::config() const {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
return config_;
}
void AudioSendStream::SetTransportOverhead(int transport_overhead_per_packet) {
- RTC_DCHECK(thread_checker_.CalledOnValidThread());
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
send_side_cc_->SetTransportOverhead(transport_overhead_per_packet);
channel_proxy_->SetTransportOverhead(transport_overhead_per_packet);
}
diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h
index 436c498..f50f7c4 100644
--- a/webrtc/audio/audio_send_stream.h
+++ b/webrtc/audio/audio_send_stream.h
@@ -12,12 +12,15 @@
#define WEBRTC_AUDIO_AUDIO_SEND_STREAM_H_
#include <memory>
+#include <vector>
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/call/audio_send_stream.h"
#include "webrtc/call/audio_state.h"
#include "webrtc/call/bitrate_allocator.h"
+#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "webrtc/voice_engine/transport_feedback_packet_loss_tracker.h"
namespace webrtc {
class SendSideCongestionController;
@@ -33,7 +36,8 @@
namespace internal {
class AudioSendStream final : public webrtc::AudioSendStream,
- public webrtc::BitrateAllocatorObserver {
+ public webrtc::BitrateAllocatorObserver,
+ public webrtc::PacketFeedbackObserver {
public:
AudioSendStream(const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
@@ -62,6 +66,11 @@
int64_t rtt,
int64_t probing_interval_ms) override;
+ // From PacketFeedbackObserver.
+ void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) override;
+ void OnPacketFeedbackVector(
+ const std::vector<PacketFeedback>& packet_feedback_vector) override;
+
const webrtc::AudioSendStream::Config& config() const;
void SetTransportOverhead(int transport_overhead_per_packet);
@@ -70,7 +79,8 @@
bool SetupSendCodec();
- rtc::ThreadChecker thread_checker_;
+ rtc::ThreadChecker worker_thread_checker_;
+ rtc::ThreadChecker pacer_thread_checker_;
rtc::TaskQueue* worker_queue_;
const webrtc::AudioSendStream::Config config_;
rtc::scoped_refptr<webrtc::AudioState> audio_state_;
@@ -80,6 +90,10 @@
SendSideCongestionController* const send_side_cc_;
std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
+ rtc::CriticalSection packet_loss_tracker_cs_;
+ TransportFeedbackPacketLossTracker packet_loss_tracker_
+ GUARDED_BY(&packet_loss_tracker_cs_);
+
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream);
};
} // namespace internal
diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc
index 9b836be..62772b7 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -106,10 +106,11 @@
receive_side_cc_.Process();
}
-void CongestionController::AddPacket(uint16_t sequence_number,
+void CongestionController::AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) {
- send_side_cc_.AddPacket(sequence_number, length, pacing_info);
+ send_side_cc_.AddPacket(ssrc, sequence_number, length, pacing_info);
}
void CongestionController::OnTransportFeedback(
diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc
index 8073ae1..5621936 100644
--- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc
@@ -79,7 +79,8 @@
}
void OnSentPacket(const PacketFeedback& packet_feedback) {
- controller_->AddPacket(packet_feedback.sequence_number,
+ constexpr uint32_t ssrc = 0;
+ controller_->AddPacket(ssrc, packet_feedback.sequence_number,
packet_feedback.payload_size,
packet_feedback.pacing_info);
controller_->OnSentPacket(rtc::SentPacket(packet_feedback.sequence_number,
diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h
index e962f9f..be2d68d 100644
--- a/webrtc/modules/congestion_controller/include/congestion_controller.h
+++ b/webrtc/modules/congestion_controller/include/congestion_controller.h
@@ -119,7 +119,8 @@
void Process() override;
// Implements TransportFeedbackObserver.
- void AddPacket(uint16_t sequence_number,
+ void AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override;
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;
diff --git a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h
index 9aa4727..8130feb 100644
--- a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h
+++ b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h
@@ -67,6 +67,9 @@
std::unique_ptr<PacedSender> pacer);
virtual ~SendSideCongestionController();
+ void RegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
+ void DeRegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
+
virtual void SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps);
@@ -111,7 +114,8 @@
void Process() override;
// Implements TransportFeedbackObserver.
- void AddPacket(uint16_t sequence_number,
+ void AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override;
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;
diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
index f3cbed9..782c060 100644
--- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc
@@ -83,6 +83,16 @@
SendSideCongestionController::~SendSideCongestionController() {}
+void SendSideCongestionController::RegisterPacketFeedbackObserver(
+ PacketFeedbackObserver* observer) {
+ transport_feedback_adapter_.RegisterPacketFeedbackObserver(observer);
+}
+
+void SendSideCongestionController::DeRegisterPacketFeedbackObserver(
+ PacketFeedbackObserver* observer) {
+ transport_feedback_adapter_.DeRegisterPacketFeedbackObserver(observer);
+}
+
void SendSideCongestionController::SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps) {
@@ -203,10 +213,12 @@
}
void SendSideCongestionController::AddPacket(
+ uint32_t ssrc,
uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) {
- transport_feedback_adapter_.AddPacket(sequence_number, length, pacing_info);
+ transport_feedback_adapter_.AddPacket(ssrc, sequence_number, length,
+ pacing_info);
}
void SendSideCongestionController::OnTransportFeedback(
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
index ddffa72..9fb1af5 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
+++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
@@ -36,19 +36,49 @@
local_net_id_(0),
remote_net_id_(0) {}
-TransportFeedbackAdapter::~TransportFeedbackAdapter() {}
+TransportFeedbackAdapter::~TransportFeedbackAdapter() {
+ RTC_DCHECK(observers_.empty());
+}
-void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number,
+void TransportFeedbackAdapter::RegisterPacketFeedbackObserver(
+ PacketFeedbackObserver* observer) {
+ rtc::CritScope cs(&observers_lock_);
+ RTC_DCHECK(observer);
+ RTC_DCHECK(std::find(observers_.begin(), observers_.end(), observer) ==
+ observers_.end());
+ observers_.push_back(observer);
+}
+
+void TransportFeedbackAdapter::DeRegisterPacketFeedbackObserver(
+ PacketFeedbackObserver* observer) {
+ rtc::CritScope cs(&observers_lock_);
+ RTC_DCHECK(observer);
+ const auto it = std::find(observers_.begin(), observers_.end(), observer);
+ RTC_DCHECK(it != observers_.end());
+ observers_.erase(it);
+}
+
+void TransportFeedbackAdapter::AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) {
- rtc::CritScope cs(&lock_);
- if (send_side_bwe_with_overhead_) {
- length += transport_overhead_bytes_per_packet_;
+ {
+ rtc::CritScope cs(&lock_);
+ if (send_side_bwe_with_overhead_) {
+ length += transport_overhead_bytes_per_packet_;
+ }
+ const int64_t creation_time_ms = clock_->TimeInMilliseconds();
+ send_time_history_.AddAndRemoveOld(
+ PacketFeedback(creation_time_ms, sequence_number, length, local_net_id_,
+ remote_net_id_, pacing_info));
}
- const int64_t creation_time_ms = clock_->TimeInMilliseconds();
- send_time_history_.AddAndRemoveOld(
- PacketFeedback(creation_time_ms, sequence_number, length, local_net_id_,
- remote_net_id_, pacing_info));
+
+ {
+ rtc::CritScope cs(&observers_lock_);
+ for (auto observer : observers_) {
+ observer->OnPacketAdded(ssrc, sequence_number);
+ }
+ }
}
void TransportFeedbackAdapter::OnSentPacket(uint16_t sequence_number,
@@ -154,6 +184,12 @@
void TransportFeedbackAdapter::OnTransportFeedback(
const rtcp::TransportFeedback& feedback) {
last_packet_feedback_vector_ = GetPacketFeedbackVector(feedback);
+ {
+ rtc::CritScope cs(&observers_lock_);
+ for (auto observer : observers_) {
+ observer->OnPacketFeedbackVector(last_packet_feedback_vector_);
+ }
+ }
}
std::vector<PacketFeedback>
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h
index 616bebe..2e70e44 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h
+++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h
@@ -21,6 +21,8 @@
namespace webrtc {
+class PacketFeedbackObserver;
+
namespace rtcp {
class TransportFeedback;
} // namespace rtcp
@@ -30,7 +32,11 @@
explicit TransportFeedbackAdapter(const Clock* clock);
virtual ~TransportFeedbackAdapter();
- void AddPacket(uint16_t sequence_number,
+ void RegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
+ void DeRegisterPacketFeedbackObserver(PacketFeedbackObserver* observer);
+
+ void AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info);
void OnSentPacket(uint16_t sequence_number, int64_t send_time_ms);
@@ -57,8 +63,11 @@
int64_t current_offset_ms_;
int64_t last_timestamp_us_;
std::vector<PacketFeedback> last_packet_feedback_vector_;
- uint16_t local_net_id_;
- uint16_t remote_net_id_;
+ uint16_t local_net_id_ GUARDED_BY(&lock_);
+ uint16_t remote_net_id_ GUARDED_BY(&lock_);
+
+ rtc::CriticalSection observers_lock_;
+ std::vector<PacketFeedbackObserver*> observers_ GUARDED_BY(&observers_lock_);
};
} // namespace webrtc
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
index 22e8b59..97b7f5f 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
+++ b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
@@ -38,6 +38,13 @@
namespace test {
+class MockPacketFeedbackObserver : public webrtc::PacketFeedbackObserver {
+ public:
+ MOCK_METHOD2(OnPacketAdded, void(uint32_t ssrc, uint16_t seq_num));
+ MOCK_METHOD1(OnPacketFeedbackVector,
+ void(const std::vector<PacketFeedback>& packet_feedback_vector));
+};
+
class TransportFeedbackAdapterTest : public ::testing::Test {
public:
TransportFeedbackAdapterTest() : clock_(0) {}
@@ -58,17 +65,75 @@
int64_t now_ms) {}
void OnSentPacket(const PacketFeedback& packet_feedback) {
- adapter_->AddPacket(packet_feedback.sequence_number,
+ adapter_->AddPacket(kSsrc, packet_feedback.sequence_number,
packet_feedback.payload_size,
packet_feedback.pacing_info);
adapter_->OnSentPacket(packet_feedback.sequence_number,
packet_feedback.send_time_ms);
}
+ static constexpr uint32_t kSsrc = 8492;
+
SimulatedClock clock_;
std::unique_ptr<TransportFeedbackAdapter> adapter_;
};
+TEST_F(TransportFeedbackAdapterTest, ObserverSanity) {
+ MockPacketFeedbackObserver mock;
+ adapter_->RegisterPacketFeedbackObserver(&mock);
+
+ const std::vector<PacketFeedback> packets = {
+ PacketFeedback(100, 200, 0, 1000, kPacingInfo0),
+ PacketFeedback(110, 210, 1, 2000, kPacingInfo0),
+ PacketFeedback(120, 220, 2, 3000, kPacingInfo0)
+ };
+
+ rtcp::TransportFeedback feedback;
+ feedback.SetBase(packets[0].sequence_number,
+ packets[0].arrival_time_ms * 1000);
+
+ for (const PacketFeedback& packet : packets) {
+ EXPECT_CALL(mock, OnPacketAdded(kSsrc, packet.sequence_number)).Times(1);
+ OnSentPacket(packet);
+ EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number,
+ packet.arrival_time_ms * 1000));
+ }
+
+ EXPECT_CALL(mock, OnPacketFeedbackVector(_)).Times(1);
+ adapter_->OnTransportFeedback(feedback);
+
+ adapter_->DeRegisterPacketFeedbackObserver(&mock);
+
+ // After deregistration, the observer no longers gets indications.
+ EXPECT_CALL(mock, OnPacketAdded(_, _)).Times(0);
+ const PacketFeedback new_packet(130, 230, 3, 4000, kPacingInfo0);
+ OnSentPacket(new_packet);
+
+ rtcp::TransportFeedback second_feedback;
+ second_feedback.SetBase(new_packet.sequence_number,
+ new_packet.arrival_time_ms * 1000);
+ EXPECT_TRUE(feedback.AddReceivedPacket(new_packet.sequence_number,
+ new_packet.arrival_time_ms * 1000));
+ EXPECT_CALL(mock, OnPacketFeedbackVector(_)).Times(0);
+ adapter_->OnTransportFeedback(second_feedback);
+}
+
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+TEST_F(TransportFeedbackAdapterTest, ObserverDoubleRegistrationDeathTest) {
+ MockPacketFeedbackObserver mock;
+ adapter_->RegisterPacketFeedbackObserver(&mock);
+ EXPECT_DEATH(adapter_->RegisterPacketFeedbackObserver(&mock), "");
+ adapter_->DeRegisterPacketFeedbackObserver(&mock);
+}
+
+TEST_F(TransportFeedbackAdapterTest, ObserverMissingDeRegistrationDeathTest) {
+ MockPacketFeedbackObserver mock;
+ adapter_->RegisterPacketFeedbackObserver(&mock);
+ EXPECT_DEATH(adapter_.reset(), "");
+ adapter_->DeRegisterPacketFeedbackObserver(&mock);
+}
+#endif
+
TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
std::vector<PacketFeedback> packets;
packets.push_back(PacketFeedback(100, 200, 0, 1500, kPacingInfo0));
diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index 7df8a13..ddfec4d 100644
--- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -342,7 +342,8 @@
virtual ~TransportFeedbackObserver() {}
// Note: Transport-wide sequence number as sequence number.
- virtual void AddPacket(uint16_t sequence_number,
+ virtual void AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) = 0;
@@ -351,6 +352,15 @@
virtual std::vector<PacketFeedback> GetTransportFeedbackVector() const = 0;
};
+class PacketFeedbackObserver {
+ public:
+ virtual ~PacketFeedbackObserver() = default;
+
+ virtual void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) = 0;
+ virtual void OnPacketFeedbackVector(
+ const std::vector<PacketFeedback>& packet_feedback_vector) = 0;
+};
+
class RtcpRttStats {
public:
virtual void OnRttUpdate(int64_t rtt) = 0;
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index 4b0255f..d419caf 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -73,8 +73,9 @@
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
- MOCK_METHOD2(AddPacket, void(uint16_t, size_t));
- MOCK_METHOD3(AddPacket, void(uint16_t, size_t, const PacedPacketInfo&));
+ MOCK_METHOD3(AddPacket, void(uint32_t, uint16_t, size_t));
+ MOCK_METHOD4(AddPacket,
+ void(uint32_t, uint16_t, size_t, const PacedPacketInfo&));
MOCK_METHOD1(OnTransportFeedback, void(const rtcp::TransportFeedback&));
MOCK_CONST_METHOD0(GetTransportFeedbackVector, std::vector<PacketFeedback>());
};
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index 2fd8b3e..52b0891 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -1256,7 +1256,7 @@
}
if (transport_feedback_observer_) {
- transport_feedback_observer_->AddPacket(packet_id, packet_size,
+ transport_feedback_observer_->AddPacket(SSRC(), packet_id, packet_size,
pacing_info);
}
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index fed52ee..6539f73 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -126,7 +126,8 @@
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
- MOCK_METHOD3(AddPacket, void(uint16_t, size_t, const PacedPacketInfo&));
+ MOCK_METHOD4(AddPacket,
+ void(uint32_t, uint16_t, size_t, const PacedPacketInfo&));
MOCK_METHOD1(OnTransportFeedback, void(const rtcp::TransportFeedback&));
MOCK_CONST_METHOD0(GetTransportFeedbackVector, std::vector<PacketFeedback>());
};
@@ -355,7 +356,7 @@
.Times(1);
EXPECT_CALL(
feedback_observer_,
- AddPacket(kTransportSequenceNumber,
+ AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber,
sizeof(kPayloadData) + kGenericHeaderLength, PacedPacketInfo()))
.Times(1);
@@ -406,7 +407,7 @@
.Times(1);
EXPECT_CALL(
feedback_observer_,
- AddPacket(kTransportSequenceNumber,
+ AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber,
sizeof(kPayloadData) + kGenericHeaderLength, PacedPacketInfo()))
.Times(1);
@@ -1487,7 +1488,7 @@
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(testing::Return(kTransportSequenceNumber));
EXPECT_CALL(feedback_observer_,
- AddPacket(kTransportSequenceNumber,
+ AddPacket(rtp_sender_->SSRC(), kTransportSequenceNumber,
sizeof(kPayloadData) + kGenericHeaderLength +
kRtpOverheadBytesPerPacket,
PacedPacketInfo()))
diff --git a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
index bcb075d..cf19573 100644
--- a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
+++ b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
@@ -245,7 +245,7 @@
if (!may_continue) {
return false;
}
- tracker->OnNewTransportFeedbackVector(feedback_vector);
+ tracker->OnPacketFeedbackVector(feedback_vector);
tracker->Validate();
}
diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h
index 9aeb2d1..24adcc2 100644
--- a/webrtc/test/mock_voe_channel_proxy.h
+++ b/webrtc/test/mock_voe_channel_proxy.h
@@ -87,6 +87,7 @@
MOCK_METHOD1(SetSendCodec, bool(const CodecInst& codec_inst));
MOCK_METHOD2(SetSendCNPayloadType,
bool(int type, PayloadFrequencies frequency));
+ MOCK_METHOD1(OnTwccBasedUplinkPacketLossRate, void(float packet_loss_rate));
};
} // namespace test
} // namespace webrtc
diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc
index fa772d7..c986ff7 100644
--- a/webrtc/tools/event_log_visualizer/analyzer.cc
+++ b/webrtc/tools/event_log_visualizer/analyzer.cc
@@ -1070,7 +1070,8 @@
const LoggedRtpPacket& rtp = *rtp_iterator->second;
if (rtp.header.extension.hasTransportSequenceNumber) {
RTC_DCHECK(rtp.header.extension.hasTransportSequenceNumber);
- cc.AddPacket(rtp.header.extension.transportSequenceNumber,
+ cc.AddPacket(rtp.header.ssrc,
+ rtp.header.extension.transportSequenceNumber,
rtp.total_length, PacedPacketInfo());
rtc::SentPacket sent_packet(
rtp.header.extension.transportSequenceNumber, rtp.timestamp / 1000);
@@ -1169,7 +1170,8 @@
const LoggedRtpPacket& rtp = *rtp_iterator->second;
if (rtp.header.extension.hasTransportSequenceNumber) {
RTC_DCHECK(rtp.header.extension.hasTransportSequenceNumber);
- feedback_adapter.AddPacket(rtp.header.extension.transportSequenceNumber,
+ feedback_adapter.AddPacket(rtp.header.ssrc,
+ rtp.header.extension.transportSequenceNumber,
rtp.total_length, PacedPacketInfo());
feedback_adapter.OnSentPacket(
rtp.header.extension.transportSequenceNumber, rtp.timestamp / 1000);
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 0f0831c..7e49e32 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -246,13 +246,14 @@
}
// Implements TransportFeedbackObserver.
- void AddPacket(uint16_t sequence_number,
+ void AddPacket(uint32_t ssrc,
+ uint16_t sequence_number,
size_t length,
const PacedPacketInfo& pacing_info) override {
RTC_DCHECK(pacer_thread_.CalledOnValidThread());
rtc::CritScope lock(&crit_);
if (feedback_observer_)
- feedback_observer_->AddPacket(sequence_number, length, pacing_info);
+ feedback_observer_->AddPacket(ssrc, sequence_number, length, pacing_info);
}
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {
@@ -395,7 +396,7 @@
(fraction_lost_aggregate + total_number_of_packets / 2) /
total_number_of_packets;
}
- owner_->OnIncomingFractionLoss(weighted_fraction_lost);
+ owner_->OnUplinkPacketLossRate(weighted_fraction_lost / 255.0f);
}
private:
@@ -902,7 +903,9 @@
rtp_packet_sender_proxy_(new RtpPacketSenderProxy()),
retransmission_rate_limiter_(new RateLimiter(Clock::GetRealTimeClock(),
kMaxRetransmissionWindowMs)),
- decoder_factory_(config.acm_config.decoder_factory) {
+ decoder_factory_(config.acm_config.decoder_factory),
+ // TODO(elad.alon): Subsequent CL experiments with PLR source.
+ use_twcc_plr_for_ana_(false) {
WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId, _channelId),
"Channel::Channel() - ctor");
AudioCodingModule::Config acm_config(config.acm_config);
@@ -1301,10 +1304,23 @@
retransmission_rate_limiter_->SetMaxRate(bitrate_bps);
}
-void Channel::OnIncomingFractionLoss(int fraction_lost) {
+void Channel::OnTwccBasedUplinkPacketLossRate(float packet_loss_rate) {
+ if (!use_twcc_plr_for_ana_)
+ return;
audio_coding_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
- if (*encoder)
- (*encoder)->OnReceivedUplinkPacketLossFraction(fraction_lost / 255.0f);
+ if (*encoder) {
+ (*encoder)->OnReceivedUplinkPacketLossFraction(packet_loss_rate);
+ }
+ });
+}
+
+void Channel::OnUplinkPacketLossRate(float packet_loss_rate) {
+ if (use_twcc_plr_for_ana_)
+ return;
+ audio_coding_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
+ if (*encoder) {
+ (*encoder)->OnReceivedUplinkPacketLossFraction(packet_loss_rate);
+ }
});
}
diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h
index 68d022d..d24eb5f 100644
--- a/webrtc/voice_engine/channel.h
+++ b/webrtc/voice_engine/channel.h
@@ -378,10 +378,15 @@
// From OverheadObserver in the RTP/RTCP module
void OnOverheadChanged(size_t overhead_bytes_per_packet) override;
- protected:
- void OnIncomingFractionLoss(int fraction_lost);
+ // The existence of this function alongside OnUplinkPacketLossRate is
+ // a compromise. We want the encoder to be agnostic of the PLR source, but
+ // we also don't want it to receive conflicting information from TWCC and
+ // from RTCP-XR.
+ void OnTwccBasedUplinkPacketLossRate(float packet_loss_rate);
private:
+ void OnUplinkPacketLossRate(float packet_loss_rate);
+
bool InputMute() const;
bool OnRtpPacketWithHeader(const uint8_t* received_packet,
size_t length,
@@ -508,6 +513,8 @@
rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_;
rtc::ThreadChecker construction_thread_;
+
+ const bool use_twcc_plr_for_ana_;
};
} // namespace voe
diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc
index 7f6e96c..5388b8d 100644
--- a/webrtc/voice_engine/channel_proxy.cc
+++ b/webrtc/voice_engine/channel_proxy.cc
@@ -367,6 +367,11 @@
return channel()->SetSendCNPayloadType(type, frequency) == 0;
}
+void ChannelProxy::OnTwccBasedUplinkPacketLossRate(float packet_loss_rate) {
+ RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+ channel()->OnTwccBasedUplinkPacketLossRate(packet_loss_rate);
+}
+
Channel* ChannelProxy::channel() const {
RTC_DCHECK(channel_owner_.channel());
return channel_owner_.channel();
diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h
index 28d0364..fd25378 100644
--- a/webrtc/voice_engine/channel_proxy.h
+++ b/webrtc/voice_engine/channel_proxy.h
@@ -116,6 +116,7 @@
virtual bool SetOpusMaxPlaybackRate(int frequency_hz);
virtual bool SetSendCodec(const CodecInst& codec_inst);
virtual bool SetSendCNPayloadType(int type, PayloadFrequencies frequency);
+ virtual void OnTwccBasedUplinkPacketLossRate(float packet_loss_rate);
private:
Channel* channel() const;
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
index 6cdeec2..07af861 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
@@ -98,7 +98,7 @@
}
}
-void TransportFeedbackPacketLossTracker::OnNewTransportFeedbackVector(
+void TransportFeedbackPacketLossTracker::OnPacketFeedbackVector(
const std::vector<PacketFeedback>& packet_feedback_vector) {
for (const PacketFeedback& packet : packet_feedback_vector) {
const auto& it = packet_status_window_.find(packet.sequence_number);
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
index 231fb70..ea82f74 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
@@ -38,7 +38,7 @@
void OnPacketAdded(uint16_t seq_num, int64_t send_time_ms);
- void OnNewTransportFeedbackVector(
+ void OnPacketFeedbackVector(
const std::vector<PacketFeedback>& packet_feedbacks_vector);
// Returns the packet loss rate, if the window has enough packet statuses to
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
index 1e463dd..9f7d578 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
@@ -86,7 +86,7 @@
++seq_num;
}
- tracker->OnNewTransportFeedbackVector(packet_feedback_vector);
+ tracker->OnPacketFeedbackVector(packet_feedback_vector);
tracker->Validate();
}