From 8d656ed36075a25366ef5239722714883b2699e6 Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Thu, 27 Jan 2022 13:08:40 +0000 Subject: [PATCH] Hoist block offset multiplication to top of loops This is more efficient, and should avoid the CodeQL warnings about multiplication overflow within array indices or vector offsets in buffer.cpp and server.cpp Replaces the changes that were done in #2292 --- src/buffer.cpp | 22 +++++++++++----------- src/server.cpp | 12 ++++++++---- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/buffer.cpp b/src/buffer.cpp index 4b29382318..6ed910623e 100644 --- a/src/buffer.cpp +++ b/src/buffer.cpp @@ -146,12 +146,14 @@ bool CNetBuf::Put ( const CVector& vecbyData, int iInSize ) const int iNumBlocks = /* floor */ ( iInSize / iBlockSize ); // copy new data in internal buffer - // iBlock is a long to avoid overflowing a mulitplication later in the code - for ( long iBlock = 0; iBlock < iNumBlocks; iBlock++ ) + for ( int iBlock = 0; iBlock < iNumBlocks; iBlock++ ) { + // calculate the block offset once per loop instead of repeated multiplying + const int iBlockOffset = iBlock * ( iBlockSize + iNumBytesSeqNum ); + // extract sequence number of current received block (per definition // the sequence number is appended after the coded audio data) - const int iCurrentSequenceNumber = vecbyData[iBlock * ( iBlockSize + iNumBytesSeqNum ) + iBlockSize]; + const int iCurrentSequenceNumber = vecbyData[iBlockOffset + iBlockSize]; // calculate the sequence number difference and take care of wrap int iSeqNumDiff = iCurrentSequenceNumber - static_cast ( iSequenceNumberAtGetPos ); @@ -243,9 +245,7 @@ bool CNetBuf::Put ( const CVector& vecbyData, int iInSize ) if ( !bIsSimulation ) { // copy one block of data in buffer - std::copy ( vecbyData.begin() + iBlock * ( iBlockSize + iNumBytesSeqNum ), - vecbyData.begin() + iBlock * ( iBlockSize + iNumBytesSeqNum ) + iBlockSize, - vecvecMemory[iBlockPutPos].begin() ); + std::copy ( vecbyData.begin() + iBlockOffset, vecbyData.begin() + iBlockOffset + iBlockSize, vecvecMemory[iBlockPutPos].begin() ); } // valid packet added, set flag @@ -264,16 +264,16 @@ bool CNetBuf::Put ( const CVector& vecbyData, int iInSize ) // copy new data in internal buffer const int iNumBlocks = iInSize / iBlockSize; - // iBlock is a long to avoid overflowing a mulitplication later in the code - for ( long iBlock = 0; iBlock < iNumBlocks; iBlock++ ) + for ( int iBlock = 0; iBlock < iNumBlocks; iBlock++ ) { // for simultion buffer only update pointer, no data copying if ( !bIsSimulation ) { + // calculate the block offset once per loop instead of repeated multiplying + const int iBlockOffset = iBlock * iBlockSize; + // copy one block of data in buffer - std::copy ( vecbyData.begin() + iBlock * iBlockSize, - vecbyData.begin() + iBlock * iBlockSize + iBlockSize, - vecvecMemory[iBlockPutPos].begin() ); + std::copy ( vecbyData.begin() + iBlockOffset, vecbyData.begin() + iBlockOffset + iBlockSize, vecvecMemory[iBlockPutPos].begin() ); } // set the put position one block further diff --git a/src/server.cpp b/src/server.cpp index 551689e5d5..a979a37d2a 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1041,7 +1041,7 @@ void CServer::DecodeReceiveData ( const int iChanCnt, const int iNumClients ) // get current number of OPUS coded bytes const int iCeltNumCodedBytes = vecChannels[iCurChanID].GetCeltNumCodedBytes(); - for ( size_t iB = 0; iB < (size_t) vecNumFrameSizeConvBlocks[iChanCnt]; iB++ ) + for ( int iB = 0; iB < vecNumFrameSizeConvBlocks[iChanCnt]; iB++ ) { // get data const EGetDataStat eGetStat = vecChannels[iCurChanID].GetData ( vecvecbyCodedData[iChanCnt], iCeltNumCodedBytes ); @@ -1081,10 +1081,12 @@ void CServer::DecodeReceiveData ( const int iChanCnt, const int iNumClients ) // OPUS decode received data stream if ( CurOpusDecoder != nullptr ) { + const int iOffset = iB * SYSTEM_FRAME_SIZE_SAMPLES * vecNumAudioChannels[iChanCnt]; + iUnused = opus_custom_decode ( CurOpusDecoder, pCurCodedData, iCeltNumCodedBytes, - &vecvecsData[iChanCnt][iB * SYSTEM_FRAME_SIZE_SAMPLES * vecNumAudioChannels[iChanCnt]], + &vecvecsData[iChanCnt][iOffset], iClientFrameSizeSamples ); } } @@ -1354,10 +1356,12 @@ void CServer::MixEncodeTransmitData ( const int iChanCnt, const int iNumClients opus_custom_encoder_ctl ( pCurOpusEncoder, OPUS_SET_BITRATE ( CalcBitRateBitsPerSecFromCodedBytes ( iCeltNumCodedBytes, iClientFrameSizeSamples ) ) ); // clang-format on - for ( size_t iB = 0; iB < (size_t) vecNumFrameSizeConvBlocks[iChanCnt]; iB++ ) + for ( int iB = 0; iB < vecNumFrameSizeConvBlocks[iChanCnt]; iB++ ) { + const int iOffset = iB * SYSTEM_FRAME_SIZE_SAMPLES * vecNumAudioChannels[iChanCnt]; + iUnused = opus_custom_encode ( pCurOpusEncoder, - &vecsSendData[iB * SYSTEM_FRAME_SIZE_SAMPLES * vecNumAudioChannels[iChanCnt]], + &vecsSendData[iOffset], iClientFrameSizeSamples, &vecvecbyCodedData[iChanCnt][0], iCeltNumCodedBytes );