From f65309cd47e3de323ee6d569a87bc424e5c42157 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 20 Dec 2018 10:26:00 +0100 Subject: [PATCH] Removes return value and Try prefix from TryDeliverPacket. The return value is not used. This change prepares for future refactoring by removing the requirement that TryDeliverPacket must be synchronous. Also renaming to DeliverPacket as we no longer need to indicate the meaning of the return value. Bug: webrtc:9510 Change-Id: I78536434b198fa7bf4df88b10d6add23684767f1 Reviewed-on: https://webrtc-review.googlesource.com/c/115181 Commit-Queue: Sebastian Jansson Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#26066} --- test/scenario/call_client.cc | 13 +++++----- test/scenario/call_client.h | 6 ++--- test/scenario/network_node.cc | 43 +++++++++++++++------------------ test/scenario/network_node.h | 24 +++++++++--------- test/scenario/scenario.cc | 8 +++--- test/scenario/simulated_time.cc | 26 +++++++++----------- test/scenario/simulated_time.h | 12 ++++----- 7 files changed, 64 insertions(+), 68 deletions(-) diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc index ccfcd5527c..03ff8b76cb 100644 --- a/test/scenario/call_client.cc +++ b/test/scenario/call_client.cc @@ -166,9 +166,9 @@ Call::Stats CallClient::GetStats() { return call_->GetStats(); } -bool CallClient::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) { +void CallClient::DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) { // Removes added overhead before delivering packet to sender. RTC_DCHECK_GE(packet.size(), route_overhead_.at(receiver).bytes()); packet.SetSize(packet.size() - route_overhead_.at(receiver).bytes()); @@ -178,12 +178,13 @@ bool CallClient::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, RTPHeader header; bool success = header_parser_->Parse(packet.cdata(), packet.size(), &header); - if (!success) - return false; + if (!success) { + RTC_DLOG(LS_ERROR) << "Failed to parse RTP header of packet"; + return; + } media_type = ssrc_media_types_[header.ssrc]; } call_->Receiver()->DeliverPacket(media_type, packet, at_time.us()); - return true; } uint32_t CallClient::GetNextVideoSsrc() { diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h index aa5fa93c77..c0be18da46 100644 --- a/test/scenario/call_client.h +++ b/test/scenario/call_client.h @@ -71,9 +71,9 @@ class CallClient : public NetworkReceiverInterface { return DataRate::bps(GetStats().send_bandwidth_bps); } - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; private: friend class Scenario; diff --git a/test/scenario/network_node.cc b/test/scenario/network_node.cc index 1b82b53090..a715093f64 100644 --- a/test/scenario/network_node.cc +++ b/test/scenario/network_node.cc @@ -27,20 +27,17 @@ SimulatedNetwork::Config CreateSimulationConfig(NetworkNodeConfig config) { } } // namespace -bool NullReceiver::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) { - return true; -} +void NullReceiver::DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) {} ActionReceiver::ActionReceiver(std::function action) : action_(action) {} -bool ActionReceiver::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) { +void ActionReceiver::DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) { action_(); - return true; } NetworkNode::~NetworkNode() = default; @@ -61,19 +58,17 @@ void NetworkNode::ClearRoute(uint64_t receiver_id) { routing_.erase(it); } -bool NetworkNode::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) { +void NetworkNode::DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) { rtc::CritScope crit(&crit_sect_); if (routing_.find(receiver) == routing_.end()) - return false; + return; uint64_t packet_id = next_packet_id_++; - bool sent = behavior_->EnqueuePacket(PacketInFlightInfo( - packet.size() + packet_overhead_, at_time.us(), packet_id)); - if (sent) { + if (behavior_->EnqueuePacket(PacketInFlightInfo( + packet.size() + packet_overhead_, at_time.us(), packet_id))) { packets_.emplace_back(StoredPacket{packet, receiver, packet_id, false}); } - return sent; } void NetworkNode::Process(Timestamp at_time) { @@ -104,8 +99,8 @@ void NetworkNode::Process(Timestamp at_time) { } // We don't want to keep the lock here. Otherwise we would get a deadlock if // the receiver tries to push a new packet. - receiver->TryDeliverPacket(packet->packet_data, packet->receiver_id, - Timestamp::us(delivery_info.receive_time_us)); + receiver->DeliverPacket(packet->packet_data, packet->receiver_id, + Timestamp::us(delivery_info.receive_time_us)); { rtc::CritScope crit(&crit_sect_); while (!packets_.empty() && packets_.front().removed) { @@ -196,7 +191,8 @@ bool NetworkNodeTransport::SendRtp(const uint8_t* packet, rtc::CopyOnWriteBuffer buffer(packet, length, length + packet_overhead_.bytes()); buffer.SetSize(length + packet_overhead_.bytes()); - return send_net_->TryDeliverPacket(buffer, receiver_id_, send_time); + send_net_->DeliverPacket(buffer, receiver_id_, send_time); + return true; } bool NetworkNodeTransport::SendRtcp(const uint8_t* packet, size_t length) { @@ -206,7 +202,8 @@ bool NetworkNodeTransport::SendRtcp(const uint8_t* packet, size_t length) { buffer.SetSize(length + packet_overhead_.bytes()); if (!send_net_) return false; - return send_net_->TryDeliverPacket(buffer, receiver_id_, send_time); + send_net_->DeliverPacket(buffer, receiver_id_, send_time); + return true; } void NetworkNodeTransport::Connect(NetworkNode* send_node, @@ -262,8 +259,8 @@ void CrossTrafficSource::Process(Timestamp at_time, TimeDelta delta) { } pending_size_ += TrafficRate() * delta; if (pending_size_ > config_.min_packet_size) { - target_->TryDeliverPacket(rtc::CopyOnWriteBuffer(pending_size_.bytes()), - receiver_id_, at_time); + target_->DeliverPacket(rtc::CopyOnWriteBuffer(pending_size_.bytes()), + receiver_id_, at_time); pending_size_ = DataSize::Zero(); } } diff --git a/test/scenario/network_node.h b/test/scenario/network_node.h index 4e90f6f252..e47d435e8b 100644 --- a/test/scenario/network_node.h +++ b/test/scenario/network_node.h @@ -30,24 +30,24 @@ namespace test { class NetworkReceiverInterface { public: - virtual bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) = 0; + virtual void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) = 0; virtual ~NetworkReceiverInterface() = default; }; class NullReceiver : public NetworkReceiverInterface { public: - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; }; class ActionReceiver : public NetworkReceiverInterface { public: explicit ActionReceiver(std::function action); virtual ~ActionReceiver() = default; - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; private: std::function action_; @@ -60,9 +60,9 @@ class NetworkNode : public NetworkReceiverInterface { ~NetworkNode() override; RTC_DISALLOW_COPY_AND_ASSIGN(NetworkNode); - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; // Creates a route for the given receiver_id over all the given nodes to the // given receiver. static void Route(int64_t receiver_id, diff --git a/test/scenario/scenario.cc b/test/scenario/scenario.cc index 161a8329ee..8deaead6a3 100644 --- a/test/scenario/scenario.cc +++ b/test/scenario/scenario.cc @@ -221,8 +221,8 @@ void Scenario::TriggerPacketBurst(std::vector over_nodes, int64_t route_id = next_route_id_++; NetworkNode::Route(route_id, over_nodes, &null_receiver_); for (size_t i = 0; i < num_packets; ++i) - over_nodes[0]->TryDeliverPacket(rtc::CopyOnWriteBuffer(packet_size), - route_id, Now()); + over_nodes[0]->DeliverPacket(rtc::CopyOnWriteBuffer(packet_size), route_id, + Now()); } void Scenario::NetworkDelayedAction(std::vector over_nodes, @@ -231,8 +231,8 @@ void Scenario::NetworkDelayedAction(std::vector over_nodes, int64_t route_id = next_route_id_++; action_receivers_.emplace_back(new ActionReceiver(action)); NetworkNode::Route(route_id, over_nodes, action_receivers_.back().get()); - over_nodes[0]->TryDeliverPacket(rtc::CopyOnWriteBuffer(packet_size), route_id, - Now()); + over_nodes[0]->DeliverPacket(rtc::CopyOnWriteBuffer(packet_size), route_id, + Now()); } CrossTrafficSource* Scenario::CreateCrossTraffic( diff --git a/test/scenario/simulated_time.cc b/test/scenario/simulated_time.cc index 90ea5188e8..ee5db6cd23 100644 --- a/test/scenario/simulated_time.cc +++ b/test/scenario/simulated_time.cc @@ -210,9 +210,9 @@ SimulatedFeedback::SimulatedFeedback(SimulatedTimeClientConfig config, // Polls receiver side for a feedback report and sends it to the stream sender // via return_node_, -bool SimulatedFeedback::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) { +void SimulatedFeedback::DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) { int64_t sequence_number; memcpy(&sequence_number, packet.cdata(), sizeof(sequence_number)); receive_times_.insert({sequence_number, at_time}); @@ -231,17 +231,16 @@ bool SimulatedFeedback::TryDeliverPacket(rtc::CopyOnWriteBuffer packet, } if (report.receive_times.size() >= RawFeedbackReportPacket::MAX_FEEDBACKS) { - return_node_->TryDeliverPacket(FeedbackToBuffer(report), - return_receiver_id_, at_time); + return_node_->DeliverPacket(FeedbackToBuffer(report), + return_receiver_id_, at_time); report = SimpleFeedbackReportPacket(); } } if (!report.receive_times.empty()) - return_node_->TryDeliverPacket(FeedbackToBuffer(report), - return_receiver_id_, at_time); + return_node_->DeliverPacket(FeedbackToBuffer(report), return_receiver_id_, + at_time); last_feedback_time_ = at_time; } - return true; } SimulatedTimeClient::SimulatedTimeClient( @@ -284,9 +283,9 @@ SimulatedTimeClient::SimulatedTimeClient( // Pulls feedback reports from sender side based on the recieved feedback // packet. Updates congestion controller with the resulting report. -bool SimulatedTimeClient::TryDeliverPacket(rtc::CopyOnWriteBuffer raw_buffer, - uint64_t receiver, - Timestamp at_time) { +void SimulatedTimeClient::DeliverPacket(rtc::CopyOnWriteBuffer raw_buffer, + uint64_t receiver, + Timestamp at_time) { auto report = sender_.PullFeedbackReport(FeedbackFromBuffer(raw_buffer), at_time); for (PacketResult& feedback : report.packet_feedbacks) { @@ -299,7 +298,6 @@ bool SimulatedTimeClient::TryDeliverPacket(rtc::CopyOnWriteBuffer raw_buffer, at_time.seconds()); } Update(congestion_controller_->OnTransportPacketsFeedback(report)); - return true; } SimulatedTimeClient::~SimulatedTimeClient() { if (packet_log_) @@ -331,8 +329,8 @@ void SimulatedTimeClient::CongestionProcess(Timestamp at_time) { void SimulatedTimeClient::PacerProcess(Timestamp at_time) { ProcessFrames(at_time); for (auto to_send : sender_.PaceAndPullSendPackets(at_time)) { - sender_.send_node_->TryDeliverPacket(to_send.data, - sender_.send_receiver_id_, at_time); + sender_.send_node_->DeliverPacket(to_send.data, sender_.send_receiver_id_, + at_time); Update(congestion_controller_->OnSentPacket(to_send.send_info)); } } diff --git a/test/scenario/simulated_time.h b/test/scenario/simulated_time.h index da5e972c80..3e67aa50f4 100644 --- a/test/scenario/simulated_time.h +++ b/test/scenario/simulated_time.h @@ -52,9 +52,9 @@ class SimulatedFeedback : NetworkReceiverInterface { SimulatedFeedback(SimulatedTimeClientConfig config, uint64_t return_receiver_id, NetworkNode* return_node); - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; private: friend class SimulatedTimeClient; @@ -141,9 +141,9 @@ class SimulatedTimeClient : NetworkReceiverInterface { DataRate link_capacity() const; DataRate padding_rate() const; - bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, - uint64_t receiver, - Timestamp at_time) override; + void DeliverPacket(rtc::CopyOnWriteBuffer packet, + uint64_t receiver, + Timestamp at_time) override; private: friend class Scenario;