diff --git a/src/engine/agent/Agent_Impl.cpp b/src/engine/agent/Agent_Impl.cpp index 78623452..e1e1a0c4 100644 --- a/src/engine/agent/Agent_Impl.cpp +++ b/src/engine/agent/Agent_Impl.cpp @@ -181,9 +181,6 @@ void AgentImpl::processConfig(JsonCpp::Value &d, JsonCpp::Value &answer) config()[CONFIG_IPV4] = d["ipv4"].asBool(); config()[CONFIG_IPV6] = d["ipv6"].asBool(); - if (transport == "tls") - config()[CONFIG_SIPS] = true; - // Log file std::string logfile = d["logfile"].asString(); ice::Logger& logger = ice::GLogger; @@ -195,7 +192,7 @@ void AgentImpl::processConfig(JsonCpp::Value &d, JsonCpp::Value &answer) mUseNativeAudio = d["nativeaudio"].asBool(); config()[CONFIG_OWN_DNS] = d["dns_servers"].asString(); - config()[CONFIG_SIPS] = d["secure"].asBool(); + config()[CONFIG_SIPS] = d["secure"].asBool() || transport == "tls"; config()[CONFIG_STUNSERVER_IP] = d["stun_server"].asString(); answer["status"] = Status_Ok; @@ -483,18 +480,18 @@ void AgentImpl::processDestroySession(JsonCpp::Value& request, JsonCpp::Value& a void AgentImpl::processWaitForEvent(JsonCpp::Value &request, JsonCpp::Value &answer) { - std::unique_lock l(mAgentMutex); - - //int x = 0; - //int y = 1/x; + // Deliberately does NOT take mAgentMutex: events are produced by the worker + // thread inside process(), which needs mAgentMutex. Holding it here would + // stall all SIP/media processing for the whole timeout and guarantee that + // the awaited event can never arrive during the wait. int timeout = 0; if (request.isMember("timeout")) timeout = request["timeout"].asInt(); std::unique_lock eventLock(mEventListMutex); - if (mEventList.empty()) - mEventListChangeCondVar.wait_for(eventLock, chrono::milliseconds(timeout)); + mEventListChangeCondVar.wait_for(eventLock, chrono::milliseconds(timeout), + [this]() { return !mEventList.empty(); }); if (!mEventList.empty()) { @@ -521,7 +518,7 @@ void AgentImpl::processGetMediaStats(JsonCpp::Value& request, JsonCpp::Value& an answer["codec"] = result[SessionInfo_AudioCodec].asStdString(); if (result.exists(SessionInfo_NetworkMos)) answer["network_mos"] = result[SessionInfo_NetworkMos].asFloat(); - if (result.exists(SessionInfo_PacketLoss)) + if (result.exists(SessionInfo_LostRtp)) answer["rtp_lost"] = result[SessionInfo_LostRtp].asInt(); if (result.exists(SessionInfo_DroppedRtp)) answer["rtp_dropped"] = result[SessionInfo_DroppedRtp].asInt(); @@ -749,7 +746,8 @@ void AgentImpl::onSessionTerminated(PSession s, int responsecode, int reason) if (mOutgoingAudioDump) mOutgoingAudioDump->close(); */ - mAudioManager->stop(mUseNativeAudio ? AudioManager::atReceiver : AudioManager::atNull); + if (mAudioManager) + mAudioManager->stop(mUseNativeAudio ? AudioManager::atReceiver : AudioManager::atNull); // Gather statistics before EVENT_WITH_NAME("session_terminated"); v["session_id"] = s->id(); diff --git a/src/engine/agent/Agent_Impl.h b/src/engine/agent/Agent_Impl.h index 76c6496d..5fc4a782 100644 --- a/src/engine/agent/Agent_Impl.h +++ b/src/engine/agent/Agent_Impl.h @@ -13,6 +13,7 @@ #include "Agent_AudioManager.h" #include #include +#include class AgentImpl: public UserAgent, public MT::Stream::MediaObserver @@ -32,7 +33,7 @@ protected: std::shared_ptr mThread; - volatile bool mShutdown; + std::atomic mShutdown; std::shared_ptr mTerminal; std::shared_ptr mAudioManager; Audio::DataConnection* mAudioMonitoring = nullptr; diff --git a/src/engine/audio/Audio_DataWindow.cpp b/src/engine/audio/Audio_DataWindow.cpp index 8f327ec6..5c725c6a 100644 --- a/src/engine/audio/Audio_DataWindow.cpp +++ b/src/engine/audio/Audio_DataWindow.cpp @@ -24,23 +24,22 @@ void DataWindow::setCapacity(size_t capacity) { Lock l(mMutex); - if (capacity >= mCapacity) + // The window only ever grows; a smaller request keeps the current buffer. + if (capacity <= mCapacity) + return; + + size_t tail = capacity - mCapacity; + char* buffer = mData; + mData = (char*)realloc(mData, capacity); + if (!mData) { - size_t tail = capacity - mCapacity; - char* buffer = mData; - mData = (char*)realloc(mData, capacity); - if (!mData) - { - // Realloc failed - mData = buffer; - throw std::bad_alloc(); - } - if (tail > 0) - memset(mData + mCapacity, 0, tail); - mCapacity = capacity; - } - else + // Realloc failed + mData = buffer; throw std::bad_alloc(); + } + if (tail > 0) + memset(mData + mCapacity, 0, tail); + mCapacity = capacity; } void DataWindow::addZero(size_t length) diff --git a/src/engine/audio/Audio_Mixer.cpp b/src/engine/audio/Audio_Mixer.cpp index 5cade70c..fdd89626 100644 --- a/src/engine/audio/Audio_Mixer.cpp +++ b/src/engine/audio/Audio_Mixer.cpp @@ -94,6 +94,7 @@ Mixer::~Mixer() void Mixer::unregisterChannel(void* channel) { + Lock l(mMutex); for (int i=0; i(got); } int Mixer::available() diff --git a/src/engine/audio/Audio_WavFile.cpp b/src/engine/audio/Audio_WavFile.cpp index f286fa52..8a917088 100644 --- a/src/engine/audio/Audio_WavFile.cpp +++ b/src/engine/audio/Audio_WavFile.cpp @@ -60,17 +60,11 @@ std::string WavFileReader::readChunk() uint32_t size = 0; readBuffer(&size, 4); - if (result == "fact") - { - uint32_t dataLength = 0; - readBuffer(&dataLength, sizeof dataLength); - mDataLength = dataLength; - } - else - if (result != "data") - mInput->seekg(size, std::ios_base::beg); - else + if (result == "data") mDataLength = size; + else + // Skip the chunk body; RIFF chunks are word-aligned, so odd sizes carry a pad byte + mInput->seekg(std::streamoff(size + (size & 1)), std::ios_base::cur); return result; } @@ -151,7 +145,9 @@ bool WavFileReader::open(const std::filesystem::path& p) mBits = 0; readBuffer(&mBits, sizeof(mBits)); - if (mBits !=8 && mBits != 16) + // Only 16-bit PCM is supported: the read path feeds the data + // directly into a 16-bit resampler. + if (mBits != 16) THROW_READERROR; // Look for the chunk 'data' @@ -222,7 +218,8 @@ size_t WavFileReader::read(short* buffer, size_t samples) auto filePosition = mInput->tellg(); // Check how much data we can read - size_t fileAvailable = mDataLength + mDataOffset - filePosition; + std::streamoff dataEnd = std::streamoff(mDataLength) + mDataOffset; + size_t fileAvailable = filePosition < dataEnd ? size_t(dataEnd - filePosition) : 0; requiredBytes = fileAvailable < requiredBytes ? fileAvailable : requiredBytes; } @@ -254,8 +251,9 @@ size_t WavFileReader::readRaw(short* buffer, size_t samples) auto filePosition = mInput->tellg(); // Check how much data we can read - size_t fileAvailable = mDataLength + mDataOffset - filePosition; - requiredBytes = (int)fileAvailable < requiredBytes ? (int)fileAvailable : requiredBytes; + std::streamoff dataEnd = std::streamoff(mDataLength) + mDataOffset; + size_t fileAvailable = filePosition < dataEnd ? size_t(dataEnd - filePosition) : 0; + requiredBytes = fileAvailable < requiredBytes ? fileAvailable : requiredBytes; } size_t readBytes = tryReadBuffer(buffer, requiredBytes); @@ -332,10 +330,11 @@ bool WavFileWriter::open(const std::filesystem::path& p, int samplerate, int cha mChannels = channels; mOutput = std::make_unique(p, std::ios::binary | std::ios::trunc); - if (!mOutput) + if (!mOutput->is_open()) { int errorcode = errno; ICELogError(<< "Failed to create .wav file: filename = " << p << " , error = " << errorcode); + mOutput.reset(); return false; } @@ -420,7 +419,7 @@ size_t WavFileWriter::write(const void* buffer, size_t bytes) bool WavFileWriter::isOpened() const { LOCK; - return mOutput.get(); + return mOutput && mOutput->is_open(); } std::filesystem::path WavFileWriter::path() const diff --git a/src/engine/endpoint/EP_AudioProvider.cpp b/src/engine/endpoint/EP_AudioProvider.cpp index 43d1023d..d8d2fd9f 100644 --- a/src/engine/endpoint/EP_AudioProvider.cpp +++ b/src/engine/endpoint/EP_AudioProvider.cpp @@ -93,11 +93,13 @@ void AudioProvider::updateSdpOffer(resip::SdpContents::Session::Medium& sdp, Sdp // Check if SRTP suite is found already or not if (mSrtpSuite == SRTP_NONE) { + // RFC 4568 requires a unique tag per crypto attribute; use the suite id. for (int suite = SRTP_AES_128_AUTH_80; suite <= SRTP_LAST; suite++) - sdp.addAttribute("crypto", resip::Data(createCryptoAttribute((SrtpSuite)suite))); + sdp.addAttribute("crypto", resip::Data(createCryptoAttribute((SrtpSuite)suite, suite))); } else - sdp.addAttribute("crypto", resip::Data(createCryptoAttribute(mSrtpSuite))); + // Answer/re-offer: echo the tag of the negotiated attribute. + sdp.addAttribute("crypto", resip::Data(createCryptoAttribute(mSrtpSuite, mSrtpTag))); } // Use CodecListPriority mCodecPriority adapter to work with codec priorities @@ -246,11 +248,13 @@ bool AudioProvider::processSdpOffer(const resip::SdpContents::Session::Medium& m { const resip::Data& attr = *attrIter; ByteBuffer tempkey; - SrtpSuite suite = processCryptoAttribute(attr, tempkey); - if (suite > ss) + int tag = 1; + SrtpSuite suite = processCryptoAttribute(attr, tempkey, &tag); + if (srtpSuiteStrength(suite) > srtpSuiteStrength(ss)) { ss = suite; mSrtpSuite = suite; + mSrtpTag = tag; key = tempkey; } } @@ -295,26 +299,30 @@ MT::PStream AudioProvider::activeStream() return mActiveStream; } -std::string AudioProvider::createCryptoAttribute(SrtpSuite suite) +std::string AudioProvider::createCryptoAttribute(SrtpSuite suite, int tag) { if (!mActiveStream) return ""; // Print key to base64 string PByteBuffer keyBuffer = mActiveStream->srtp().outgoingKey(suite).first; + if (!keyBuffer) + return ""; resip::Data d(keyBuffer->data(), keyBuffer->size()); resip::Data keyText = d.base64encode(); - return std::format("{} {} inline:{}", 1, toString(suite), keyText.c_str()); + return std::format("{} {} inline:{}", tag, toString(suite), keyText.c_str()); } -SrtpSuite AudioProvider::processCryptoAttribute(const resip::Data& value, ByteBuffer& key) +SrtpSuite AudioProvider::processCryptoAttribute(const resip::Data& value, ByteBuffer& key, int* tag) { int srtpTag = 0; char suite[64], keyChunk[256]; int components = sscanf(value.c_str(), "%d %63s inline: %255s", &srtpTag, suite, keyChunk); if (components != 3) return SRTP_NONE; + if (tag) + *tag = srtpTag; const char* delimiter = strchr(keyChunk, '|'); resip::Data keyText; diff --git a/src/engine/endpoint/EP_AudioProvider.h b/src/engine/endpoint/EP_AudioProvider.h index 6c81dbf0..0b31864e 100644 --- a/src/engine/endpoint/EP_AudioProvider.h +++ b/src/engine/endpoint/EP_AudioProvider.h @@ -74,7 +74,7 @@ public: void setupMirror(bool enable); void configureMediaObserver(MT::Stream::MediaObserver* observer, void* userTag); - static SrtpSuite processCryptoAttribute(const resip::Data& value, ByteBuffer& key); + static SrtpSuite processCryptoAttribute(const resip::Data& value, ByteBuffer& key, int* tag = nullptr); protected: // SDP's stream name @@ -93,6 +93,7 @@ protected: unsigned mState; SrtpSuite mSrtpSuite; + int mSrtpTag = 1; // RFC 4568 tag of the negotiated crypto attribute struct RemoteCodec { RemoteCodec(MT::Codec::Factory* factory, int payloadType) @@ -109,7 +110,7 @@ protected: MT::Stream::MediaObserver* mMediaObserver = nullptr; void* mMediaObserverTag = nullptr; - std::string createCryptoAttribute(SrtpSuite suite); + std::string createCryptoAttribute(SrtpSuite suite, int tag); void findRfc2833(const resip::SdpContents::Session::Medium::CodecContainer& codecs); // Implements setState() logic. This allows to be called from constructor (it is not virtual function) diff --git a/src/engine/endpoint/EP_Engine.cpp b/src/engine/endpoint/EP_Engine.cpp index dbe54379..c856809e 100644 --- a/src/engine/endpoint/EP_Engine.cpp +++ b/src/engine/endpoint/EP_Engine.cpp @@ -470,8 +470,10 @@ void UserAgent::process() // Send generated packet via provider's method to allow custom scheme of encryption ICELogDebug(<<"Sending ICE packet to " << buffer->remoteAddress().toStdString() << " with " << buffer->comment()); - PDatagramSocket s = iceComponentId == ICE_RTP_ID ? stream.socket4().mRtp : stream.socket4().mRtcp; - stream.provider()->sendData(s, buffer->remoteAddress(), buffer->data(), buffer->size()); + RtpPair& pair = buffer->remoteAddress().family() == AF_INET6 ? stream.socket6() : stream.socket4(); + PDatagramSocket s = iceComponentId == ICE_RTP_ID ? pair.mRtp : pair.mRtcp; + if (s) + stream.provider()->sendData(s, buffer->remoteAddress(), buffer->data(), buffer->size()); break; } } // end of provider iterating @@ -805,7 +807,10 @@ void UserAgent::onEarlyMedia(resip::ClientInviteSessionHandle h, const resip::Si /// called when dialog enters the Early state - typically after getting 18x void UserAgent::onProvisional(resip::ClientInviteSessionHandle h, const resip::SipMessage& msg) { - PSession s = getUserSession(CAST2RESIPSESSION(h)->mSessionId); + ResipSession* rs = CAST2RESIPSESSION(h); + if (!rs) + return; + PSession s = getUserSession(rs->mSessionId); if (!s) return; @@ -821,7 +826,10 @@ void UserAgent::onProvisional(resip::ClientInviteSessionHandle h, const resip::S /// called when a dialog initiated as a UAC enters the connected state void UserAgent::onConnected(resip::ClientInviteSessionHandle h, const resip::SipMessage& msg) { - PSession s = getUserSession(CAST2RESIPSESSION(h)->mSessionId); + ResipSession* rs = CAST2RESIPSESSION(h); + if (!rs) + return; + PSession s = getUserSession(rs->mSessionId); if (!s) return; @@ -874,7 +882,10 @@ void UserAgent::onConnected(resip::InviteSessionHandle h, const resip::SipMessag void UserAgent::onTerminated(resip::InviteSessionHandle h, resip::InviteSessionHandler::TerminatedReason reason, const resip::SipMessage* related) { - PSession s = getUserSession(CAST2RESIPSESSION(h)->mSessionId); + ResipSession* rs = CAST2RESIPSESSION(h); + if (!rs) + return; + PSession s = getUserSession(rs->mSessionId); if (!s) return; @@ -920,6 +931,8 @@ void UserAgent::onAnswer(resip::InviteSessionHandle h, const resip::SipMessage& if (!resipSession) return; Session* s = resipSession->session(); + if (!s) + return; bool iceAvailable = true; @@ -1069,7 +1082,8 @@ void UserAgent::onAnswer(resip::InviteSessionHandle h, const resip::SipMessage& /// Called when an SDP offer is received - must send an answer soon after this void UserAgent::onOffer(resip::InviteSessionHandle h, const resip::SipMessage& msg, const resip::SdpContents& sdp) { - PSession s = getUserSession(CAST2RESIPSESSION(h)->mSessionId); + ResipSession* rs = CAST2RESIPSESSION(h); + PSession s = rs ? getUserSession(rs->mSessionId) : PSession(); if (!s) { h->reject(488); @@ -1091,7 +1105,8 @@ void UserAgent::onOffer(resip::InviteSessionHandle h, const resip::SipMessage& m uint64_t version = sdp.session().origin().getVersion(); std::string remoteIp = sdp.session().connection().getAddress().c_str(); - int code; + // Default to 200: a retransmitted offer (same origin version) keeps the session. + int code = 200; if ((uint64_t)-1 == s->mRemoteOriginVersion) { code = s->processSdp(version, iceAvailable, icePwd, iceUfrag, remoteIp, sdp.session().media()); @@ -1299,6 +1314,8 @@ void UserAgent::onPresenceUpdate(PClientObserver observer, const std::string& pe void UserAgent::onNewSubscription(resip::ServerSubscriptionHandle h, const resip::SipMessage& sub) { ResipSession* s = CAST2RESIPSESSION(h); + if (!s) + return; // Get the event package name const char* event = sub.header(resip::h_Event).value().c_str(); diff --git a/src/engine/endpoint/EP_Session.cpp b/src/engine/endpoint/EP_Session.cpp index 4fa93b04..7972582b 100644 --- a/src/engine/endpoint/EP_Session.cpp +++ b/src/engine/endpoint/EP_Session.cpp @@ -346,6 +346,10 @@ void Session::stop() // Free socket SocketHeap::instance().freeSocketPair( dataStream.socket4() ); SocketHeap::instance().freeSocketPair( dataStream.socket6() ); + + // Drop the references so the destructor's cleanup does not free them again + dataStream.setSocket4(RtpPair()); + dataStream.setSocket6(RtpPair()); } } @@ -475,7 +479,7 @@ void Session::getSessionInfo(Session::InfoOptions options, VariantMap& info) if (stat.mReceivedRtp) info[SessionInfo_PacketLoss] = static_cast((stat.mPacketLoss * 1000) / stat.mReceivedRtp); - if (media) + if (media && mIceStack) info[SessionInfo_AudioPeer] = mIceStack->remoteAddress(media->iceInfo().mStreamId, media->iceInfo().mComponentId.mRtp).toStdString(); info[SessionInfo_Jitter] = stat.mJitter; @@ -485,7 +489,8 @@ void Session::getSessionInfo(Session::InfoOptions options, VariantMap& info) info[SessionInfo_BitrateSwitchCounter] = stat.mBitrateSwitchCounter; info[SessionInfo_CngCounter] = stat.mCng; #endif - info[SessionInfo_SSRC] = stat.mSsrc; + // Variant stores VTYPE_INT here; keep the 32 bits (consumers read it back with asInt()). + info[SessionInfo_SSRC] = static_cast(stat.mSsrc); info[SessionInfo_RemotePeer] = stat.mRemotePeer.toStdString(); } @@ -741,9 +746,12 @@ PDataProvider Session::findProviderByPort(int family, unsigned short port) { Stream& s = mStreamList[i]; - if ((s.socket4().mRtp->localport() == port || s.socket4().mRtcp->localport() == port) && family == AF_INET) + // Sockets may not be allocated yet (stream created from SDP, sockets follow later) + if (family == AF_INET && s.socket4().mRtp && s.socket4().mRtcp && + (s.socket4().mRtp->localport() == port || s.socket4().mRtcp->localport() == port)) return s.provider(); - if ((s.socket6().mRtp->localport() == port || s.socket6().mRtcp->localport() == port) && family == AF_INET6) + if (family == AF_INET6 && s.socket6().mRtp && s.socket6().mRtcp && + (s.socket6().mRtp->localport() == port || s.socket6().mRtcp->localport() == port)) return s.provider(); } diff --git a/src/engine/helper/HL_IuUP.cpp b/src/engine/helper/HL_IuUP.cpp index 4a84fd6f..e10cb871 100644 --- a/src/engine/helper/HL_IuUP.cpp +++ b/src/engine/helper/HL_IuUP.cpp @@ -5,6 +5,10 @@ bool IuUP::TwoBytePseudoheader = false; bool IuUP::parse(const uint8_t *packet, int size, IuUP::Frame &result) { + // Data-with-CRC frames carry a 4 byte header + if (size < 4) + return false; + // Wrap incoming packet in byte buffer BitReader reader(packet, size); @@ -45,6 +49,10 @@ bool IuUP::parse2(const uint8_t* packet, int size, Frame& result) size -= 2; } + // Frame header is 3 bytes (no CRC) or 4 bytes (with CRC) + if (size < 3) + return false; + BitReader reader(packet, size); result.mPduType = (PduType)reader.readBits(4); @@ -52,6 +60,9 @@ bool IuUP::parse2(const uint8_t* packet, int size, Frame& result) if (result.mPduType != PduType::DataNoCrc && result.mPduType != PduType::DataWithCrc) return false; + if (result.mPduType == PduType::DataWithCrc && size < 4) + return false; + result.mFrameNumber = reader.readBits(4); result.mFqc = reader.readBits(2); result.mRfci = reader.readBits(6); diff --git a/src/engine/helper/HL_Rtp.cpp b/src/engine/helper/HL_Rtp.cpp index b78c95c8..37d5ff21 100644 --- a/src/engine/helper/HL_Rtp.cpp +++ b/src/engine/helper/HL_Rtp.cpp @@ -93,7 +93,8 @@ bool RtpHelper::isRtp(const void* buffer, size_t length) bool RtpHelper::isRtpOrRtcp(const void* buffer, size_t length) { - if (length < 12) + // A minimal RTCP packet (e.g. an empty receiver report) is 8 bytes + if (length < 8) return false; const RtcpHeader* h = reinterpret_cast(buffer); return h->version == 2; @@ -390,7 +391,8 @@ void RtpDump::add(const void* buffer, size_t len, uint32_t offsetMs) if (!buffer || len == 0) return; - if (len > MAX_RTP_PACKET_SIZE) + // The record length field is 16-bit and covers payload + 8 byte header + if (len > MAX_RTP_PACKET_SIZE - 8) throw std::runtime_error("Packet too large: " + std::to_string(len)); RtpData entry; diff --git a/src/engine/helper/HL_SocketHeap.cpp b/src/engine/helper/HL_SocketHeap.cpp index deb2df56..04ff3b21 100644 --- a/src/engine/helper/HL_SocketHeap.cpp +++ b/src/engine/helper/HL_SocketHeap.cpp @@ -97,7 +97,20 @@ RtpPair SocketHeap::allocSocketPair(int family, SocketSink *sin rtcp = allocSocket(family, sink, rtp->localport() + 1); } catch(...) - {} + { + // Release a partially allocated pair before retrying - otherwise + // the RTP socket from this attempt leaks into the socket map. + if (rtp) + { + freeSocket(rtp); + rtp.reset(); + } + if (rtcp) + { + freeSocket(rtcp); + rtcp.reset(); + } + } } if (!rtp || !rtcp) @@ -139,6 +152,9 @@ PDatagramSocket SocketHeap::allocSocket(int family, SocketSink* sink, int port) sockaddr_in6 addr6; int result = 0; int testport; + // A fixed port cannot be retried (it would loop forever if the port is + // owned by another process); random ports get a bounded number of attempts. + int attemptsLeft = port ? 1 : 100; do { testport = port ? port : rand() % ((mFinish - mStart) / 2) * 2 + mStart; @@ -164,7 +180,7 @@ PDatagramSocket SocketHeap::allocSocket(int family, SocketSink* sink, int port) break; } - } while (result == WSAEADDRINUSE); + } while (result == WSAEADDRINUSE && --attemptsLeft > 0); if (result) { diff --git a/src/engine/helper/HL_String.cpp b/src/engine/helper/HL_String.cpp index cba30d8d..8f50ee98 100644 --- a/src/engine/helper/HL_String.cpp +++ b/src/engine/helper/HL_String.cpp @@ -48,9 +48,12 @@ std::string strx::appendPath(const std::string& s1, const std::string& s2) std::string strx::makeUtf8(const std::tstring &arg) { #if defined(TARGET_WIN) - size_t required = WideCharToMultiByte(CP_UTF8, 0, arg.c_str(), -1, NULL, 0, NULL, NULL); - char *result = (char*)_alloca(required + 1); - WideCharToMultiByte(CP_UTF8, 0, arg.c_str(), -1, result, required+1, NULL, NULL); + int required = WideCharToMultiByte(CP_UTF8, 0, arg.c_str(), -1, NULL, 0, NULL, NULL); + if (required <= 0) + return std::string(); + std::string result(static_cast(required), '\0'); + WideCharToMultiByte(CP_UTF8, 0, arg.c_str(), -1, &result[0], required, NULL, NULL); + result.resize(strlen(result.c_str())); // strip the trailing NUL written by the API return result; #else return arg; @@ -65,9 +68,12 @@ std::string strx::toUtf8(const std::tstring &arg) std::tstring strx::makeTstring(const std::string& arg) { #if defined(TARGET_WIN) - size_t count = MultiByteToWideChar(CP_UTF8, 0, arg.c_str(), -1, NULL, 0); - wchar_t* result = (wchar_t*)_alloca(count * 2); - MultiByteToWideChar(CP_UTF8, 0, arg.c_str(), -1, result, count); + int count = MultiByteToWideChar(CP_UTF8, 0, arg.c_str(), -1, NULL, 0); + if (count <= 0) + return std::tstring(); + std::wstring result(static_cast(count), L'\0'); + MultiByteToWideChar(CP_UTF8, 0, arg.c_str(), -1, &result[0], count); + result.resize(wcslen(result.c_str())); // strip the trailing NUL written by the API return result; #else return arg; @@ -93,11 +99,7 @@ int strx::toInt(const char *s, int defaultValue, bool* isOk) uint64_t strx::toUint64(const char* s, uint64_t def, bool *isOk) { uint64_t result = def; -#if defined(TARGET_WIN) - if (sscanf(s, "%I64d", &result) != 1) -#else - if (sscanf(s, "%llu", &result) != 1) -#endif + if (sscanf(s, "%" SCNu64, &result) != 1) { if (isOk) *isOk = false; @@ -143,7 +145,6 @@ std::string strx::toHex(const uint8_t* input, size_t inputLength) *r++ = hexmap[hi]; *r++ = hexmap[low]; } - *r = 0; return result; } @@ -171,11 +172,9 @@ std::string strx::doubleToString(double value, int precision) const char* strx::findSubstring(const char* buffer, const char* substring, size_t bufferLength) { -#if defined(TARGET_WIN) - return (const char*)strstr(buffer, substring); -#else + // The buffer is not necessarily NUL-terminated, so a bounded search is + // required on every platform (a memmem replacement for MSVC is provided below). return (const char*)memmem(buffer, bufferLength, substring, strlen(substring)); -#endif } @@ -332,7 +331,7 @@ std::string strx::fromHex2String(const std::string& s) std::string result; result.resize(s.size() / 2); const char* t = s.c_str(); for (size_t i = 0; i < result.size(); i++) - result[i] = hex2code(t[i*2]); + result[i] = static_cast((hex2code(t[i*2]) << 4) | hex2code(t[i*2+1])); return result; } @@ -367,15 +366,19 @@ std::string strx::decodeUri(const std::string& s) char ch; - int i, ii; + int i, ii = 0; for (i=0; i<(int)s.length(); i++) { - if (s[i] == 37) + if (s[i] == '%' && i + 2 < (int)s.length()) { - sscanf(s.substr(i+1,2).c_str(), "%x", &ii); - ch = static_cast(ii); - ret += ch; - i += 2; + if (sscanf(s.substr(i+1,2).c_str(), "%x", &ii) == 1) + { + ch = static_cast(ii); + ret += ch; + i += 2; + } + else + ret += s[i]; } else ret += s[i]; @@ -385,14 +388,16 @@ std::string strx::decodeUri(const std::string& s) bool strx::startsWith(const std::string& s, const std::string& prefix) { - std::string::size_type p = s.find(prefix); - return p == 0; + if (prefix.size() > s.size()) + return false; + return s.compare(0, prefix.size(), prefix) == 0; } bool strx::endsWith(const std::string& s, const std::string& suffix) { - std::string::size_type p = s.rfind(suffix); - return (p == s.size() - suffix.size()); + if (suffix.size() > s.size()) + return false; + return s.compare(s.size() - suffix.size(), suffix.size(), suffix) == 0; } int strx::stringToDuration(const std::string& s) diff --git a/src/engine/helper/HL_Sync.cpp b/src/engine/helper/HL_Sync.cpp index 6c62326b..424d831b 100644 --- a/src/engine/helper/HL_Sync.cpp +++ b/src/engine/helper/HL_Sync.cpp @@ -20,7 +20,8 @@ void SyncHelper::delay(unsigned int microseconds) { #ifdef TARGET_WIN - ::Sleep(microseconds/1000); + // Round up so sub-millisecond delays do not become Sleep(0) + ::Sleep((microseconds + 999) / 1000); #endif #if defined(TARGET_OSX) || defined(TARGET_LINUX) timespec requested, remaining; @@ -93,8 +94,9 @@ uint32_t chronox::getDelta(uint32_t later, uint32_t earlier) if (later > earlier) return later - earlier; + // Counter wrapped: unsigned subtraction yields the correct modulo-2^32 delta if (later < earlier && later < 0x7FFFFFFF && earlier >= 0x7FFFFFFF) - return 0xFFFFFFFF - earlier + later; + return later - earlier; return 0; } @@ -115,8 +117,8 @@ uint64_t chronox::toTimestamp(const timeval& ts) int64_t chronox::getDelta(const timespec& a, const timespec& b) { - uint64_t ms_a = (uint64_t)a.tv_sec * 1000 + a.tv_nsec / 10000000; - uint64_t ms_b = (uint64_t)b.tv_sec * 1000 + b.tv_nsec / 10000000; + int64_t ms_a = (int64_t)a.tv_sec * 1000 + a.tv_nsec / 1000000; + int64_t ms_b = (int64_t)b.tv_sec * 1000 + b.tv_nsec / 1000000; return ms_a - ms_b; } @@ -162,13 +164,11 @@ void BufferQueue::push(const void* data, int bytes) BufferQueue::PBlock BufferQueue::pull(int milliseconds) { std::unique_lock l(mMutex); - std::cv_status status = mBlockList.empty() ? std::cv_status::timeout : std::cv_status::no_timeout; - - if (mBlockList.empty()) - status = mSignal.wait_for(l, std::chrono::milliseconds(milliseconds)); + mSignal.wait_for(l, std::chrono::milliseconds(milliseconds), + [this]() { return !mBlockList.empty(); }); PBlock r; - if (status == std::cv_status::no_timeout && !mBlockList.empty()) + if (!mBlockList.empty()) { r = mBlockList.front(); mBlockList.pop_front(); diff --git a/src/engine/media/MT_AudioCodec.cpp b/src/engine/media/MT_AudioCodec.cpp index 92b2958e..75688e36 100644 --- a/src/engine/media/MT_AudioCodec.cpp +++ b/src/engine/media/MT_AudioCodec.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #define LOG_SUBSYSTEM "media" @@ -434,9 +435,10 @@ Codec::Info OpusCodec::info() { Codec::EncodeResult OpusCodec::encode(std::span input, std::span output) { - // Send number of samples for input and number of bytes for output + // opus_encode() takes the frame size in samples per channel and the output + // capacity in bytes. int written = opus_encode(mEncoderCtx, (const opus_int16*)input.data(), input.size_bytes() / (sizeof(short) * channels()), - output.data(), output.size_bytes() / (sizeof(short) * channels())); + output.data(), output.size_bytes()); if (written < 0) return {.mEncoded = 0}; else @@ -445,10 +447,13 @@ Codec::EncodeResult OpusCodec::encode(std::span input, std::span< Codec::DecodeResult OpusCodec::decode(std::span input, std::span output) { - int result = 0; + if (input.empty()) + return {0}; // Examine the number of channels available in incoming packet int nr_of_channels = opus_packet_get_nb_channels(input.data()); + if (nr_of_channels != 1 && nr_of_channels != 2) + return {0}; // Recreate decoder if needed if (mDecoderChannels != nr_of_channels) @@ -473,80 +478,97 @@ Codec::DecodeResult OpusCodec::decode(std::span input, std::span< if (nr_of_frames <= 0) return {0}; - // We support stereo and mono here. - int buffer_capacity = nr_of_frames * sizeof(opus_int16) * nr_of_channels; - opus_int16 *buffer_decode = (opus_int16 *)alloca(buffer_capacity); + // Output must match channels() - that is what info() promises downstream. + size_t needed = (size_t)nr_of_frames * sizeof(opus_int16) * channels(); + if (needed > output.size_bytes()) + return {0}; + + if (nr_of_channels == channels()) + { + int decoded = opus_decode(mDecoderCtx, input.data(), input.size_bytes(), + (opus_int16*)output.data(), nr_of_frames, 0); + if (decoded < 0) + { + ICELogCritical(<< "opus_decode() returned " << decoded); + return {0}; + } + return {.mDecoded = (size_t)decoded * sizeof(opus_int16) * nr_of_channels}; + } + + // Channel count differs from the negotiated one - decode to a temporary + // buffer and convert. + std::vector temp((size_t)nr_of_frames * nr_of_channels); int decoded = opus_decode(mDecoderCtx, input.data(), input.size_bytes(), - buffer_decode, nr_of_frames, 0); + temp.data(), nr_of_frames, 0); if (decoded < 0) { ICELogCritical(<< "opus_decode() returned " << decoded); return {0}; } - opus_int16 *buffer_stereo = nullptr; - int buffer_stereo_capacity = buffer_capacity * 2; - - switch (nr_of_channels) { - case 1: - // Convert to stereo before - buffer_stereo = (opus_int16 *) alloca(buffer_stereo_capacity); - for (int i = 0; i < nr_of_frames; i++) { - buffer_stereo[i * 2 + 1] = buffer_decode[i]; - buffer_stereo[i * 2] = buffer_decode[i]; - } - assert(buffer_stereo_capacity <= output.size_bytes()); - memcpy(output.data(), buffer_stereo, buffer_stereo_capacity); - result = buffer_stereo_capacity; - break; - - case 2: - assert(buffer_capacity <= output.size_bytes()); - memcpy(output.data(), buffer_decode, buffer_capacity); - result = buffer_capacity; - break; - - default: - assert(0); + opus_int16* out = (opus_int16*)output.data(); + if (channels() == 2 && nr_of_channels == 1) + { + for (int i = 0; i < decoded; i++) + out[i * 2] = out[i * 2 + 1] = temp[i]; + return {.mDecoded = (size_t)decoded * sizeof(opus_int16) * 2}; + } + else // mono negotiated, stereo packet + { + for (int i = 0; i < decoded; i++) + out[i] = (opus_int16)((int(temp[i * 2]) + temp[i * 2 + 1]) / 2); + return {.mDecoded = (size_t)decoded * sizeof(opus_int16)}; } - - return {.mDecoded = (size_t)result}; } size_t OpusCodec::plc(int lostPackets, std::span output) { - // Find how much frames do we need to produce and prefill it with silence - int frames_per_packet = (int)pcmLength() / (sizeof(opus_int16) * channels()); - memset(output.data(), 0, output.size_bytes()); + if (lostPackets <= 0 || output.empty()) + return 0; - // Use this pointer as output - opus_int16* data_output = reinterpret_cast(output.data()); + // Total bytes we are asked to conceal, clamped to the output capacity. + size_t packet_bytes = (size_t)pcmLength(); + size_t total = std::min(output.size_bytes(), packet_bytes * (size_t)lostPackets); + memset(output.data(), 0, total); - int nr_of_decoded_frames = 0; + // No decoder yet (PLC before the first decoded packet) - leave silence. + if (!mDecoderCtx || (mDecoderChannels != 1 && mDecoderChannels != 2)) + return total; - // Buffer for single lost frame - opus_int16* buffer_plc = (opus_int16*)alloca(frames_per_packet * mDecoderChannels * sizeof(opus_int16)); - for (int i=0; i(output.data()); + std::vector temp((size_t)samples_per_packet * mDecoderChannels); + + for (int packet = 0; packet < lostPackets; packet++) { - nr_of_decoded_frames = opus_decode(mDecoderCtx, nullptr, 0, buffer_plc, frames_per_packet, 0); - assert(nr_of_decoded_frames == frames_per_packet); - switch (mDecoderChannels) - { - case 1: - // Convert mono to stereo - for (int i=0; i < nr_of_decoded_frames; i++) - data_output[i * 2] = data_output[i * 2 + 1] = buffer_plc[i]; - data_output += frames_per_packet * mChannels; - break; + size_t offset_bytes = (size_t)packet * packet_bytes; + if (offset_bytes + packet_bytes > total) + break; - case 2: - // Just copy data - memcpy(data_output, buffer_plc, frames_per_packet * sizeof(opus_int16) * mDecoderChannels); - data_output += frames_per_packet * mChannels; - break; + int decoded = opus_decode(mDecoderCtx, nullptr, 0, temp.data(), samples_per_packet, 0); + if (decoded <= 0) + break; // keep the pre-filled silence + + opus_int16* dst = out + offset_bytes / sizeof(opus_int16); + if (mDecoderChannels == channels()) + { + memcpy(dst, temp.data(), (size_t)decoded * sizeof(opus_int16) * mDecoderChannels); + } + else if (channels() == 2 && mDecoderChannels == 1) + { + for (int i = 0; i < decoded; i++) + dst[i * 2] = dst[i * 2 + 1] = temp[i]; + } + else // mono negotiated, stereo decoder + { + for (int i = 0; i < decoded; i++) + dst[i] = (opus_int16)((int(temp[i * 2]) + temp[i * 2 + 1]) / 2); } } - return ((uint8_t*)data_output - output.data()); + return total; } size_t OpusCodec::getNumberOfSamples(std::span payload) @@ -1021,14 +1043,29 @@ Codec::EncodeResult GsmCodec::encode(std::span input, std::span input, std::span output) { - if (input.size_bytes() % rtpLength() != 0) + const size_t frameSize = (size_t)rtpLength(); + if (!frameSize || input.size_bytes() % frameSize != 0) return {.mDecoded = 0}; - int i=0; - for (i = 0; i < input.size_bytes() / rtpLength(); i++) - gsm_decode(mGSM, (gsm_byte *)input.data() + 33 * i, (gsm_signal *)output.data() + 160 * i); + // Bytes_65 carries a WAV49 frame pair (33 + 32 bytes) and produces 320 samples + const size_t pcmPerFrame = (mCodecType == Type::Bytes_65) ? 640 : 320; + size_t frames = input.size_bytes() / frameSize; - return {.mDecoded = (size_t)i * 320}; + size_t i; + for (i = 0; i < frames; i++) + { + if ((i + 1) * pcmPerFrame > output.size_bytes()) + break; + + const gsm_byte* in = (const gsm_byte*)input.data() + frameSize * i; + gsm_signal* out = (gsm_signal*)output.data() + (pcmPerFrame / 2) * i; + + gsm_decode(mGSM, (gsm_byte*)in, out); + if (mCodecType == Type::Bytes_65) + gsm_decode(mGSM, (gsm_byte*)(in + 33), out + 160); + } + + return {.mDecoded = i * pcmPerFrame}; } size_t GsmCodec::plc(int lostFrames, std::span output) @@ -1327,8 +1364,11 @@ hr_ref_from_canon(uint16_t *hr_ref, const uint8_t *canon) */ Codec::DecodeResult GsmHrCodec::decode(std::span input, std::span output) { - ByteBuffer bb(input, ByteBuffer::CopyBehavior::UseExternal); - BitReader br(bb); + // hr_ref_from_canon() reads 112 bits (14 bytes) starting at offset 1, + // and the decoder produces 160 samples (320 bytes). + if (input.size_bytes() < 15 || output.size_bytes() < 320) + return {.mDecoded = 0}; + uint16_t hr_ref[22]; hr_ref_from_canon(hr_ref, input.data() + 1); diff --git a/src/engine/media/MT_AudioReceiver.cpp b/src/engine/media/MT_AudioReceiver.cpp index 1ab28300..bece0629 100644 --- a/src/engine/media/MT_AudioReceiver.cpp +++ b/src/engine/media/MT_AudioReceiver.cpp @@ -59,8 +59,6 @@ std::vector& RtpBuffer::Packet::pcm() RtpBuffer::RtpBuffer(Statistics& stat) :mStat(stat) { - if (mStat.mPacketLoss) - std::cout << "Warning: packet loss is not zero" << std::endl; } RtpBuffer::~RtpBuffer() @@ -126,14 +124,14 @@ std::shared_ptr RtpBuffer::add(const std::shared_ptr(packet->GetSSRC()); + mStat.mSsrc = packet->GetSSRC(); // Update jitter ICELogMedia(<< "Adding new packet seqno " << packet->GetSequenceNumber() << " into jitter buffer"); mAddCounter++; // Look for maximum&minimal sequence number; check for dublicates - unsigned maxno = 0xFFFFFFFF, minno = 0; + unsigned maxno = 0, minno = 0xFFFFFFFF; // New sequence number unsigned newSeqno = packet->GetExtendedSequenceNumber(); @@ -414,40 +412,6 @@ CodecList::Settings& AudioReceiver::getCodecSettings() return mCodecSettings; } -size_t decode_packet(Codec& codec, RTPPacket& p, void* output_buffer, size_t output_capacity) -{ - // How much data was produced - size_t result = 0; - - // Handle here regular RTP packets - // Check if payload length is ok - int tail = codec.rtpLength() ? p.GetPayloadLength() % codec.rtpLength() : 0; - - if (!tail) - { - // Find number of frames - int frame_count = codec.rtpLength() ? p.GetPayloadLength() / codec.rtpLength() : 1; - int frame_length = codec.rtpLength() ? codec.rtpLength() : (int)p.GetPayloadLength(); - - // Save last packet time length - // mLastPacketTimeLength = mFrameCount * mCodec->frameTime(); - - // Decode - - for (int i=0; i < frame_count; i++) - { - auto r = codec.decode({p.GetPayloadData() + i * codec.rtpLength(), (size_t)frame_length}, - {(uint8_t*)output_buffer, output_capacity}); - - result += r.mDecoded; - } - } - else - ICELogMedia(<< "RTP packet with tail."); - - return result; -} - Codec* AudioReceiver::add(const std::shared_ptr& p) { Codec* codec = nullptr; @@ -775,8 +739,15 @@ AudioReceiver::DecodeResult AudioReceiver::decodeEmptyTo(Audio::DataWindow& outp // Try to decode it - replay previous audio decoded or use CNG decoder (if payload type is 13) if (mCngPacket->rtp()->GetPayloadType() == 13) { - // Using latest CNG packet to produce comfort noise - auto produced = mCngDecoder.produce(fmt.rate(), options.mElapsed.count(), (short*)(output.data() + output.filled()), false); + // Using latest CNG packet to produce comfort noise. + // Clamp the produced amount to the remaining capacity of the output window - + // the CNG decoder writes straight into its buffer. + size_t bytesPerMs = (size_t)fmt.rate() / 1000 * sizeof(short) * fmt.channels(); + size_t room = output.capacity() - output.filled(); + int ms = bytesPerMs ? (int)std::min(options.mElapsed.count(), (int64_t)(room / bytesPerMs)) : 0; + if (ms <= 0) + return {.mStatus = DecodeResult::Status::Skip}; + auto produced = mCngDecoder.produce(fmt.rate(), ms, (short*)(output.mutableData() + output.filled()), false); output.setFilled(output.filled() + produced); return {.mStatus = DecodeResult::Status::Ok, .mSamplerate = fmt.rate(), .mChannels = fmt.channels()}; } @@ -1037,21 +1008,25 @@ DtmfReceiver::~DtmfReceiver() void DtmfReceiver::add(const std::shared_ptr& p) { auto ev = DtmfBuilder::parseRfc2833({p->GetPayloadData(), p->GetPayloadLength()}); - if (ev.mTone != mEvent || ev.mEnd != mEventEnded) + if (!ev.mTone) + return; // Malformed or unknown event payload + + // A new digit begins when the tone changes, or when the same tone starts + // again after the previous occurrence ended. Retransmitted start/end + // packets keep both fields unchanged and are ignored. The end packet of + // the current tone only updates state - the digit was already reported. + bool newEvent = (ev.mTone != mEvent) || (mEventEnded && !ev.mEnd); + + if (newEvent) { - if (!(mEvent == ev.mTone && !mEventEnded && ev.mEnd)) - { - // New tone is here - if (mCallback) - mCallback(ev.mTone); + if (mCallback) + mCallback(ev.mTone); - // Queue statistics item - mStat.mDtmf2833Timeline.emplace_back(Dtmf2833Event{.mTone = ev.mTone, - .mTimestamp = RtpHelper::toMicroseconds(p->GetReceiveTime())}); - - // Store to avoid triggering on the packet - mEvent = ev.mTone; - mEventEnded = ev.mEnd; - } + // Queue statistics item + mStat.mDtmf2833Timeline.emplace_back(Dtmf2833Event{.mTone = ev.mTone, + .mTimestamp = RtpHelper::toMicroseconds(p->GetReceiveTime())}); } + + mEvent = ev.mTone; + mEventEnded = ev.mEnd; } diff --git a/src/engine/media/MT_AudioStream.cpp b/src/engine/media/MT_AudioStream.cpp index c6a1b610..4f860731 100644 --- a/src/engine/media/MT_AudioStream.cpp +++ b/src/engine/media/MT_AudioStream.cpp @@ -238,6 +238,9 @@ void AudioStream::addData(const void* buffer, int bytes) void AudioStream::copyDataTo(Audio::Mixer& mixer, int needed) { + // mStreamMap is also mutated from the network thread (dataArrived) + Lock l(mMutex); + // Local audio mixer - used to send audio to media observer Audio::Mixer localMixer; Audio::DataWindow forObserver; @@ -282,22 +285,27 @@ void AudioStream::copyDataTo(Audio::Mixer& mixer, int needed) if (mMediaObserver) { - localMixer.mixAndGetPcm(forObserver); - mMediaObserver->onMedia(forObserver.data(), forObserver.capacity(), MT::Stream::MediaDirection::Incoming, this, mMediaObserverTag); + int mixedBytes = localMixer.mixAndGetPcm(forObserver); + if (mixedBytes > 0) + mMediaObserver->onMedia(forObserver.data(), mixedBytes, MT::Stream::MediaDirection::Incoming, this, mMediaObserverTag); } } void AudioStream::dataArrived(PDatagramSocket s, const void* buffer, int length, InternetAddress& source) { + // Protects mStreamMap (also iterated by copyDataTo on the audio thread) + // and the receive/decrypt buffers. + Lock l(mMutex); + jrtplib::RTPIPv6Address addr6; jrtplib::RTPIPv4Address addr4; jrtplib::RTPExternalTransmissionInfo* info = dynamic_cast(mRtpSession.GetTransmissionInfo()); assert(info); // Drop RTP packets if stream is not receiving now; let RTCP go - if (!(state() & (int)StreamState::Receiving) && RtpHelper::isRtpOrRtcp(buffer, length)) + if (!(state() & (int)StreamState::Receiving) && RtpHelper::isRtp(buffer, length)) { - ICELogMedia(<< "Stream is not allowed to receive RTP stream. Ignore the RT(C)P packet"); + ICELogMedia(<< "Stream is not allowed to receive RTP stream. Ignore the RTP packet"); return; } diff --git a/src/engine/media/MT_CngHelper.cpp b/src/engine/media/MT_CngHelper.cpp index 56f7c140..e5292732 100644 --- a/src/engine/media/MT_CngHelper.cpp +++ b/src/engine/media/MT_CngHelper.cpp @@ -135,7 +135,7 @@ namespace MT // Get noise level unsigned char noiseLevel = *dataIn; - float linear = float(1.0 / noiseLevel ? noiseLevel : 1); + float linear = 1.0f / float(noiseLevel ? noiseLevel : 1); // Generate white noise for 16KHz sample rate LPFilter lpf; HPFilter hpf; diff --git a/src/engine/media/MT_CodecList.cpp b/src/engine/media/MT_CodecList.cpp index 12bf901d..0f4f9546 100644 --- a/src/engine/media/MT_CodecList.cpp +++ b/src/engine/media/MT_CodecList.cpp @@ -233,7 +233,7 @@ static int findOctetMode(const char* line) p += strlen(param_name); char int_buf[8] = {0}; size_t int_buf_offset = 0; - while (*p && isdigit(*p) && int_buf_offset < sizeof(int_buf)) + while (*p && isdigit(*p) && int_buf_offset < sizeof(int_buf) - 1) int_buf[int_buf_offset++] = *p++; return atoi(int_buf); } diff --git a/src/engine/media/MT_Dtmf.cpp b/src/engine/media/MT_Dtmf.cpp index 173bea0e..983cc244 100644 --- a/src/engine/media/MT_Dtmf.cpp +++ b/src/engine/media/MT_Dtmf.cpp @@ -37,12 +37,11 @@ void DtmfBuilder::buildRfc2833(const Rfc2833Event& ev, void* output) char* packet = (char*)output; + // RFC 4733: byte 1 is E(1) R(1) volume(6) packet[0] = toneValue; - packet[1] = 1 | (ev.mVolume << 2); + packet[1] = ev.mVolume & 0x3F; if (ev.mEnd) - packet[1] |= 128; - else - packet[1] &= 127; + packet[1] |= 0x80; unsigned short durationValue = htons(ev.mDuration); memcpy(packet + 2, &durationValue, 2); @@ -58,11 +57,11 @@ DtmfBuilder::Rfc2833Event DtmfBuilder::parseRfc2833(std::span payload) uint8_t b0 = payload[0]; uint8_t b1 = payload[1]; - if (b0 >=0 && b0 <= 9) + if (b0 <= 9) r.mTone = '0' + b0; else - if (b0 >= 12 && b0 <= 17) - r.mTone = 'A' + b0; + if (b0 >= 12 && b0 <= 15) + r.mTone = 'A' + b0 - 12; else if (b0 == 10) r.mTone = '*'; @@ -70,9 +69,10 @@ DtmfBuilder::Rfc2833Event DtmfBuilder::parseRfc2833(std::span payload) if (b0 == 11) r.mTone = '#'; - r.mEnd = (b1 & 128); - r.mVolume = (b1 & 127) >> 2; - r.mDuration = ntohs(*(uint16_t*)payload.data()+2); + // RFC 4733: byte 1 is E(1) R(1) volume(6); duration is bytes 2-3, network order + r.mEnd = (b1 & 0x80) != 0; + r.mVolume = b1 & 0x3F; + r.mDuration = (uint16_t(payload[2]) << 8) | payload[3]; return r; } @@ -202,7 +202,7 @@ void PDTMFEncoder_AddTone(double f1, double f2, unsigned ms1, unsigned ms2, unsi int ival = ifix(val); if (ival < -32768) ival = -32768; - else if (val > 32767) + else if (ival > 32767) ival = 32767; result[dataPtr++] = ival / 2; @@ -280,8 +280,9 @@ void DtmfContext::stopTone() switch (mType) { case Dtmf_Rfc2833: + // Mark stopped but keep the entry: getRfc2833() emits the end + // packet(s) for a stopped tone and erases it afterwards. mQueue.front().mStopped = true; - mQueue.erase(mQueue.begin()); break; case Dtmf_Inband: @@ -769,7 +770,7 @@ int zap_dtmf_detect (dtmf_detect_state_t *s, s->fax_tone.v2 = s->fax_tone.v3; s->fax_tone.v3 = s->fax_tone.fac*s->fax_tone.v2 - v1 + famp; - v1 = s->fax_tone.v2; + v1 = s->fax_tone2nd.v2; s->fax_tone2nd.v2 = s->fax_tone2nd.v3; s->fax_tone2nd.v3 = s->fax_tone2nd.fac*s->fax_tone2nd.v2 - v1 + famp; } @@ -865,7 +866,7 @@ printf("Fax energy/Second Harmonic: %f/%f\n", fax_energy, fax_energy_2nd); s->detected_digits++; if (s->current_digits < MAX_DTMF_DIGITS) { - s->digits[s->current_digits++] = hit; + s->digits[s->current_digits++] = 'f'; s->digits[s->current_digits] = '\0'; } else diff --git a/src/engine/media/MT_SrtpHelper.cpp b/src/engine/media/MT_SrtpHelper.cpp index 70a68a7c..e971f7ee 100644 --- a/src/engine/media/MT_SrtpHelper.cpp +++ b/src/engine/media/MT_SrtpHelper.cpp @@ -48,6 +48,24 @@ extern std::string_view toString(SrtpSuite suite) return {}; } +extern int srtpSuiteStrength(SrtpSuite suite) +{ + switch (suite) + { + case SRTP_NONE: return 0; + case SRTP_AES_128_AUTH_NULL: return 1; // no authentication - weakest + case SRTP_AES_128_AUTH_32: return 2; + case SRTP_AES_192_AUTH_32: return 3; + case SRTP_AES_256_AUTH_32: return 4; + case SRTP_AES_128_AUTH_80: return 5; + case SRTP_AES_192_AUTH_80: return 6; + case SRTP_AES_256_AUTH_80: return 7; + case SRTP_AED_AES_128_GCM: return 8; + case SRTP_AED_AES_256_GCM: return 9; + } + return 0; +} + typedef void (*set_srtp_policy_function) (srtp_crypto_policy_t*); set_srtp_policy_function findPolicyFunction(SrtpSuite suite) @@ -95,6 +113,7 @@ SrtpSession::SrtpSession() // Generate outgoing keys for all ciphers auto putKey = [this](SrtpSuite suite, size_t length){ + assert(suite > SRTP_NONE && suite <= SRTP_LAST); auto key = std::make_shared(); key->resize(length); RAND_bytes(key->mutableData(), key->size()); @@ -103,9 +122,9 @@ SrtpSession::SrtpSession() putKey(SRTP_AES_128_AUTH_80, 30); putKey(SRTP_AES_128_AUTH_32, 30); putKey(SRTP_AES_192_AUTH_80, 38); putKey(SRTP_AES_192_AUTH_32, 38); putKey(SRTP_AES_256_AUTH_80, 46); putKey(SRTP_AES_256_AUTH_32, 46); + putKey(SRTP_AES_128_AUTH_NULL, 30); // NULL auth still encrypts - it needs a key+salt putKey(SRTP_AED_AES_128_GCM, 28); putKey(SRTP_AED_AES_256_GCM, 44); - } SrtpSession::~SrtpSession() @@ -214,7 +233,9 @@ SrtpKeySalt& SrtpSession::outgoingKey(SrtpSuite suite) { assert(suite > SRTP_NONE && suite <= SRTP_LAST); Lock l(mGuard); - return mOutgoingKey[int(suite)-1]; // The automated review sometimes give the hints about the possible underflow array index access + // Must use the same indexing as the constructor and open(): the SDP + // crypto attribute has to advertise the key the session encrypts with. + return mOutgoingKey[int(suite)]; } bool SrtpSession::protectRtp(void* buffer, int* length) diff --git a/src/engine/media/MT_SrtpHelper.h b/src/engine/media/MT_SrtpHelper.h index 6a6af54f..0fe5fe33 100644 --- a/src/engine/media/MT_SrtpHelper.h +++ b/src/engine/media/MT_SrtpHelper.h @@ -36,6 +36,10 @@ enum SrtpSuite extern SrtpSuite toSrtpSuite(const std::string_view& s); extern std::string_view toString(SrtpSuite suite); +// Relative cryptographic strength used to pick a suite from an SDP offer. +// Bigger is stronger. The raw enum values do NOT follow strength order. +extern int srtpSuiteStrength(SrtpSuite suite); + typedef std::pair SrtpKeySalt; typedef std::pair SrtpStream; @@ -68,8 +72,10 @@ protected: srtp_t mInboundSession, mOutboundSession; + // Outgoing keys are indexed by the SrtpSuite enum value directly; + // index 0 (SRTP_NONE) is unused. SrtpKeySalt mIncomingKey, - mOutgoingKey[SRTP_LAST]; + mOutgoingKey[SRTP_LAST + 1]; srtp_policy_t mInboundPolicy; srtp_policy_t mOutboundPolicy; SrtpSuite mSuite; diff --git a/src/engine/media/MT_Statistics.h b/src/engine/media/MT_Statistics.h index d9a36f1b..49315322 100644 --- a/src/engine/media/MT_Statistics.h +++ b/src/engine/media/MT_Statistics.h @@ -112,7 +112,7 @@ public: std::map mLoss; // Every item is number of loss of corresping length std::chrono::milliseconds mAudioTime = 0ms; // Decoded/found time in milliseconds size_t mDecodedSize = 0; // Number of decoded bytes - uint16_t mSsrc = 0; // Last known SSRC ID in a RTP stream + uint32_t mSsrc = 0; // Last known SSRC ID in a RTP stream ice::NetworkAddress mRemotePeer; // Last known remote RTP address // AMR codec bitrate switch counter