From f662d1f7cd9d00b9d54ab978f10e72ddc225212b Mon Sep 17 00:00:00 2001 From: Phil Taylor Date: Thu, 5 May 2022 08:57:50 +0100 Subject: [PATCH 1/2] Found another retransmit bug in server --- udphandler.cpp | 13 +++++++--- udpserver.cpp | 66 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/udphandler.cpp b/udphandler.cpp index 65ce54d..405a04f 100644 --- a/udphandler.cpp +++ b/udphandler.cpp @@ -1217,7 +1217,10 @@ void udpBase::dataReceived(QByteArray r) udpMutex.unlock(); } else { - qDebug(logUdp()) << this->metaObject()->className() << ": Remote requested packet " << QString("0x%1").arg(in->seq, 0, 16) << " not found, have " << txSeqBuf.firstKey() << "to" << txSeqBuf.lastKey(); + qDebug(logUdp()) << this->metaObject()->className() << ": Remote requested packet" + << QString("0x%1").arg(in->seq, 0, 16) << + "not found, have " << QString("0x%1").arg(txSeqBuf.firstKey(), 0, 16) << + "to" << QString("0x%1").arg(txSeqBuf.lastKey(), 0, 16); } txBufferMutex.unlock(); } @@ -1298,7 +1301,10 @@ void udpBase::dataReceived(QByteArray r) quint16 seq = (quint8)r[i] | (quint8)r[i + 1] << 8; auto match = txSeqBuf.find(seq); if (match == txSeqBuf.end()) { - qDebug(logUdp()) << this->metaObject()->className() << ": Remote requested packet " << QString("0x%1").arg(seq, 0, 16) << " not found, have " << txSeqBuf.firstKey() << "to" << txSeqBuf.lastKey(); + qDebug(logUdp()) << this->metaObject()->className() << ": Remote requested packet" + << QString("0x%1").arg(seq, 0, 16) << + "not found, have " << QString("0x%1").arg(txSeqBuf.firstKey(), 0, 16) << + "to" << QString("0x%1").arg(txSeqBuf.lastKey(), 0, 16); // Just send idle packet. sendControl(false, 0, seq); } @@ -1538,11 +1544,12 @@ void udpBase::sendTrackedPacket(QByteArray d) txSeqBuf.clear(); congestion = 0; } - txSeqBuf.insert(sendSeq,s); if (txSeqBuf.size() > BUFSIZE) { txSeqBuf.erase(txSeqBuf.begin()); } + txSeqBuf.insert(sendSeq, s); + txBufferMutex.unlock(); } else { qInfo(logUdp()) << this->metaObject()->className() << ": txBuffer mutex is locked"; diff --git a/udpserver.cpp b/udpserver.cpp index 2158451..14df13d 100644 --- a/udpserver.cpp +++ b/udpserver.cpp @@ -818,7 +818,10 @@ void udpServer::commonReceived(QList* l, CLIENT* current, QByteArray r) } else { // Just send an idle! - qInfo(logUdpServer()) << current->ipAddress.toString() << "(" << current->type << "): Requested (single) packet " << QString("0x%1").arg(in->seq, 0, 16) << "not found, have " << current->txSeqBuf.firstKey() << "to" << current->txSeqBuf.lastKey(); + qInfo(logUdpServer()) << current->ipAddress.toString() << "(" << current->type << + "): Requested (single) packet " << QString("0x%1").arg(in->seq, 0, 16) << + "not found, have " << QString("0x%1").arg(current->txSeqBuf.firstKey(), 0, 16) << + "to" << QString("0x%1").arg(current->txSeqBuf.lastKey(), 0, 16); sendControl(current, 0x00, in->seq); } } @@ -840,7 +843,11 @@ void udpServer::commonReceived(QList* l, CLIENT* current, QByteArray r) quint16 seq = (quint8)r[i] | (quint8)r[i + 1] << 8; auto match = current->txSeqBuf.find(seq); if (match == current->txSeqBuf.end()) { - qInfo(logUdpServer()) << current->ipAddress.toString() << "(" << current->type << "): Requested (multiple) packet " << QString("0x%1").arg(seq,0,16) << " not found , have " << current->txSeqBuf.firstKey() << "to" << current->txSeqBuf.lastKey(); + qInfo(logUdpServer()) << current->ipAddress.toString() << "(" << current->type << + "): Requested (multiple) packet " << QString("0x%1").arg(seq,0,16) << + "not found, have " << QString("0x%1").arg(current->txSeqBuf.firstKey(), 0, 16) << + "to" << QString("0x%1").arg(current->txSeqBuf.lastKey(), 0, 16); + // Just send idle packet. sendControl(current, 0, in->seq); } @@ -998,12 +1005,16 @@ void udpServer::sendControl(CLIENT* c, quint8 type, quint16 seq) s.data = QByteArray::fromRawData((const char*)p.packet, sizeof(p)); if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else + if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } - c->txSeqBuf.insert(seq, s); + c->txSeqBuf.insert(c->txSeq, s); c->txSeq++; c->txMutex.unlock(); } @@ -1131,7 +1142,10 @@ void udpServer::sendLoginResponse(CLIENT* c, bool allowed) s.data = QByteArray::fromRawData((const char*)p.packet, sizeof(p)); if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } @@ -1253,11 +1267,14 @@ void udpServer::sendCapabilities(CLIENT* c) if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } - c->txSeqBuf.insert(p.seq, s); + c->txSeqBuf.insert(c->txSeq, s); c->txSeq++; c->txMutex.unlock(); } @@ -1335,11 +1352,14 @@ void udpServer::sendConnectionInfo(CLIENT* c, quint8 guid[GUIDLEN]) if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } - c->txSeqBuf.insert(p.seq, s); + c->txSeqBuf.insert(c->txSeq, s); c->txSeq++; c->txMutex.unlock(); } @@ -1393,11 +1413,14 @@ void udpServer::sendTokenResponse(CLIENT* c, quint8 type) if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } - c->txSeqBuf.insert(p.seq, s); + c->txSeqBuf.insert(c->txSeq, s); c->txSeq++; c->txMutex.unlock(); } @@ -1509,12 +1532,15 @@ void udpServer::sendStatus(CLIENT* c) s.data = QByteArray::fromRawData((const char*)p.packet, sizeof(p)); if (c->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (c->txSeqBuf.size() > BUFSIZE) + if (c->txSeq == 0) { + c->txSeqBuf.clear(); + } + else if (c->txSeqBuf.size() > BUFSIZE) { c->txSeqBuf.remove(c->txSeqBuf.firstKey()); } c->txSeq++; - c->txSeqBuf.insert(p.seq, s); + c->txSeqBuf.insert(c->txSeq, s); c->txMutex.unlock(); } else { @@ -1581,11 +1607,14 @@ void udpServer::dataForServer(QByteArray d) if (client->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (client->txSeqBuf.size() > BUFSIZE) + if (client->txSeq == 0) { + client->txSeqBuf.clear(); + } + else if (client->txSeqBuf.size() > BUFSIZE) { client->txSeqBuf.remove(client->txSeqBuf.firstKey()); } - client->txSeqBuf.insert(p.seq, s); + client->txSeqBuf.insert(client->txSeq, s); client->txSeq++; //client->innerSeq = (qToBigEndian(qFromBigEndian(client->innerSeq) + 1)); client->txMutex.unlock(); @@ -1652,11 +1681,14 @@ void udpServer::receiveAudioData(const audioPacket& d) s.data = t; if (client->txMutex.try_lock_for(std::chrono::milliseconds(LOCK_PERIOD))) { - if (client->txSeqBuf.size() > BUFSIZE) + if (client->txSeq == 0) { + client->txSeqBuf.clear(); + } + else if (client->txSeqBuf.size() > BUFSIZE) { client->txSeqBuf.remove(client->txSeqBuf.firstKey()); } - client->txSeqBuf.insert(p.seq, s); + client->txSeqBuf.insert(client->txSeq, s); client->txSeq++; client->sendAudioSeq++; client->txMutex.unlock(); From 7924d2b87f5908365222cf734c5738cc334bb90a Mon Sep 17 00:00:00 2001 From: Phil Taylor Date: Thu, 5 May 2022 09:01:04 +0100 Subject: [PATCH 2/2] Fix retransmit missing message --- udphandler.cpp | 2 +- udpserver.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/udphandler.cpp b/udphandler.cpp index 405a04f..f19e6fd 100644 --- a/udphandler.cpp +++ b/udphandler.cpp @@ -1440,7 +1440,7 @@ void udpBase::sendRetransmitRequest() it++; } else { - qInfo(logUdp()) << this->metaObject()->className() << ": No response for missing packet" << it.key() << "deleting"; + qInfo(logUdp()) << this->metaObject()->className() << ": No response for missing packet" << QString("0x%1").arg(it.key(), 0, 16) << "deleting"; it = rxMissing.erase(it); } } else { diff --git a/udpserver.cpp b/udpserver.cpp index 14df13d..9c8db8c 100644 --- a/udpserver.cpp +++ b/udpserver.cpp @@ -1762,7 +1762,7 @@ void udpServer::sendRetransmitRequest(CLIENT* c) else { // We have tried 4 times to request this packet, time to give up! - qInfo(logUdp()) << this->metaObject()->className() << ": No response for missing packet" << it.key() << "deleting"; + qInfo(logUdp()) << this->metaObject()->className() << ": No response for missing packet" << QString("0x%1").arg(it.key(), 0, 16) << "deleting"; it = c->rxMissing.erase(it); } } else {