From abece9ca044527a83be930c278cd740e446f3e94 Mon Sep 17 00:00:00 2001 From: Avinash Malipatil Date: Mon, 17 Apr 2023 15:03:03 +0000 Subject: Don't stop decoding RTCP compound packet when RTCP-XR report block is encountered. Skip the XR report or any other unknown report block and continue to decode next report in the RTCP compound packet. Fix possible memory leaks. Bug: 276462829 Test: atest ImsMediaNativeTests and live network verified Change-Id: Ica104680d4a0260d8a28d3eef323e415d5b071b8 --- .../libimsmedia/protocol/rtp/core/RtcpPacket.cpp | 38 +++++++++++++++++++--- .../libimsmedia/protocol/rtp/core/RtcpXrPacket.cpp | 6 ++-- .../protocol/rtp/core/RtcpPacketTest.cpp | 33 +++++++++++++++++++ .../protocol/rtp/core/RtcpXrPacketTest.cpp | 4 +-- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacket.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacket.cpp index b67fde10..2f11d8f1 100644 --- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacket.cpp +++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacket.cpp @@ -213,8 +213,8 @@ eRTP_STATUS_CODE RtcpPacket::decodeRtcpPacket(IN RtpBuffer* pobjRtcpPktBuf, usPktLen -= RTP_WORD_SIZE; if (usPktLen > iTrackCompLen) { - RTP_TRACE_ERROR( - "[DecodeRtcpPacket] RTCP packet length is Invalid.", usPktLen, iTrackCompLen); + RTP_TRACE_ERROR("[DecodeRtcpPacket] Report length is Invalid. ReportLen:%d, RtcpLen:%d", + usPktLen, iTrackCompLen); return RTP_INVALID_MSG; } @@ -265,6 +265,9 @@ eRTP_STATUS_CODE RtcpPacket::decodeRtcpPacket(IN RtpBuffer* pobjRtcpPktBuf, case RTCP_SDES: { RTP_TRACE_MESSAGE("[DecodeRtcpPacket] Decoding RTCP_SDES", 0, 0); + if (m_pobjSdesPkt != nullptr) + delete m_pobjSdesPkt; + m_pobjSdesPkt = new RtcpSdesPacket(); if (m_pobjSdesPkt == nullptr) { @@ -279,6 +282,9 @@ eRTP_STATUS_CODE RtcpPacket::decodeRtcpPacket(IN RtpBuffer* pobjRtcpPktBuf, case RTCP_BYE: { RTP_TRACE_MESSAGE("[DecodeRtcpPacket] Decoding RTCP_BYE", 0, 0); + if (m_pobjByePkt != nullptr) + delete m_pobjByePkt; + m_pobjByePkt = new RtcpByePacket(); if (m_pobjByePkt == nullptr) { @@ -293,6 +299,9 @@ eRTP_STATUS_CODE RtcpPacket::decodeRtcpPacket(IN RtpBuffer* pobjRtcpPktBuf, case RTCP_APP: { RTP_TRACE_MESSAGE("[DecodeRtcpPacket] Decoding RTCP_APP", 0, 0); + if (m_pobjAppPkt != nullptr) + delete m_pobjAppPkt; + m_pobjAppPkt = new RtcpAppPacket(); if (m_pobjAppPkt == nullptr) { @@ -321,11 +330,30 @@ eRTP_STATUS_CODE RtcpPacket::decodeRtcpPacket(IN RtpBuffer* pobjRtcpPktBuf, bFbPkt = eRTP_TRUE; break; } // RTCP_RTPFB || RTCP_PSFB + case RTCP_XR: + { + RTP_TRACE_MESSAGE("[DecodeRtcpPacket] Decoding RTCP_XR", 0, 0); + if (m_pobjRtcpXrPkt != nullptr) + delete m_pobjRtcpXrPkt; + + m_pobjRtcpXrPkt = new RtcpXrPacket(); + if (m_pobjRtcpXrPkt == nullptr) + { + RTP_TRACE_ERROR("[Memory Error] new returned NULL.", RTP_ZERO, RTP_ZERO); + return RTP_MEMORY_FAIL; + } + m_pobjRtcpXrPkt->setRtcpHdrInfo(m_objHeader); + eDecodeRes = m_pobjRtcpXrPkt->decodeRtcpXrPacket(pucBuffer, usPktLen, uiPktType); + bOtherPkt = eRTP_TRUE; + break; + } // RTCP_XR default: { - RTP_TRACE_WARNING( - "[DecodeRtcpPacket], Invalid RTCP MSG type received", RTP_ZERO, RTP_ZERO); - return RTP_INVALID_MSG; + RTP_TRACE_WARNING("[DecodeRtcpPacket], Invalid RTCP MSG type[%d] received", + uiPktType, RTP_ZERO); + // Instead of returning failure, ignore unknown report block and continue to decode + // next report block. + eDecodeRes = RTP_SUCCESS; } // default }; // switch diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacket.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacket.cpp index 819f13b6..406f04f0 100644 --- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacket.cpp +++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacket.cpp @@ -58,9 +58,11 @@ eRTP_STATUS_CODE RtcpXrPacket::decodeRtcpXrPacket( (RtpDt_Void) pucRtcpXrBuf; (RtpDt_Void) usRtcpXrLen; (RtpDt_Void) ucPktType; - RTP_TRACE_ERROR("decodeRtcpXrPacket not implemented.", RTP_ZERO, RTP_ZERO); + RTP_TRACE_WARNING("decodeRtcpXrPacket not implemented.", RTP_ZERO, RTP_ZERO); - return RTP_FAILURE; + /* TODO: Currently, there is no requirement to handle XR packets. Returning success to avoid + RTCP decoding issues. */ + return RTP_SUCCESS; } eRTP_STATUS_CODE RtcpXrPacket::formRtcpXrPacket(OUT RtpBuffer* pobjRtcpPktBuf) diff --git a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacketTest.cpp b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacketTest.cpp index 37185728..1c95e03b 100644 --- a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacketTest.cpp +++ b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpPacketTest.cpp @@ -671,6 +671,39 @@ TEST_F(RtcpPacketTest, DecodeOnlyRtcpSRHeader) EXPECT_EQ(pRtcpHeader.getLength(), 0 * RTP_WORD_SIZE); } +/** + * Test RTCP XR packet. + */ +TEST_F(RtcpPacketTest, TestDecodeRtcpXrPacket) +{ + RtcpPacket rtcpPacket; + + /* + * Real-time Transport Control Protocol (Sender Report) + * 10.. .... = Version: RFC 1889 Version (2) + * ..1. .... = Padding: False + * ...0 0001 = Report count: 1 + * Packet type: XR (207) + * Length: 5 (24 bytes) + * SSRC : 0xb1c8cb02 (2982726402) + * 0x00, 0x00, 0x00, 0x01, // XR block type: VoIP Metrics Report Block (207) + * 0x00, 0x0A, // Length of the XR block in 32-bit words: 10 + * 0x02, 0x01, // Loss rate (packets lost per million packets sent): 2 bytes; + * Type-specific: 1 0x00, 0x64, // Loss rate: 100 0x03, 0x01, // Delay + * since last report (milliseconds): 2 bytes; Type-specific: 1 0x00, 0x3C, // Delay: + * 60 milliseconds + */ + uint8_t bufPacket[] = {0xa1, 0xcf, 0x00, 0x05, 0xb1, 0xc8, 0xcb, 0x02, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x0A, 0x02, 0x01, 0x00, 0x64, 0x03, 0x01, 0x00, 0x3C, 0x00, 0x02}; + + RtcpConfigInfo rtcpConfigInfo; + RtpBuffer rtpBuffer(24, bufPacket); + eRTP_STATUS_CODE res = rtcpPacket.decodeRtcpPacket(&rtpBuffer, 0, &rtcpConfigInfo); + EXPECT_EQ(res, RTP_SUCCESS); + + // TODO: After Rtcp-Xr decoder function is implemented, add checks for each files in XR report. +} + TEST_F(RtcpPacketTest, CheckAllGetSets) { RtcpPacket rtcpPacket; diff --git a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacketTest.cpp b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacketTest.cpp index 1c9a1cb8..45624c71 100644 --- a/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacketTest.cpp +++ b/tests/native/service/src/com/android/telephony/imsmedia/lib/libimsmedia/protocol/rtp/core/RtcpXrPacketTest.cpp @@ -47,8 +47,8 @@ TEST(RtcpXrPacketTest, TestDecodeXrPacket) 0x00, 0x00, 0x00, 0x41, 0x00, 0x00, 0xc8, 0x53, 0x81, 0xca, 0x00, 0x0a}; eRTP_STATUS_CODE res = objRtcpXrPacket.decodeRtcpXrPacket(reinterpret_cast(bufXrPacket), 24, 0); - // Result should be failure because decode XR packet is not supported or implemented. - EXPECT_EQ(res, RTP_FAILURE); + + EXPECT_EQ(res, RTP_SUCCESS); } TEST(RtcpXrPacketTest, TestFormXrPacket) -- cgit v1.2.3