diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 1bfa7cdebc..0faf2a4257 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -1101,8 +1101,7 @@ RTCPReceiver::HandleRPSI(RTCPUtility::RTCPParserV2& rtcpParser, { const RTCPUtility::RTCPPacket& rtcpPacket = rtcpParser.Packet(); RTCPUtility::RTCPPacketTypes pktType = rtcpParser.Iterate(); - if (pktType == RTCPPacketTypes::kPsfbRpsi) { - rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpRpsi; // received signal that we have a confirmed reference picture + if (pktType == RTCPPacketTypes::kPsfbRpsiItem) { if(rtcpPacket.RPSI.NumberOfValidBits%8 != 0) { // to us unknown @@ -1110,6 +1109,8 @@ RTCPReceiver::HandleRPSI(RTCPUtility::RTCPParserV2& rtcpParser, rtcpParser.Iterate(); return; } + // Received signal that we have a confirmed reference picture. + rtcpPacketInformation.rtcpPacketTypeFlags |= kRtcpRpsi; rtcpPacketInformation.rpsiPictureId = 0; // convert NativeBitString to rpsiPictureId diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 507f835bc3..08f109bc29 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -29,6 +29,7 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/pli.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/remb.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rpsi.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sdes.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sli.h" @@ -165,6 +166,50 @@ TEST_F(RtcpReceiverTest, InvalidFeedbackPacketIsIgnored) { EXPECT_EQ(0U, rtcp_packet_info_.rtcpPacketTypeFlags); } +TEST_F(RtcpReceiverTest, RpsiWithFractionalPaddingIsIgnored) { + // Padding size represent fractional number of bytes. + const uint8_t kPaddingSizeBits = 0x0b; + const uint8_t bad_packet[] = {0x83, RTCPUtility::PT_PSFB, 0, 3, + 0x12, 0x34, 0x56, 0x78, + 0x98, 0x76, 0x54, 0x32, + kPaddingSizeBits, 0x00, 0x00, 0x00}; + EXPECT_EQ(0, InjectRtcpPacket(bad_packet, sizeof(bad_packet))); + EXPECT_EQ(0U, rtcp_packet_info_.rtcpPacketTypeFlags); +} + +TEST_F(RtcpReceiverTest, RpsiWithTooLargePaddingIsIgnored) { + // Padding size exceeds packet size. + const uint8_t kPaddingSizeBits = 0xa8; + const uint8_t bad_packet[] = {0x83, RTCPUtility::PT_PSFB, 0, 3, + 0x12, 0x34, 0x56, 0x78, + 0x98, 0x76, 0x54, 0x32, + kPaddingSizeBits, 0x00, 0x00, 0x00}; + EXPECT_EQ(0, InjectRtcpPacket(bad_packet, sizeof(bad_packet))); + EXPECT_EQ(0U, rtcp_packet_info_.rtcpPacketTypeFlags); +} + +// With parsing using rtcp classes this test will make no sense. +// With current stateful parser this test was failing. +TEST_F(RtcpReceiverTest, TwoHalfValidRpsiAreIgnored) { + const uint8_t bad_packet[] = {0x83, RTCPUtility::PT_PSFB, 0, 2, + 0x12, 0x34, 0x56, 0x78, + 0x98, 0x76, 0x54, 0x32, + 0x83, RTCPUtility::PT_PSFB, 0, 2, + 0x12, 0x34, 0x56, 0x78, + 0x98, 0x76, 0x54, 0x32}; + EXPECT_EQ(0, InjectRtcpPacket(bad_packet, sizeof(bad_packet))); + EXPECT_EQ(0U, rtcp_packet_info_.rtcpPacketTypeFlags); +} + +TEST_F(RtcpReceiverTest, InjectRpsiPacket) { + const uint64_t kPictureId = 0x123456789; + rtcp::Rpsi rpsi; + rpsi.WithPictureId(kPictureId); + rtc::Buffer packet = rpsi.Build(); + EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); + EXPECT_EQ(kRtcpRpsi, rtcp_packet_info_.rtcpPacketTypeFlags); +} + TEST_F(RtcpReceiverTest, InjectSrPacket) { const uint32_t kSenderSsrc = 0x10203; rtcp::SenderReport sr; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index f440da489d..dafc2e0be6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -461,6 +461,7 @@ TEST_F(RtcpSenderTest, SendRpsi) { feedback_state.send_payload_type = kPayloadType; EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state, kRtcpRpsi, 0, nullptr, false, kPictureId)); + EXPECT_EQ(1, parser()->rpsi()->num_packets()); EXPECT_EQ(kPayloadType, parser()->rpsi()->PayloadType()); EXPECT_EQ(kPictureId, parser()->rpsi()->PictureId()); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc index 63693fa9c2..9b5a83515f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc @@ -1334,11 +1334,19 @@ bool RTCPUtility::RTCPParserV2::ParseRPSIItem() { return false; } - _packetType = RTCPPacketTypes::kPsfbRpsi; uint8_t padding_bits = *_ptrRTCPData++; _packet.RPSI.PayloadType = *_ptrRTCPData++; + if (padding_bits > static_cast(length - 2) * 8) { + _state = ParseState::State_TopLevel; + + EndCurrentBlock(); + return false; + } + + _packetType = RTCPPacketTypes::kPsfbRpsiItem; + memcpy(_packet.RPSI.NativeBitString, _ptrRTCPData, length - 2); _ptrRTCPData += length - 2; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h index 0b03ceb56e..4067a40886 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h @@ -272,6 +272,7 @@ enum class RTCPPacketTypes { kPsfbPli, kPsfbRpsi, + kPsfbRpsiItem, kPsfbSli, kPsfbSliItem, kPsfbApp, diff --git a/webrtc/test/rtcp_packet_parser.cc b/webrtc/test/rtcp_packet_parser.cc index 8ce249e0b6..65f450d2e3 100644 --- a/webrtc/test/rtcp_packet_parser.cc +++ b/webrtc/test/rtcp_packet_parser.cc @@ -68,7 +68,7 @@ void RtcpPacketParser::Parse(const void *data, size_t len) { case RTCPPacketTypes::kPsfbSliItem: sli_item_.Set(parser.Packet().SLIItem); break; - case RTCPPacketTypes::kPsfbRpsi: + case RTCPPacketTypes::kPsfbRpsiItem: rpsi_.Set(parser.Packet().RPSI); break; case RTCPPacketTypes::kPsfbFir: