From 575ceefc6df49587df0d590fa8dcc5cdb8f61ed2 Mon Sep 17 00:00:00 2001 From: Lu Liu Date: Wed, 29 Nov 2017 21:14:53 +0000 Subject: [PATCH] Revert "Replaced magic numbers with constants for PacketFeedback." This reverts commit 37b52232895fc200188c0e3ded261aedcb558b7b. Reason for revert: Breaking internal builds Original change's description: > Replaced magic numbers with constants for PacketFeedback. > > Bug: None > Change-Id: Ie22475227406f4e800052b52fa644ea6966db3f1 > Reviewed-on: https://webrtc-review.googlesource.com/27100 > Reviewed-by: Stefan Holmer > Commit-Queue: Sebastian Jansson > Cr-Commit-Position: refs/heads/master@{#20938} TBR=stefan@webrtc.org,srte@webrtc.org Change-Id: I891977c9535c4c887013f3f5badc83666c29e3f8 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: None Reviewed-on: https://webrtc-review.googlesource.com/27220 Reviewed-by: Lu Liu Commit-Queue: Lu Liu Cr-Commit-Position: refs/heads/master@{#20943} --- .../acknowledged_bitrate_estimator.cc | 2 +- .../congestion_controller/delay_based_bwe_unittest.cc | 10 ++++------ .../send_side_congestion_controller_unittest.cc | 2 +- .../send_time_history_unittest.cc | 4 ++-- modules/rtp_rtcp/include/rtp_rtcp_defines.h | 8 +++----- rtc_tools/event_log_visualizer/analyzer.cc | 2 +- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/modules/congestion_controller/acknowledged_bitrate_estimator.cc b/modules/congestion_controller/acknowledged_bitrate_estimator.cc index c0161ea0d9..223c4769e6 100644 --- a/modules/congestion_controller/acknowledged_bitrate_estimator.cc +++ b/modules/congestion_controller/acknowledged_bitrate_estimator.cc @@ -19,7 +19,7 @@ namespace webrtc { namespace { bool IsInSendTimeHistory(const PacketFeedback& packet) { - return packet.send_time_ms != PacketFeedback::kNoSendTime; + return packet.send_time_ms >= 0; } } // namespace diff --git a/modules/congestion_controller/delay_based_bwe_unittest.cc b/modules/congestion_controller/delay_based_bwe_unittest.cc index 250cb036a3..e53667b16f 100644 --- a/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -34,12 +34,10 @@ TEST_F(DelayBasedBweTest, NoCrashEmptyFeedback) { TEST_F(DelayBasedBweTest, NoCrashOnlyLostFeedback) { std::vector packet_feedback_vector; - packet_feedback_vector.push_back(PacketFeedback(PacketFeedback::kNotReceived, - PacketFeedback::kNoSendTime, - 0, 1500, PacedPacketInfo())); - packet_feedback_vector.push_back(PacketFeedback(PacketFeedback::kNotReceived, - PacketFeedback::kNoSendTime, - 1, 1500, PacedPacketInfo())); + packet_feedback_vector.push_back( + PacketFeedback(-1, -1, 0, 1500, PacedPacketInfo())); + packet_feedback_vector.push_back( + PacketFeedback(-1, -1, 1, 1500, PacedPacketInfo())); bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector, rtc::nullopt); } diff --git a/modules/congestion_controller/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/send_side_congestion_controller_unittest.cc index 67d2c402c4..71aff4a620 100644 --- a/modules/congestion_controller/send_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/send_side_congestion_controller_unittest.cc @@ -384,7 +384,7 @@ TEST_F(SendSideCongestionControllerTest, LongFeedbackDelays) { // Check that packets have timed out. for (PacketFeedback& packet : packets) { - packet.send_time_ms = PacketFeedback::kNoSendTime; + packet.send_time_ms = -1; packet.payload_size = 0; packet.pacing_info = PacedPacketInfo(); } diff --git a/modules/remote_bitrate_estimator/send_time_history_unittest.cc b/modules/remote_bitrate_estimator/send_time_history_unittest.cc index 6f05bcf846..bda245c03c 100644 --- a/modules/remote_bitrate_estimator/send_time_history_unittest.cc +++ b/modules/remote_bitrate_estimator/send_time_history_unittest.cc @@ -116,8 +116,8 @@ TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) { } for (size_t i = 0; i < num_items; ++i) { PacketFeedback packet = sent_packets[i]; - packet.arrival_time_ms = PacketFeedback::kNotReceived; - packet.send_time_ms = PacketFeedback::kNoSendTime; + packet.arrival_time_ms = -1; + packet.send_time_ms = -1; history_.AddAndRemoveOld(packet); } for (size_t i = 0; i < num_items; ++i) diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index ede19939df..0c8e389c04 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -282,7 +282,6 @@ class RtcpBandwidthObserver { int64_t rtt, int64_t now_ms) = 0; - protected: virtual ~RtcpBandwidthObserver() {} }; @@ -290,7 +289,7 @@ struct PacketFeedback { PacketFeedback(int64_t arrival_time_ms, uint16_t sequence_number) : PacketFeedback(-1, arrival_time_ms, - kNoSendTime, + -1, sequence_number, 0, 0, @@ -318,8 +317,8 @@ struct PacketFeedback { uint16_t remote_net_id, const PacedPacketInfo& pacing_info) : PacketFeedback(creation_time_ms, - kNotReceived, - kNoSendTime, + -1, + -1, sequence_number, payload_size, local_net_id, @@ -345,7 +344,6 @@ struct PacketFeedback { static constexpr int kNotAProbe = -1; static constexpr int64_t kNotReceived = -1; - static constexpr int64_t kNoSendTime = -1; // NOTE! The variable |creation_time_ms| is not used when testing equality. // This is due to |creation_time_ms| only being used by SendTimeHistory diff --git a/rtc_tools/event_log_visualizer/analyzer.cc b/rtc_tools/event_log_visualizer/analyzer.cc index 21bc5808e1..bbbde3f4c1 100644 --- a/rtc_tools/event_log_visualizer/analyzer.cc +++ b/rtc_tools/event_log_visualizer/analyzer.cc @@ -1443,7 +1443,7 @@ void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { float x = static_cast(clock.TimeInMicroseconds() - begin_time_) / 1000000; - if (packet.send_time_ms == PacketFeedback::kNoSendTime) { + if (packet.send_time_ms == -1) { late_feedback_series.points.emplace_back(x, prev_y); continue; }