diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index 8e0f755cd6..8d81ccbe69 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -44,11 +44,6 @@ class ReceiveSideCongestionController : public CallStatsObserver { void OnReceivedPacket(const RtpPacketReceived& packet, MediaType media_type); - // TODO(perkj, bugs.webrtc.org/14859): Remove all usage. This method is - // currently not used by PeerConnections. - virtual void OnReceivedPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header); // Implements CallStatsObserver. void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index cc314699f7..d4c745b546 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -106,19 +106,6 @@ void ReceiveSideCongestionController::OnReceivedPacket( } } -void ReceiveSideCongestionController::OnReceivedPacket( - int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) { - remote_estimator_proxy_.IncomingPacket(arrival_time_ms, payload_size, header); - if (!header.extension.hasTransportSequenceNumber) { - // Receive-side BWE. - MutexLock lock(&mutex_); - PickEstimator(header.extension.hasAbsoluteSendTime); - rbe_->IncomingPacket(arrival_time_ms, payload_size, header); - } -} - void ReceiveSideCongestionController::OnBitrateChanged(int bitrate_bps) { remote_estimator_proxy_.OnBitrateChanged(bitrate_bps); } diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 00eac49a6f..c417f35fb6 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -28,9 +28,7 @@ namespace webrtc { namespace { -// The maximum allowed value for a timestamp in milliseconds. This is lower -// than the numerical limit since we often convert to microseconds. -constexpr int64_t kMaxTimeMs = std::numeric_limits::max() / 1000; + constexpr TimeDelta kBackWindow = TimeDelta::Millis(500); constexpr TimeDelta kMinInterval = TimeDelta::Millis(50); constexpr TimeDelta kMaxInterval = TimeDelta::Millis(250); @@ -89,15 +87,11 @@ void RemoteEstimatorProxy::IncomingPacket(const RtpPacketReceived& packet) { return; } - Packet internal_packet = {.arrival_time = packet.arrival_time(), - .size = DataSize::Bytes(packet.size()), - .ssrc = packet.Ssrc()}; - uint16_t seqnum; - if (packet.GetExtension(&seqnum) || - packet.GetExtension( - &seqnum, &internal_packet.feedback_request)) { - internal_packet.transport_sequence_number = seqnum; - } else { + uint16_t seqnum = 0; + absl::optional feedback_request; + if (!packet.GetExtension(&seqnum) && + !packet.GetExtension(&seqnum, + &feedback_request)) { // This function expected to be called only for packets that have // TransportSequenceNumber rtp header extension, however malformed RTP // packet may contain unparsable TransportSequenceNumber. @@ -107,81 +101,49 @@ void RemoteEstimatorProxy::IncomingPacket(const RtpPacketReceived& packet) { return; } - internal_packet.absolute_send_time_24bits = - packet.GetExtension(); - MutexLock lock(&lock_); send_periodic_feedback_ = packet.HasExtension(); - IncomingPacket(internal_packet); -} + media_ssrc_ = packet.Ssrc(); + int64_t seq = unwrapper_.Unwrap(seqnum); -void RemoteEstimatorProxy::IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) { - if (arrival_time_ms < 0 || arrival_time_ms >= kMaxTimeMs) { - RTC_LOG(LS_WARNING) << "Arrival time out of bounds: " << arrival_time_ms; + if (send_periodic_feedback_) { + MaybeCullOldPackets(seq, packet.arrival_time()); + + if (!periodic_window_start_seq_ || seq < *periodic_window_start_seq_) { + periodic_window_start_seq_ = seq; + } + } + + // We are only interested in the first time a packet is received. + if (packet_arrival_times_.has_received(seq)) { return; } - Packet packet = {.arrival_time = Timestamp::Millis(arrival_time_ms), - .size = DataSize::Bytes(header.headerLength + payload_size), - .ssrc = header.ssrc}; - if (header.extension.hasTransportSequenceNumber) { - packet.transport_sequence_number = header.extension.transportSequenceNumber; - } - if (header.extension.hasAbsoluteSendTime) { - packet.absolute_send_time_24bits = header.extension.absoluteSendTime; - } - packet.feedback_request = header.extension.feedback_request; - MutexLock lock(&lock_); - IncomingPacket(packet); -} + packet_arrival_times_.AddPacket(seq, packet.arrival_time()); -void RemoteEstimatorProxy::IncomingPacket(Packet packet) { - media_ssrc_ = packet.ssrc; - int64_t seq = 0; - - if (packet.transport_sequence_number.has_value()) { - seq = unwrapper_.Unwrap(*packet.transport_sequence_number); - - if (send_periodic_feedback_) { - MaybeCullOldPackets(seq, packet.arrival_time); - - if (!periodic_window_start_seq_ || seq < *periodic_window_start_seq_) { - periodic_window_start_seq_ = seq; - } - } - - // We are only interested in the first time a packet is received. - if (packet_arrival_times_.has_received(seq)) { - return; - } - - packet_arrival_times_.AddPacket(seq, packet.arrival_time); - - // Limit the range of sequence numbers to send feedback for. - if (!periodic_window_start_seq_.has_value() || - periodic_window_start_seq_.value() < - packet_arrival_times_.begin_sequence_number()) { - periodic_window_start_seq_ = - packet_arrival_times_.begin_sequence_number(); - } - - if (packet.feedback_request) { - // Send feedback packet immediately. - SendFeedbackOnRequest(seq, *packet.feedback_request); - } + // Limit the range of sequence numbers to send feedback for. + if (periodic_window_start_seq_ < + packet_arrival_times_.begin_sequence_number()) { + periodic_window_start_seq_ = packet_arrival_times_.begin_sequence_number(); } - if (network_state_estimator_ && packet.absolute_send_time_24bits) { + if (feedback_request.has_value()) { + // Send feedback packet immediately. + SendFeedbackOnRequest(seq, *feedback_request); + } + + absl::optional absolute_send_time_24bits = + packet.GetExtension(); + if (network_state_estimator_ && absolute_send_time_24bits.has_value()) { PacketResult packet_result; - packet_result.receive_time = packet.arrival_time; - abs_send_timestamp_ += GetAbsoluteSendTimeDelta( - *packet.absolute_send_time_24bits, previous_abs_send_time_); - previous_abs_send_time_ = *packet.absolute_send_time_24bits; + packet_result.receive_time = packet.arrival_time(); + abs_send_timestamp_ += GetAbsoluteSendTimeDelta(*absolute_send_time_24bits, + previous_abs_send_time_); + previous_abs_send_time_ = *absolute_send_time_24bits; packet_result.sent_packet.send_time = abs_send_timestamp_; - packet_result.sent_packet.size = packet.size + packet_overhead_; + packet_result.sent_packet.size = + DataSize::Bytes(packet.size()) + packet_overhead_; packet_result.sent_packet.sequence_number = seq; network_state_estimator_->OnReceivedPacket(packet_result); } diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index a28e85c1a7..561410b0e0 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -47,12 +47,6 @@ class RemoteEstimatorProxy { void IncomingPacket(const RtpPacketReceived& packet); - // TODO(perkj, bugs.webrtc.org/14859): Remove all usage. This method is - // currently not used by PeerConnections. - void IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header); - // Sends periodic feedback if it is time to send it. // Returns time until next call to Process should be made. TimeDelta Process(Timestamp now); @@ -61,16 +55,6 @@ class RemoteEstimatorProxy { void SetTransportOverhead(DataSize overhead_per_packet); private: - struct Packet { - Timestamp arrival_time; - DataSize size; - uint32_t ssrc; - absl::optional absolute_send_time_24bits; - absl::optional transport_sequence_number; - absl::optional feedback_request; - }; - void IncomingPacket(Packet packet) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); - void MaybeCullOldPackets(int64_t sequence_number, Timestamp arrival_time) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); void SendPeriodicFeedbacks() RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_);