diff --git a/srtcore/core.cpp b/srtcore/core.cpp index c099fe671..a661b20e0 100644 --- a/srtcore/core.cpp +++ b/srtcore/core.cpp @@ -9006,7 +9006,7 @@ void srt::CUDT::updateAfterSrtHandshake(int hsv) } } -int srt::CUDT::packLostData(CPacket& w_packet, steady_clock::time_point& w_origintime) +int srt::CUDT::packLostData(CPacket& w_packet) { // protect m_iSndLastDataAck from updating by ACK processing UniqueLock ackguard(m_RecvAckLock); @@ -9061,8 +9061,8 @@ int srt::CUDT::packLostData(CPacket& w_packet, steady_clock::time_point& w_origi } int msglen; - - const int payload = m_pSndBuffer->readData(offset, (w_packet), (w_origintime), (msglen)); + steady_clock::time_point tsOrigin; + const int payload = m_pSndBuffer->readData(offset, (w_packet), (tsOrigin), (msglen)); if (payload == -1) { int32_t seqpair[2]; @@ -9101,6 +9101,7 @@ int srt::CUDT::packLostData(CPacket& w_packet, steady_clock::time_point& w_origi { w_packet.m_iMsgNo |= PACKET_SND_REXMIT; } + setDataPacketTS(w_packet, tsOrigin); return payload; } @@ -9194,6 +9195,42 @@ class snd_logger snd_logger g_snd_logger; #endif // SRT_DEBUG_TRACE_SND +void srt::CUDT::setPacketTS(CPacket& p, const time_point& ts) +{ + enterCS(m_StatsLock); + const time_point tsStart = m_stats.tsStartTime; + leaveCS(m_StatsLock); + p.m_iTimeStamp = makeTS(ts, tsStart); +} + +void srt::CUDT::setDataPacketTS(CPacket& p, const time_point& ts) +{ + enterCS(m_StatsLock); + const time_point tsStart = m_stats.tsStartTime; + leaveCS(m_StatsLock); + + if (!m_bPeerTsbPd) + { + // If TSBPD is disabled, use the current time as the source (timestamp using the sending time). + p.m_iTimeStamp = makeTS(steady_clock::now(), tsStart); + return; + } + + // TODO: Might be better for performance to ensure this condition is always false, and just use SRT_ASSERT here. + if (ts < tsStart) + { + p.m_iTimeStamp = makeTS(steady_clock::now(), tsStart); + LOGC(qslog.Warn, + log << CONID() << "setPacketTS: reference time=" << FormatTime(ts) + << " is in the past towards start time=" << FormatTime(tsStart) + << " - setting NOW as reference time for the data packet"); + return; + } + + // Use the provided source time for the timestamp. + p.m_iTimeStamp = makeTS(ts, tsStart); +} + bool srt::CUDT::isRetransmissionAllowed(const time_point& tnow SRT_ATR_UNUSED) { // Prioritization of original packets only applies to Live CC. @@ -9237,9 +9274,7 @@ std::pair srt::CUDT::packData(CPacket& w_packet) { int payload = 0; bool probe = false; - steady_clock::time_point origintime; bool new_packet_packed = false; - bool filter_ctl_pkt = false; const steady_clock::time_point enter_time = steady_clock::now(); @@ -9248,8 +9283,6 @@ std::pair srt::CUDT::packData(CPacket& w_packet) m_tdSendTimeDiff = m_tdSendTimeDiff.load() + (enter_time - m_tsNextSendTime); } - string reason = "reXmit"; - ScopedLock connectguard(m_ConnectionLock); // If a closing action is done simultaneously, then // m_bOpened should already be false, and it's set @@ -9262,20 +9295,20 @@ std::pair srt::CUDT::packData(CPacket& w_packet) return std::make_pair(false, enter_time); payload = isRetransmissionAllowed(enter_time) - ? packLostData((w_packet), (origintime)) + ? packLostData((w_packet)) : 0; + IF_HEAVY_LOGGING(const char* reason); // The source of the data packet (normal/rexmit/filter) if (payload > 0) { - reason = "reXmit"; + IF_HEAVY_LOGGING(reason = "reXmit"); } else if (m_PacketFilter && m_PacketFilter.packControlPacket(m_iSndCurrSeqNo, m_pCryptoControl->getSndCryptoFlags(), (w_packet))) { HLOGC(qslog.Debug, log << CONID() << "filter: filter/CTL packet ready - packing instead of data."); payload = (int) w_packet.getLength(); - reason = "filter"; - filter_ctl_pkt = true; // Mark that this packet ALREADY HAS timestamp field and it should not be set + IF_HEAVY_LOGGING(reason = "filter"); // Stats ScopedLock lg(m_StatsLock); @@ -9283,7 +9316,7 @@ std::pair srt::CUDT::packData(CPacket& w_packet) } else { - if (!packUniqueData(w_packet, origintime)) + if (!packUniqueData(w_packet)) { m_tsNextSendTime = steady_clock::time_point(); m_tdSendTimeDiff = steady_clock::duration(); @@ -9296,46 +9329,10 @@ std::pair srt::CUDT::packData(CPacket& w_packet) probe = true; payload = (int) w_packet.getLength(); - reason = "normal"; - } - - // Normally packet.m_iTimeStamp field is set exactly here, - // usually as taken from m_stats.tsStartTime and current time, unless live - // mode in which case it is based on 'origintime' as set during scheduling. - // In case when this is a filter control packet, the m_iTimeStamp field already - // contains the exactly needed value, and it's a timestamp clip, not a real - // timestamp. - if (!filter_ctl_pkt) - { - if (m_bPeerTsbPd) - { - /* - * When timestamp is carried over in this sending stream from a received stream, - * it may be older than the session start time causing a negative packet time - * that may block the receiver's Timestamp-based Packet Delivery. - * XXX Isn't it then better to not decrease it by m_stats.tsStartTime? As long as it - * doesn't screw up the start time on the other side. - */ - if (origintime >= m_stats.tsStartTime) - { - setPacketTS(w_packet, origintime); - } - else - { - setPacketTS(w_packet, steady_clock::now()); - LOGC(qslog.Warn, - log << CONID() << "packData: reference time=" << FormatTime(origintime) - << " is in the past towards start time=" << FormatTime(m_stats.tsStartTime) - << " - setting NOW as reference time for the data packet"); - } - } - else - { - setPacketTS(w_packet, steady_clock::now()); - } + IF_HEAVY_LOGGING(reason = "normal"); } - w_packet.m_iID = m_PeerID; + w_packet.m_iID = m_PeerID; // Set the destination SRT socket ID. if (new_packet_packed && m_PacketFilter) { @@ -9408,7 +9405,7 @@ std::pair srt::CUDT::packData(CPacket& w_packet) return std::make_pair(payload >= 0, m_tsNextSendTime); } -bool srt::CUDT::packUniqueData(CPacket& w_packet, time_point& w_origintime) +bool srt::CUDT::packUniqueData(CPacket& w_packet) { // Check the congestion/flow window limit const int cwnd = std::min(int(m_iFlowWindowSize), int(m_dCongestionWindow)); @@ -9428,7 +9425,8 @@ bool srt::CUDT::packUniqueData(CPacket& w_packet, time_point& w_origintime) // isn't a useless redundant state copy. If it is, then taking the flags here can be removed. const int kflg = m_pCryptoControl->getSndCryptoFlags(); int pktskipseqno = 0; - const int pld_size = m_pSndBuffer->readData((w_packet), (w_origintime), kflg, (pktskipseqno)); + time_point tsOrigin; + const int pld_size = m_pSndBuffer->readData((w_packet), (tsOrigin), kflg, (pktskipseqno)); if (pktskipseqno) { // Some packets were skipped due to TTL expiry. @@ -9520,7 +9518,10 @@ bool srt::CUDT::packUniqueData(CPacket& w_packet, time_point& w_origintime) w_packet.m_iSeqNo = m_iSndCurrSeqNo; } - // Encrypt if 1st time this packet is sent and crypto is enabled + // Set missing fields before encrypting the packet, because those fields might be used for encryption. + w_packet.m_iID = m_PeerID; // Destination SRT Socket ID + setDataPacketTS(w_packet, tsOrigin); + if (kflg != EK_NOENC) { // Note that the packet header must have a valid seqno set, as it is used as a counter for encryption. diff --git a/srtcore/core.h b/srtcore/core.h index ca8e90b31..c6d7cbaf2 100644 --- a/srtcore/core.h +++ b/srtcore/core.h @@ -376,22 +376,29 @@ class CUDT return m_config.bMessageAPI ? (len+ps-1)/ps : 1; } - int32_t makeTS(const time_point& from_time) const + static int32_t makeTS(const time_point& from_time, const time_point& tsStartTime) { // NOTE: // - This calculates first the time difference towards start time. // - This difference value is also CUT OFF THE SEGMENT information // (a multiple of MAX_TIMESTAMP+1) // So, this can be simply defined as: TS = (RTS - STS) % (MAX_TIMESTAMP+1) - // XXX Would be nice to check if local_time > m_tsStartTime, - // otherwise it may go unnoticed with clock skew. - return (int32_t) sync::count_microseconds(from_time - m_stats.tsStartTime); + SRT_ASSERT(from_time >= tsStartTime); + return (int32_t) sync::count_microseconds(from_time - tsStartTime); } - void setPacketTS(CPacket& p, const time_point& local_time) - { - p.m_iTimeStamp = makeTS(local_time); - } + /// @brief Set the timestamp field of the packet using the provided value (no check) + /// @param p the packet structure to set the timestamp on. + /// @param ts timestamp to use as a source for packet timestamp. + SRT_ATTR_EXCLUDES(m_StatsLock) + void setPacketTS(CPacket& p, const time_point& ts); + + /// @brief Set the timestamp field of the packet according the TSBPD mode. + /// Also checks the connection start time (m_tsStartTime). + /// @param p the packet structure to set the timestamp on. + /// @param ts timestamp to use as a source for packet timestamp. Ignored if m_bPeerTsbPd is false. + SRT_ATTR_EXCLUDES(m_StatsLock) + void setDataPacketTS(CPacket& p, const time_point& ts); // Utility used for closing a listening socket // immediately to free the socket @@ -437,16 +444,13 @@ class CUDT private: /// initialize a UDT entity and bind to a local address. - void open(); /// Start listening to any connection request. - void setListenState(); /// Connect to a UDT entity listening at address "peer". /// @param peer [in] The address of the listening UDT entity. - void startConnect(const sockaddr_any& peer, int32_t forced_isn); /// Process the response handshake packet. Failure reasons can be: @@ -457,7 +461,6 @@ class CUDT /// @retval 0 Connection successful /// @retval 1 Connection in progress (m_ConnReq turned into RESPONSE) /// @retval -1 Connection failed - SRT_ATR_NODISCARD SRT_ATTR_REQUIRES(m_ConnectionLock) EConnectStatus processConnectResponse(const CPacket& pkt, CUDTException* eout) ATR_NOEXCEPT; @@ -1029,20 +1032,15 @@ class CUDT void updateSndLossListOnACK(int32_t ackdata_seqno); /// Pack a packet from a list of lost packets. - /// /// @param packet [in, out] a packet structure to fill - /// @param origintime [in, out] origin timestamp of the packet - /// /// @return payload size on success, <=0 on failure - int packLostData(CPacket &packet, time_point &origintime); + int packLostData(CPacket &packet); /// Pack a unique data packet (never sent so far) in CPacket for sending. - /// /// @param packet [in, out] a CPacket structure to fill. - /// @param origintime [in, out] origin timestamp of the packet. /// /// @return true if a packet has been packets; false otherwise. - bool packUniqueData(CPacket& packet, time_point& origintime); + bool packUniqueData(CPacket& packet); /// Pack in CPacket the next data to be send. ///