From a1232d56af998ea370985ab795b8ab0930562d03 Mon Sep 17 00:00:00 2001 From: Thulinma Date: Wed, 14 Jun 2023 13:46:49 +0200 Subject: [PATCH] RTP library fixes and improvements. - Fixes marker bit bug in H264 output over RTP-based protocols - Fixes RTCP packets going over the wrong channel in TCP-based RTSP - Fixes RTSP RTCP packets having wrong timestamps - Fixes payload padding parser bug in H264, H265, VP8 and VP9 RTP-based inputs. - Cleans up WebRTC's needless use of thisPacket.getTime() to use thisTime instead --- lib/rtp.cpp | 34 ++++++++++++++++++++++++++-------- lib/rtp.h | 2 +- src/output/output_rtsp.cpp | 20 ++++++++++---------- src/output/output_sdp.cpp | 2 +- src/output/output_webrtc.cpp | 10 +++++----- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/lib/rtp.cpp b/lib/rtp.cpp index 4457a5ce..354332fb 100644 --- a/lib/rtp.cpp +++ b/lib/rtp.cpp @@ -22,6 +22,11 @@ namespace RTP{ } unsigned int Packet::getPayloadSize() const{ + // If there is more padding than content, ignore the packet + if (getHsize() + (getPadding() ? data[maxDataLen - 1] : 0) >= maxDataLen){ + WARN_MSG("Packet has more padding than payload; ignoring packet"); + return 0; + } return maxDataLen - getHsize() - (getPadding() ? data[maxDataLen - 1] : 0); } @@ -273,7 +278,6 @@ namespace RTP{ void Packet::sendH264(void *socket, void callBack(void *, const char *, size_t, uint8_t), const char *payload, uint32_t payloadlen, uint32_t channel, bool lastOfAccesUnit){ - if ((payload[0] & 0x1F) == 12){return;} /// \todo This function probably belongs in DMS somewhere. if (payloadlen + getHsize() + 2 <= maxDataLen){ data[1] &= 0x7F; // setting the RTP marker bit to 0 @@ -446,12 +450,22 @@ namespace RTP{ unsigned int payloadlen, unsigned int channel, std::string codec){ if (codec == "H264"){ unsigned long sent = 0; + const char * lastPtr = 0; + size_t lastLen = 0; while (sent < payloadlen){ unsigned long nalSize = ntohl(*((unsigned long *)(payload + sent))); - sendH264(socket, callBack, payload + sent + 4, nalSize, channel, - (sent + nalSize + 4) >= payloadlen ? true : false); + // Since we skip filler data, we need to delay sending by one NAL unit to reliably + // detect the end of the access unit. + if ((payload[sent + 4] & 0x1F) != 12){ + // If we have a pointer stored, we know it's not the last one, so send it as non-last. + if (lastPtr){sendH264(socket, callBack, lastPtr, lastLen, channel, false);} + lastPtr = payload + sent + 4; + lastLen = nalSize; + } sent += nalSize + 4; } + // Still a pointer stored? That means it was the last one. Mark it as such and send. + if (lastPtr){sendH264(socket, callBack, lastPtr, lastLen, channel, true);} return; } if (codec == "VP8"){ @@ -512,7 +526,7 @@ namespace RTP{ increaseSequence(); } - void Packet::sendRTCP_SR(void *socket, void callBack(void *, const char *, size_t, uint8_t)){ + void Packet::sendRTCP_SR(void *socket, uint8_t channel, void callBack(void *, const char *, size_t, uint8_t)){ char *rtcpData = (char *)malloc(32); if (!rtcpData){ FAIL_MSG("Could not allocate 32 bytes. Something is seriously messed up."); @@ -529,7 +543,7 @@ namespace RTP{ //*((int *)(rtcpData+16) ) = htonl(getTimeStamp());//rtpts Bit::htobl(rtcpData + 20, sentPackets); // packet Bit::htobl(rtcpData + 24, sentBytes); // octet - callBack(socket, (char *)rtcpData, 28, 0); + callBack(socket, (char *)rtcpData, 28, channel); free(rtcpData); } @@ -867,6 +881,10 @@ namespace RTP{ /// Adds an RTP packet to the converter, outputting DTSC packets and/or updating init data, /// as-needed. void toDTSC::addRTP(const RTP::Packet &pkt){ + if (pkt.getPayloadType() >= 72 && pkt.getPayloadType() <= 76){ + INFO_MSG("RTCP packet, ignoring for decoding"); + return; + } if (codec.empty()){ MEDIUM_MSG("Unknown codec - ignoring RTP packet."); return; @@ -908,17 +926,17 @@ namespace RTP{ // From here on, there is codec-specific parsing. We call handler functions for each codec, // except for the trivial codecs. if (codec == "H264"){ - return handleH264(msTime, pl, plSize, missed, (pkt.getPadding() == 1) ? true : false); + return handleH264(msTime, pl, plSize, missed, false); } if (codec == "AAC"){return handleAAC(msTime, pl, plSize);} if (codec == "MP2" || codec == "MP3"){return handleMP2(msTime, pl, plSize);} if (codec == "HEVC"){return handleHEVC(msTime, pl, plSize, missed);} if (codec == "MPEG2"){return handleMPEG2(msTime, pl, plSize);} if (codec == "VP8"){ - return handleVP8(msTime, pl, plSize, missed, (pkt.getPadding() == 1) ? true : false); + return handleVP8(msTime, pl, plSize, missed, false); } if (codec == "VP9"){ - return handleVP8(msTime, pl, plSize, missed, (pkt.getPadding() == 1) ? true : false); + return handleVP8(msTime, pl, plSize, missed, false); } // Trivial codecs just fill a packet with raw data and continue. Easy peasy, lemon squeezy. if (codec == "ALAW" || codec == "opus" || codec == "PCM" || codec == "ULAW"){ diff --git a/lib/rtp.h b/lib/rtp.h index 9c3f9120..a751857c 100644 --- a/lib/rtp.h +++ b/lib/rtp.h @@ -105,7 +105,7 @@ namespace RTP{ const char *payload, unsigned int payloadlen, unsigned int channel); void sendData(void *socket, void callBack(void *, const char *, size_t, uint8_t), const char *payload, unsigned int payloadlen, unsigned int channel, std::string codec); - void sendRTCP_SR(void *socket, void callBack(void *, const char *, size_t, uint8_t)); + void sendRTCP_SR(void *socket, uint8_t channel, void callBack(void *, const char *, size_t, uint8_t)); void sendRTCP_RR(SDP::Track &sTrk, void callBack(void *, const char *, size_t, uint8_t)); Packet(); diff --git a/src/output/output_rtsp.cpp b/src/output/output_rtsp.cpp index 87de58ac..ef546824 100644 --- a/src/output/output_rtsp.cpp +++ b/src/output/output_rtsp.cpp @@ -148,19 +148,9 @@ namespace Mist{ if (sdpState.tracks[thisIdx].channel == -1){// UDP connection socket = &sdpState.tracks[thisIdx].data; callBack = sendUDP; - if (Util::bootSecs() != sdpState.tracks[thisIdx].rtcpSent){ - sdpState.tracks[thisIdx].pack.setTimestamp(timestamp * SDP::getMultiplier(&M, thisIdx)); - sdpState.tracks[thisIdx].rtcpSent = Util::bootSecs(); - sdpState.tracks[thisIdx].pack.sendRTCP_SR(&sdpState.tracks[thisIdx].rtcp, sendUDP); - } }else{ socket = &myConn; callBack = sendTCP; - if (Util::bootSecs() != sdpState.tracks[thisIdx].rtcpSent){ - sdpState.tracks[thisIdx].pack.setTimestamp(timestamp * SDP::getMultiplier(&M, thisIdx)); - sdpState.tracks[thisIdx].rtcpSent = Util::bootSecs(); - sdpState.tracks[thisIdx].pack.sendRTCP_SR(socket, sendTCP); - } } uint64_t offset = thisPacket.getInt("offset"); @@ -168,6 +158,16 @@ namespace Mist{ sdpState.tracks[thisIdx].pack.sendData(socket, callBack, dataPointer, dataLen, sdpState.tracks[thisIdx].channel, meta.getCodec(thisIdx)); + + if (Util::bootSecs() != sdpState.tracks[thisIdx].rtcpSent){ + if (sdpState.tracks[thisIdx].channel == -1){// UDP connection + sdpState.tracks[thisIdx].pack.sendRTCP_SR(&sdpState.tracks[thisIdx].rtcp, 0, callBack); + }else{ + sdpState.tracks[thisIdx].pack.sendRTCP_SR(socket, sdpState.tracks[thisIdx].channel, callBack); + } + sdpState.tracks[thisIdx].rtcpSent = Util::bootSecs(); + } + static uint64_t lastAnnounce = Util::bootSecs(); if (reqUrl.size() && lastAnnounce + 5 < Util::bootSecs()){ INFO_MSG("Sending announce"); diff --git a/src/output/output_sdp.cpp b/src/output/output_sdp.cpp index 7049defe..26eba8d0 100644 --- a/src/output/output_sdp.cpp +++ b/src/output/output_sdp.cpp @@ -170,7 +170,7 @@ namespace Mist{ if (Util::bootSecs() != sdpState.tracks[thisIdx].rtcpSent){ sdpState.tracks[thisIdx].pack.setTimestamp(timestamp * SDP::getMultiplier(&M, thisIdx)); sdpState.tracks[thisIdx].rtcpSent = Util::bootSecs(); - sdpState.tracks[thisIdx].pack.sendRTCP_SR(&sdpState.tracks[thisIdx].rtcp, sendUDP); + sdpState.tracks[thisIdx].pack.sendRTCP_SR(&sdpState.tracks[thisIdx].rtcp, sdpState.tracks[thisIdx].channel, sendUDP); } }else{ FAIL_MSG("RTP SDP output does not support TCP. No data will be sent to the target address"); diff --git a/src/output/output_webrtc.cpp b/src/output/output_webrtc.cpp index 1cef2884..286749a6 100644 --- a/src/output/output_webrtc.cpp +++ b/src/output/output_webrtc.cpp @@ -1712,8 +1712,8 @@ namespace Mist{ onIdle(); } - if (M.getLive() && stayLive && lastTimeSync + 666 < thisPacket.getTime()){ - lastTimeSync = thisPacket.getTime(); + if (M.getLive() && stayLive && lastTimeSync + 666 < thisTime){ + lastTimeSync = thisTime; if (liveSeek()){return;} } @@ -1760,9 +1760,9 @@ namespace Mist{ // This checks if we have a whole integer multiplier, and if so, // ensures only integer math is used to prevent rounding errors if (mult == (uint64_t)mult){ - rtcTrack.rtpPacketizer.setTimestamp(thisPacket.getTime() * (uint64_t)mult); + rtcTrack.rtpPacketizer.setTimestamp(thisTime * (uint64_t)mult); }else{ - rtcTrack.rtpPacketizer.setTimestamp(thisPacket.getTime() * mult); + rtcTrack.rtpPacketizer.setTimestamp(thisTime * mult); } bool isKeyFrame = thisPacket.getFlag("keyframe"); @@ -1798,7 +1798,7 @@ namespace Mist{ //If this track hasn't sent yet, actually sent if (mustSendSR.count(thisIdx)){ mustSendSR.erase(thisIdx); - rtcTrack.rtpPacketizer.sendRTCP_SR((void *)&udp, onRTPPacketizerHasRTCPDataCallback); + rtcTrack.rtpPacketizer.sendRTCP_SR((void *)&udp, 0, onRTPPacketizerHasRTCPDataCallback); } }