Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(432)

Unified Diff: webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc

Issue 2378103005: Reland: Fix race / crash in OnNetworkRouteChanged(). (Closed)
Patch Set: . Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
similarity index 78%
rename from webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
rename to webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
index b312f96f107277fc63a9b722d2af2af4ee674cbf..0c77fe9ede158500af809bf9185298c2f46d3359 100644
--- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter_unittest.cc
+++ b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc
@@ -17,8 +17,7 @@
#include "webrtc/base/checks.h"
#include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h"
-#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h"
-#include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h"
+#include "webrtc/modules/congestion_controller/transport_feedback_adapter.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "webrtc/system_wrappers/include/clock.h"
@@ -32,23 +31,16 @@ namespace test {
class TransportFeedbackAdapterTest : public ::testing::Test {
public:
TransportFeedbackAdapterTest()
- : clock_(0),
- bitrate_estimator_(nullptr),
- bitrate_controller_(this),
- receiver_estimated_bitrate_(0) {}
+ : clock_(0), bitrate_controller_(this), target_bitrate_bps_(0) {}
virtual ~TransportFeedbackAdapterTest() {}
virtual void SetUp() {
- adapter_.reset(new TransportFeedbackAdapter(&clock_));
-
- bitrate_estimator_ = new MockRemoteBitrateEstimator();
- adapter_->SetBitrateEstimator(bitrate_estimator_);
+ adapter_.reset(new TransportFeedbackAdapter(&clock_, &bitrate_controller_));
+ adapter_->InitBwe();
}
- virtual void TearDown() {
- adapter_.reset();
- }
+ virtual void TearDown() { adapter_.reset(); }
protected:
// Proxy class used since TransportFeedbackAdapter will own the instance
@@ -60,9 +52,8 @@ class TransportFeedbackAdapterTest : public ::testing::Test {
~MockBitrateControllerAdapter() override {}
- void OnReceiveBitrateChanged(const std::vector<uint32_t>& ssrcs,
- uint32_t bitrate_bps) override {
- owner_->receiver_estimated_bitrate_ = bitrate_bps;
+ void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {
+ owner_->target_bitrate_bps_ = result.target_bitrate_bps;
}
TransportFeedbackAdapterTest* const owner_;
@@ -106,11 +97,10 @@ class TransportFeedbackAdapterTest : public ::testing::Test {
}
SimulatedClock clock_;
- MockRemoteBitrateEstimator* bitrate_estimator_;
MockBitrateControllerAdapter bitrate_controller_;
std::unique_ptr<TransportFeedbackAdapter> adapter_;
- uint32_t receiver_estimated_bitrate_;
+ uint32_t target_bitrate_bps_;
};
TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
@@ -135,13 +125,8 @@ TEST_F(TransportFeedbackAdapterTest, AdaptsFeedbackAndPopulatesSendTimes) {
feedback.Build();
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke(
- [packets, this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(packets, adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) {
@@ -177,13 +162,9 @@ TEST_F(TransportFeedbackAdapterTest, HandlesDroppedPackets) {
packets.begin() + kSendSideDropBefore,
packets.begin() + kReceiveSideDropAfter + 1);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets,
- this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
@@ -217,13 +198,9 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) {
std::vector<PacketInfo> expected_packets;
expected_packets.push_back(packets[i]);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets, this](
- const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(*feedback.get());
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
}
@@ -251,13 +228,9 @@ TEST_F(TransportFeedbackAdapterTest, HandlesReordering) {
feedback.Build();
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets,
- this](const std::vector<PacketInfo>& feedback_vector) {
- ComparePacketVectors(expected_packets, feedback_vector);
- }));
adapter_->OnTransportFeedback(feedback);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
}
TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
@@ -294,14 +267,6 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
info.arrival_time_ms += (kLargePositiveDeltaUs + 1000) / 1000;
++info.sequence_number;
- // Expected to be ordered on arrival time when the feedback message has been
- // parsed.
- std::vector<PacketInfo> expected_packets;
- expected_packets.push_back(sent_packets[0]);
- expected_packets.push_back(sent_packets[3]);
- expected_packets.push_back(sent_packets[1]);
- expected_packets.push_back(sent_packets[2]);
-
// Packets will be added to send history.
for (const PacketInfo& packet : sent_packets)
OnSentPacket(packet);
@@ -327,14 +292,18 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
std::vector<PacketInfo> received_feedback;
EXPECT_TRUE(feedback.get() != nullptr);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke([expected_packets, &received_feedback](
- const std::vector<PacketInfo>& feedback_vector) {
- EXPECT_EQ(expected_packets.size(), feedback_vector.size());
- received_feedback = feedback_vector;
- }));
adapter_->OnTransportFeedback(*feedback.get());
+ {
+ // Expected to be ordered on arrival time when the feedback message has been
+ // parsed.
+ std::vector<PacketInfo> expected_packets;
+ expected_packets.push_back(sent_packets[0]);
+ expected_packets.push_back(sent_packets[3]);
+ expected_packets.push_back(sent_packets[1]);
+ expected_packets.push_back(sent_packets[2]);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
+ }
// Create a new feedback message and add the trailing item.
feedback.reset(new rtcp::TransportFeedback());
@@ -346,18 +315,41 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) {
rtcp::TransportFeedback::ParseFrom(raw_packet.data(), raw_packet.size());
EXPECT_TRUE(feedback.get() != nullptr);
- EXPECT_CALL(*bitrate_estimator_, IncomingPacketFeedbackVector(_))
- .Times(1)
- .WillOnce(Invoke(
- [&received_feedback](const std::vector<PacketInfo>& feedback_vector) {
- EXPECT_EQ(1u, feedback_vector.size());
- received_feedback.push_back(feedback_vector[0]);
- }));
adapter_->OnTransportFeedback(*feedback.get());
+ {
+ std::vector<PacketInfo> expected_packets;
+ expected_packets.push_back(info);
+ ComparePacketVectors(expected_packets,
+ adapter_->GetTransportFeedbackVector());
+ }
+}
- expected_packets.push_back(info);
-
- ComparePacketVectors(expected_packets, received_feedback);
+TEST_F(TransportFeedbackAdapterTest, UpdatesDelayBasedEstimate) {
+ uint16_t seq_num = 0;
+ size_t kPayloadSize = 1000;
+ // The test must run and insert packets/feedback long enough that the
+ // BWE computes a valid estimate.
+ const int64_t kRunTimeMs = 6000;
+ int64_t start_time_ms = clock_.TimeInMilliseconds();
+ while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) {
+ PacketInfo packet(clock_.TimeInMilliseconds(), clock_.TimeInMilliseconds(),
+ seq_num, kPayloadSize, PacketInfo::kNotAProbe);
+ OnSentPacket(packet);
+ // Create expected feedback and send into adapter.
+ std::unique_ptr<rtcp::TransportFeedback> feedback(
+ new rtcp::TransportFeedback());
+ feedback->SetBase(packet.sequence_number, packet.arrival_time_ms * 1000);
+ EXPECT_TRUE(feedback->AddReceivedPacket(packet.sequence_number,
+ packet.arrival_time_ms * 1000));
+ rtc::Buffer raw_packet = feedback->Build();
+ feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(),
+ raw_packet.size());
+ EXPECT_TRUE(feedback.get() != nullptr);
+ adapter_->OnTransportFeedback(*feedback.get());
+ clock_.AdvanceTimeMilliseconds(50);
+ ++seq_num;
+ }
+ EXPECT_GT(target_bitrate_bps_, 0u);
}
} // namespace test
« no previous file with comments | « webrtc/modules/congestion_controller/transport_feedback_adapter.cc ('k') | webrtc/modules/remote_bitrate_estimator/BUILD.gn » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698