From 5675134084ccbcd13cf15498b348e21c1a3b6846 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 30 Aug 2022 19:04:45 +0300 Subject: [PATCH 01/12] Add test to check whether mediasoup parses transport-cc packets the same way as libwebrtc. --- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 219 ++++++++++++++++++ 1 file changed, 219 insertions(+) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 954be59e42..93e47e6b2c 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -571,4 +571,223 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" delete packet; } + + SECTION("parse FeedbackRtpTransportPacket generated by Chrome with libwebrtc as a reference") + { + typedef struct { + uint32_t baseTimeRaw; + uint64_t baseTimeMs; + uint16_t baseSequence; + size_t packetStatusCount; + std::vector deltas; + std::vector buffer; + } FeedbackPacketsMeta; + + // Metadata collected by parsing buffers with libwebrtc, buffers itself. + // were generated by chrome in direction of mediasoup. + std::vector feedbackPacketsMeta = { + { + .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 13, + .packetStatusCount = 1, + .deltas = std::vector{57}, + .buffer = std::vector { + 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, + 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, + 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 + } + }, + { + .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 14, + .packetStatusCount = 4, + .deltas = std::vector{58, 2, 3, 55}, + .buffer = std::vector { + 0xaf, 0xcd, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x0E, 0x00, 0x04, + 0x00, 0x8A, 0xB0, 0x01, 0x20, 0x04, 0xE8, 0x08, + 0x0C, 0xDC, 0x00, 0x02 + } + }, + { + .baseTimeRaw = 35505, + .baseTimeMs = 1076014144, + .baseSequence = 18, + .packetStatusCount = 5, + .deltas = std::vector{60, 6, 5, 9, 22}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x12, 0x00, 0x05, + 0x00, 0x8A, 0xB1, 0x02, 0x20, 0x05, 0xF0, 0x18, + 0x14, 0x24, 0x58, 0x01 + } + }, + + { + .baseTimeRaw = 617873, + .baseTimeMs = 1113285696, + .baseSequence = 2924, + .packetStatusCount = 22, + .deltas = std::vector{3, 5, 5, 0, 10, 0, 0, 4, 0, 1, 0, 2, 0, 2, 0, 2, 0, 2, 0, 1, 0, 4}, + .buffer = std::vector { + 0x8F, 0xCD, 0x00, 0x0A, 0xFA, 0x17, 0xFA, 0x17, + 0x06, 0xF5, 0x11, 0x4C, 0x0B, 0x6C, 0x00, 0x16, + 0x09, 0x6D, 0x91, 0xEE, 0x20, 0x16, 0x0C, 0x14, + 0x14, 0x00, 0x28, 0x00, 0x00, 0x10, 0x00, 0x04, + 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, + 0x00, 0x04, 0x00, 0x10 + } + }, + + { + .baseTimeRaw = 12408746, + .baseTimeMs = 1867901568, + .baseSequence = 1, + .packetStatusCount = 2, + .deltas = std::vector{35, 17}, + .buffer = std::vector { + 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, + 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 + } + }, + + { + .baseTimeRaw = 818995, + .baseTimeMs = 1126157504, + .baseSequence = 930, + .packetStatusCount = 5, + .deltas = std::vector{62, 18, 5, 6, 19}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA2, 0x00, 0x05, + 0x0C, 0x7F, 0x33, 0x9F, 0x20, 0x05, 0xF8, 0x48, + 0x14, 0x18, 0x4C, 0x01 + } + }, + { + .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 921, + .packetStatusCount = 7, + .deltas = std::vector{14, 5, 6, 6, 7, 14, 5}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, + 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0x99, 0x00, 0x07, + 0x0C, 0x7F, 0x34, 0x9F, 0x20, 0x07, 0x38, 0x14, + 0x18, 0x18, 0x1C, 0x38, 0x14, 0x00, 0x00, 0x03 + } + }, + { + .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 935, + .packetStatusCount = 7, + .deltas = std::vector{57, 0, 6, 5, 5, 24, 0}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, + 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA7, 0x00, 0x07, + 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x07, 0xE4, 0x00, + 0x18, 0x14, 0x14, 0x60, 0x00, 0x00, 0x00, 0x03 + } + }, + { + .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 928, + .packetStatusCount = 5, + .deltas = std::vector{63, 11, 21, 6, 0}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0xA0, 0x00, 0x05, + 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x05, 0xFC, 0x2C, + 0x54, 0x18, 0x00, 0x01 + } + }, + { + .baseTimeRaw = 818997, + .baseTimeMs = 1126157632, + .baseSequence = 942, + .packetStatusCount = 6, + .deltas = std::vector{39, 13, 9, 5, 4, 13}, + .buffer = std::vector { + 0x8F, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xAE, 0x00, 0x06, + 0x0C, 0x7F, 0x35, 0xA1, 0x20, 0x06, 0x9C, 0x34, + 0x24, 0x14, 0x10, 0x34 + } + }, + { + .baseTimeRaw = 821523, + .baseTimeMs = 1126319296, + .baseSequence = 10, + .packetStatusCount = 7, + .deltas = std::vector{25, 2, 2, 3, 1, 1, 3}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, + 0x00, 0x00, 0x04, 0xD2, 0x00, 0x0A, 0x00, 0x07, + 0x0C, 0x89, 0x13, 0x00, 0x20, 0x07, 0x64, 0x08, + 0x08, 0x0C, 0x04, 0x04, 0x0C, 0x00, 0x00, 0x03 + } + }, + { + .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 2, + .deltas = std::vector{44, 18}, + .buffer = std::vector { + 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, + 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 + } + }, + { + .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 1, + .deltas = std::vector{62}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, + 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 + } + }, + { + .baseTimeRaw = 821526, + .baseTimeMs = 1126319488, + .baseSequence = 19, + .packetStatusCount = 4, + .deltas = std::vector{4, 0, 4, 0}, + .buffer = std::vector { + 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, + 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x13, 0x00, 0x04, + 0x0C, 0x89, 0x16, 0x02, 0x20, 0x04, 0x10, 0x00, + 0x10, 0x00, 0x00, 0x02 + } + } + }; + + for (const auto &packetMeta: feedbackPacketsMeta) { + auto buffer = packetMeta.buffer; + auto *feedback = FeedbackRtpTransportPacket::Parse(buffer.data(), buffer.size()); + + REQUIRE(feedback->GetReferenceTime() == packetMeta.baseTimeRaw); + REQUIRE(feedback->GetReferenceTimestamp() == packetMeta.baseTimeMs); + REQUIRE(feedback->GetBaseSequenceNumber() == packetMeta.baseSequence); + REQUIRE(feedback->GetPacketStatusCount() == packetMeta.packetStatusCount); + + auto packetsResults = feedback->GetPacketResults(); + + int deltasIt = 0; + for (auto const &delta : packetMeta.deltas) { + auto resultDelta = packetsResults[deltasIt].delta; + REQUIRE(static_cast(resultDelta/ 4) == delta); + deltasIt++; + } + } + } } From 144114d543a0fa35b5aa4e1f9d7f203b42656b59 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 30 Aug 2022 19:31:42 +0300 Subject: [PATCH 02/12] Reformat --- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 377 ++++++++---------- 1 file changed, 159 insertions(+), 218 deletions(-) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 93e47e6b2c..dbc62b0cf9 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -572,222 +572,163 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" delete packet; } - SECTION("parse FeedbackRtpTransportPacket generated by Chrome with libwebrtc as a reference") - { - typedef struct { - uint32_t baseTimeRaw; - uint64_t baseTimeMs; - uint16_t baseSequence; - size_t packetStatusCount; - std::vector deltas; - std::vector buffer; - } FeedbackPacketsMeta; - - // Metadata collected by parsing buffers with libwebrtc, buffers itself. - // were generated by chrome in direction of mediasoup. - std::vector feedbackPacketsMeta = { - { - .baseTimeRaw = 35504, - .baseTimeMs = 1076014080, - .baseSequence = 13, - .packetStatusCount = 1, - .deltas = std::vector{57}, - .buffer = std::vector { - 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, - 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, - 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 - } - }, - { - .baseTimeRaw = 35504, - .baseTimeMs = 1076014080, - .baseSequence = 14, - .packetStatusCount = 4, - .deltas = std::vector{58, 2, 3, 55}, - .buffer = std::vector { - 0xaf, 0xcd, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x0E, 0x00, 0x04, - 0x00, 0x8A, 0xB0, 0x01, 0x20, 0x04, 0xE8, 0x08, - 0x0C, 0xDC, 0x00, 0x02 - } - }, - { - .baseTimeRaw = 35505, - .baseTimeMs = 1076014144, - .baseSequence = 18, - .packetStatusCount = 5, - .deltas = std::vector{60, 6, 5, 9, 22}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x12, 0x00, 0x05, - 0x00, 0x8A, 0xB1, 0x02, 0x20, 0x05, 0xF0, 0x18, - 0x14, 0x24, 0x58, 0x01 - } - }, - - { - .baseTimeRaw = 617873, - .baseTimeMs = 1113285696, - .baseSequence = 2924, - .packetStatusCount = 22, - .deltas = std::vector{3, 5, 5, 0, 10, 0, 0, 4, 0, 1, 0, 2, 0, 2, 0, 2, 0, 2, 0, 1, 0, 4}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x0A, 0xFA, 0x17, 0xFA, 0x17, - 0x06, 0xF5, 0x11, 0x4C, 0x0B, 0x6C, 0x00, 0x16, - 0x09, 0x6D, 0x91, 0xEE, 0x20, 0x16, 0x0C, 0x14, - 0x14, 0x00, 0x28, 0x00, 0x00, 0x10, 0x00, 0x04, - 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, - 0x00, 0x04, 0x00, 0x10 - } - }, - - { - .baseTimeRaw = 12408746, - .baseTimeMs = 1867901568, - .baseSequence = 1, - .packetStatusCount = 2, - .deltas = std::vector{35, 17}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, - 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 - } - }, - - { - .baseTimeRaw = 818995, - .baseTimeMs = 1126157504, - .baseSequence = 930, - .packetStatusCount = 5, - .deltas = std::vector{62, 18, 5, 6, 19}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA2, 0x00, 0x05, - 0x0C, 0x7F, 0x33, 0x9F, 0x20, 0x05, 0xF8, 0x48, - 0x14, 0x18, 0x4C, 0x01 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 921, - .packetStatusCount = 7, - .deltas = std::vector{14, 5, 6, 6, 7, 14, 5}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0x99, 0x00, 0x07, - 0x0C, 0x7F, 0x34, 0x9F, 0x20, 0x07, 0x38, 0x14, - 0x18, 0x18, 0x1C, 0x38, 0x14, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 935, - .packetStatusCount = 7, - .deltas = std::vector{57, 0, 6, 5, 5, 24, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA7, 0x00, 0x07, - 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x07, 0xE4, 0x00, - 0x18, 0x14, 0x14, 0x60, 0x00, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 928, - .packetStatusCount = 5, - .deltas = std::vector{63, 11, 21, 6, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0xA0, 0x00, 0x05, - 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x05, 0xFC, 0x2C, - 0x54, 0x18, 0x00, 0x01 - } - }, - { - .baseTimeRaw = 818997, - .baseTimeMs = 1126157632, - .baseSequence = 942, - .packetStatusCount = 6, - .deltas = std::vector{39, 13, 9, 5, 4, 13}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xAE, 0x00, 0x06, - 0x0C, 0x7F, 0x35, 0xA1, 0x20, 0x06, 0x9C, 0x34, - 0x24, 0x14, 0x10, 0x34 - } - }, - { - .baseTimeRaw = 821523, - .baseTimeMs = 1126319296, - .baseSequence = 10, - .packetStatusCount = 7, - .deltas = std::vector{25, 2, 2, 3, 1, 1, 3}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x00, 0x00, 0x04, 0xD2, 0x00, 0x0A, 0x00, 0x07, - 0x0C, 0x89, 0x13, 0x00, 0x20, 0x07, 0x64, 0x08, - 0x08, 0x0C, 0x04, 0x04, 0x0C, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 821524, - .baseTimeMs = 1126319360, - .baseSequence = 17, - .packetStatusCount = 2, - .deltas = std::vector{44, 18}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, - 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 - } - }, - { - .baseTimeRaw = 821524, - .baseTimeMs = 1126319360, - .baseSequence = 17, - .packetStatusCount = 1, - .deltas = std::vector{62}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, - 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 - } - }, - { - .baseTimeRaw = 821526, - .baseTimeMs = 1126319488, - .baseSequence = 19, - .packetStatusCount = 4, - .deltas = std::vector{4, 0, 4, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x13, 0x00, 0x04, - 0x0C, 0x89, 0x16, 0x02, 0x20, 0x04, 0x10, 0x00, - 0x10, 0x00, 0x00, 0x02 - } - } - }; - - for (const auto &packetMeta: feedbackPacketsMeta) { - auto buffer = packetMeta.buffer; - auto *feedback = FeedbackRtpTransportPacket::Parse(buffer.data(), buffer.size()); - - REQUIRE(feedback->GetReferenceTime() == packetMeta.baseTimeRaw); - REQUIRE(feedback->GetReferenceTimestamp() == packetMeta.baseTimeMs); - REQUIRE(feedback->GetBaseSequenceNumber() == packetMeta.baseSequence); - REQUIRE(feedback->GetPacketStatusCount() == packetMeta.packetStatusCount); - - auto packetsResults = feedback->GetPacketResults(); - - int deltasIt = 0; - for (auto const &delta : packetMeta.deltas) { - auto resultDelta = packetsResults[deltasIt].delta; - REQUIRE(static_cast(resultDelta/ 4) == delta); - deltasIt++; - } - } - } + SECTION("parse FeedbackRtpTransportPacket generated by Chrome with libwebrtc as a reference") + { + typedef struct + { + uint32_t baseTimeRaw; + uint64_t baseTimeMs; + uint16_t baseSequence; + size_t packetStatusCount; + std::vector deltas; + std::vector buffer; + } FeedbackPacketsMeta; + + // Metadata collected by parsing buffers with libwebrtc, buffers itself. + // were generated by chrome in direction of mediasoup. + std::vector feedbackPacketsMeta = { + { .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 13, + .packetStatusCount = 1, + .deltas = std::vector{ 57 }, + .buffer = std::vector{ 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, + 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, + 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 } }, + { .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 14, + .packetStatusCount = 4, + .deltas = std::vector{ 58, 2, 3, 55 }, + .buffer = std::vector{ 0xaf, 0xcd, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x1C, 0xB7, + 0xDA, 0xF3, 0x00, 0x0E, 0x00, 0x04, 0x00, 0x8A, 0xB0, 0x01, + 0x20, 0x04, 0xE8, 0x08, 0x0C, 0xDC, 0x00, 0x02 } }, + { .baseTimeRaw = 35505, + .baseTimeMs = 1076014144, + .baseSequence = 18, + .packetStatusCount = 5, + .deltas = std::vector{ 60, 6, 5, 9, 22 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x1C, 0xB7, + 0xDA, 0xF3, 0x00, 0x12, 0x00, 0x05, 0x00, 0x8A, 0xB1, 0x02, + 0x20, 0x05, 0xF0, 0x18, 0x14, 0x24, 0x58, 0x01 } }, + + { .baseTimeRaw = 617873, + .baseTimeMs = 1113285696, + .baseSequence = 2924, + .packetStatusCount = 22, + .deltas = + std::vector{ 3, 5, 5, 0, 10, 0, 0, 4, 0, 1, 0, 2, 0, 2, 0, 2, 0, 2, 0, 1, 0, 4 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x0A, 0xFA, 0x17, 0xFA, 0x17, 0x06, + 0xF5, 0x11, 0x4C, 0x0B, 0x6C, 0x00, 0x16, 0x09, 0x6D, + 0x91, 0xEE, 0x20, 0x16, 0x0C, 0x14, 0x14, 0x00, 0x28, + 0x00, 0x00, 0x10, 0x00, 0x04, 0x00, 0x08, 0x00, 0x08, + 0x00, 0x08, 0x00, 0x08, 0x00, 0x04, 0x00, 0x10 } }, + + { .baseTimeRaw = 12408746, + .baseTimeMs = 1867901568, + .baseSequence = 1, + .packetStatusCount = 2, + .deltas = std::vector{ 35, 17 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, + 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 } }, + + { .baseTimeRaw = 818995, + .baseTimeMs = 1126157504, + .baseSequence = 930, + .packetStatusCount = 5, + .deltas = std::vector{ 62, 18, 5, 6, 19 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, + 0x8E, 0x50, 0x03, 0xA2, 0x00, 0x05, 0x0C, 0x7F, 0x33, 0x9F, + 0x20, 0x05, 0xF8, 0x48, 0x14, 0x18, 0x4C, 0x01 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 921, + .packetStatusCount = 7, + .deltas = std::vector{ 14, 5, 6, 6, 7, 14, 5 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x33, 0xB0, 0x4A, + 0xE8, 0x03, 0x99, 0x00, 0x07, 0x0C, 0x7F, 0x34, 0x9F, 0x20, 0x07, + 0x38, 0x14, 0x18, 0x18, 0x1C, 0x38, 0x14, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 935, + .packetStatusCount = 7, + .deltas = std::vector{ 57, 0, 6, 5, 5, 24, 0 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, 0x8E, + 0x50, 0x03, 0xA7, 0x00, 0x07, 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x07, + 0xE4, 0x00, 0x18, 0x14, 0x14, 0x60, 0x00, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 928, + .packetStatusCount = 5, + .deltas = std::vector{ 63, 11, 21, 6, 0 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x33, 0xB0, + 0x4A, 0xE8, 0x03, 0xA0, 0x00, 0x05, 0x0C, 0x7F, 0x34, 0xA0, + 0x20, 0x05, 0xFC, 0x2C, 0x54, 0x18, 0x00, 0x01 } }, + { .baseTimeRaw = 818997, + .baseTimeMs = 1126157632, + .baseSequence = 942, + .packetStatusCount = 6, + .deltas = std::vector{ 39, 13, 9, 5, 4, 13 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, + 0x8E, 0x50, 0x03, 0xAE, 0x00, 0x06, 0x0C, 0x7F, 0x35, 0xA1, + 0x20, 0x06, 0x9C, 0x34, 0x24, 0x14, 0x10, 0x34 } }, + { .baseTimeRaw = 821523, + .baseTimeMs = 1126319296, + .baseSequence = 10, + .packetStatusCount = 7, + .deltas = std::vector{ 25, 2, 2, 3, 1, 1, 3 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x00, 0x00, 0x04, + 0xD2, 0x00, 0x0A, 0x00, 0x07, 0x0C, 0x89, 0x13, 0x00, 0x20, 0x07, + 0x64, 0x08, 0x08, 0x0C, 0x04, 0x04, 0x0C, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 2, + .deltas = std::vector{ 44, 18 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, + 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 } }, + { .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 1, + .deltas = std::vector{ 62 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, + 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 } }, + { .baseTimeRaw = 821526, + .baseTimeMs = 1126319488, + .baseSequence = 19, + .packetStatusCount = 4, + .deltas = std::vector{ 4, 0, 4, 0 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x08, 0xEB, + 0x06, 0xD7, 0x00, 0x13, 0x00, 0x04, 0x0C, 0x89, 0x16, 0x02, + 0x20, 0x04, 0x10, 0x00, 0x10, 0x00, 0x00, 0x02 } } + }; + + for (const auto& packetMeta : feedbackPacketsMeta) + { + auto buffer = packetMeta.buffer; + auto* feedback = FeedbackRtpTransportPacket::Parse(buffer.data(), buffer.size()); + + REQUIRE(feedback->GetReferenceTime() == packetMeta.baseTimeRaw); + REQUIRE(feedback->GetReferenceTimestamp() == packetMeta.baseTimeMs); + REQUIRE(feedback->GetBaseSequenceNumber() == packetMeta.baseSequence); + REQUIRE(feedback->GetPacketStatusCount() == packetMeta.packetStatusCount); + + auto packetsResults = feedback->GetPacketResults(); + + int deltasIt = 0; + for (auto const& delta : packetMeta.deltas) + { + auto resultDelta = packetsResults[deltasIt].delta; + REQUIRE(static_cast(resultDelta / 4) == delta); + deltasIt++; + } + } + } } From e3f02d1b43378cd8fdec0a695838244635b48aa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Tue, 30 Aug 2022 19:19:52 +0200 Subject: [PATCH 03/12] Update worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp --- worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index dbc62b0cf9..e2a4e6e39e 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -585,7 +585,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" } FeedbackPacketsMeta; // Metadata collected by parsing buffers with libwebrtc, buffers itself. - // were generated by chrome in direction of mediasoup. + // Were generated by chrome in direction of mediasoup. std::vector feedbackPacketsMeta = { { .baseTimeRaw = 35504, .baseTimeMs = 1076014080, From 112e1d861f08a02d951cf1301948b10683a1f019 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 30 Aug 2022 19:31:42 +0300 Subject: [PATCH 04/12] Fix Formatting. --- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 377 ++++++++---------- 1 file changed, 159 insertions(+), 218 deletions(-) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 93e47e6b2c..dbc62b0cf9 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -572,222 +572,163 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" delete packet; } - SECTION("parse FeedbackRtpTransportPacket generated by Chrome with libwebrtc as a reference") - { - typedef struct { - uint32_t baseTimeRaw; - uint64_t baseTimeMs; - uint16_t baseSequence; - size_t packetStatusCount; - std::vector deltas; - std::vector buffer; - } FeedbackPacketsMeta; - - // Metadata collected by parsing buffers with libwebrtc, buffers itself. - // were generated by chrome in direction of mediasoup. - std::vector feedbackPacketsMeta = { - { - .baseTimeRaw = 35504, - .baseTimeMs = 1076014080, - .baseSequence = 13, - .packetStatusCount = 1, - .deltas = std::vector{57}, - .buffer = std::vector { - 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, - 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, - 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 - } - }, - { - .baseTimeRaw = 35504, - .baseTimeMs = 1076014080, - .baseSequence = 14, - .packetStatusCount = 4, - .deltas = std::vector{58, 2, 3, 55}, - .buffer = std::vector { - 0xaf, 0xcd, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x0E, 0x00, 0x04, - 0x00, 0x8A, 0xB0, 0x01, 0x20, 0x04, 0xE8, 0x08, - 0x0C, 0xDC, 0x00, 0x02 - } - }, - { - .baseTimeRaw = 35505, - .baseTimeMs = 1076014144, - .baseSequence = 18, - .packetStatusCount = 5, - .deltas = std::vector{60, 6, 5, 9, 22}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x1C, 0xB7, 0xDA, 0xF3, 0x00, 0x12, 0x00, 0x05, - 0x00, 0x8A, 0xB1, 0x02, 0x20, 0x05, 0xF0, 0x18, - 0x14, 0x24, 0x58, 0x01 - } - }, - - { - .baseTimeRaw = 617873, - .baseTimeMs = 1113285696, - .baseSequence = 2924, - .packetStatusCount = 22, - .deltas = std::vector{3, 5, 5, 0, 10, 0, 0, 4, 0, 1, 0, 2, 0, 2, 0, 2, 0, 2, 0, 1, 0, 4}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x0A, 0xFA, 0x17, 0xFA, 0x17, - 0x06, 0xF5, 0x11, 0x4C, 0x0B, 0x6C, 0x00, 0x16, - 0x09, 0x6D, 0x91, 0xEE, 0x20, 0x16, 0x0C, 0x14, - 0x14, 0x00, 0x28, 0x00, 0x00, 0x10, 0x00, 0x04, - 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, - 0x00, 0x04, 0x00, 0x10 - } - }, - - { - .baseTimeRaw = 12408746, - .baseTimeMs = 1867901568, - .baseSequence = 1, - .packetStatusCount = 2, - .deltas = std::vector{35, 17}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, - 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 - } - }, - - { - .baseTimeRaw = 818995, - .baseTimeMs = 1126157504, - .baseSequence = 930, - .packetStatusCount = 5, - .deltas = std::vector{62, 18, 5, 6, 19}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA2, 0x00, 0x05, - 0x0C, 0x7F, 0x33, 0x9F, 0x20, 0x05, 0xF8, 0x48, - 0x14, 0x18, 0x4C, 0x01 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 921, - .packetStatusCount = 7, - .deltas = std::vector{14, 5, 6, 6, 7, 14, 5}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0x99, 0x00, 0x07, - 0x0C, 0x7F, 0x34, 0x9F, 0x20, 0x07, 0x38, 0x14, - 0x18, 0x18, 0x1C, 0x38, 0x14, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 935, - .packetStatusCount = 7, - .deltas = std::vector{57, 0, 6, 5, 5, 24, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xA7, 0x00, 0x07, - 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x07, 0xE4, 0x00, - 0x18, 0x14, 0x14, 0x60, 0x00, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 818996, - .baseTimeMs = 1126157568, - .baseSequence = 928, - .packetStatusCount = 5, - .deltas = std::vector{63, 11, 21, 6, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x33, 0xB0, 0x4A, 0xE8, 0x03, 0xA0, 0x00, 0x05, - 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x05, 0xFC, 0x2C, - 0x54, 0x18, 0x00, 0x01 - } - }, - { - .baseTimeRaw = 818997, - .baseTimeMs = 1126157632, - .baseSequence = 942, - .packetStatusCount = 6, - .deltas = std::vector{39, 13, 9, 5, 4, 13}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x26, 0x9E, 0x8E, 0x50, 0x03, 0xAE, 0x00, 0x06, - 0x0C, 0x7F, 0x35, 0xA1, 0x20, 0x06, 0x9C, 0x34, - 0x24, 0x14, 0x10, 0x34 - } - }, - { - .baseTimeRaw = 821523, - .baseTimeMs = 1126319296, - .baseSequence = 10, - .packetStatusCount = 7, - .deltas = std::vector{25, 2, 2, 3, 1, 1, 3}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, - 0x00, 0x00, 0x04, 0xD2, 0x00, 0x0A, 0x00, 0x07, - 0x0C, 0x89, 0x13, 0x00, 0x20, 0x07, 0x64, 0x08, - 0x08, 0x0C, 0x04, 0x04, 0x0C, 0x00, 0x00, 0x03 - } - }, - { - .baseTimeRaw = 821524, - .baseTimeMs = 1126319360, - .baseSequence = 17, - .packetStatusCount = 2, - .deltas = std::vector{44, 18}, - .buffer = std::vector { - 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, - 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 - } - }, - { - .baseTimeRaw = 821524, - .baseTimeMs = 1126319360, - .baseSequence = 17, - .packetStatusCount = 1, - .deltas = std::vector{62}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, - 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 - } - }, - { - .baseTimeRaw = 821526, - .baseTimeMs = 1126319488, - .baseSequence = 19, - .packetStatusCount = 4, - .deltas = std::vector{4, 0, 4, 0}, - .buffer = std::vector { - 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, - 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x13, 0x00, 0x04, - 0x0C, 0x89, 0x16, 0x02, 0x20, 0x04, 0x10, 0x00, - 0x10, 0x00, 0x00, 0x02 - } - } - }; - - for (const auto &packetMeta: feedbackPacketsMeta) { - auto buffer = packetMeta.buffer; - auto *feedback = FeedbackRtpTransportPacket::Parse(buffer.data(), buffer.size()); - - REQUIRE(feedback->GetReferenceTime() == packetMeta.baseTimeRaw); - REQUIRE(feedback->GetReferenceTimestamp() == packetMeta.baseTimeMs); - REQUIRE(feedback->GetBaseSequenceNumber() == packetMeta.baseSequence); - REQUIRE(feedback->GetPacketStatusCount() == packetMeta.packetStatusCount); - - auto packetsResults = feedback->GetPacketResults(); - - int deltasIt = 0; - for (auto const &delta : packetMeta.deltas) { - auto resultDelta = packetsResults[deltasIt].delta; - REQUIRE(static_cast(resultDelta/ 4) == delta); - deltasIt++; - } - } - } + SECTION("parse FeedbackRtpTransportPacket generated by Chrome with libwebrtc as a reference") + { + typedef struct + { + uint32_t baseTimeRaw; + uint64_t baseTimeMs; + uint16_t baseSequence; + size_t packetStatusCount; + std::vector deltas; + std::vector buffer; + } FeedbackPacketsMeta; + + // Metadata collected by parsing buffers with libwebrtc, buffers itself. + // were generated by chrome in direction of mediasoup. + std::vector feedbackPacketsMeta = { + { .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 13, + .packetStatusCount = 1, + .deltas = std::vector{ 57 }, + .buffer = std::vector{ 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, + 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, + 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 } }, + { .baseTimeRaw = 35504, + .baseTimeMs = 1076014080, + .baseSequence = 14, + .packetStatusCount = 4, + .deltas = std::vector{ 58, 2, 3, 55 }, + .buffer = std::vector{ 0xaf, 0xcd, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x1C, 0xB7, + 0xDA, 0xF3, 0x00, 0x0E, 0x00, 0x04, 0x00, 0x8A, 0xB0, 0x01, + 0x20, 0x04, 0xE8, 0x08, 0x0C, 0xDC, 0x00, 0x02 } }, + { .baseTimeRaw = 35505, + .baseTimeMs = 1076014144, + .baseSequence = 18, + .packetStatusCount = 5, + .deltas = std::vector{ 60, 6, 5, 9, 22 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x1C, 0xB7, + 0xDA, 0xF3, 0x00, 0x12, 0x00, 0x05, 0x00, 0x8A, 0xB1, 0x02, + 0x20, 0x05, 0xF0, 0x18, 0x14, 0x24, 0x58, 0x01 } }, + + { .baseTimeRaw = 617873, + .baseTimeMs = 1113285696, + .baseSequence = 2924, + .packetStatusCount = 22, + .deltas = + std::vector{ 3, 5, 5, 0, 10, 0, 0, 4, 0, 1, 0, 2, 0, 2, 0, 2, 0, 2, 0, 1, 0, 4 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x0A, 0xFA, 0x17, 0xFA, 0x17, 0x06, + 0xF5, 0x11, 0x4C, 0x0B, 0x6C, 0x00, 0x16, 0x09, 0x6D, + 0x91, 0xEE, 0x20, 0x16, 0x0C, 0x14, 0x14, 0x00, 0x28, + 0x00, 0x00, 0x10, 0x00, 0x04, 0x00, 0x08, 0x00, 0x08, + 0x00, 0x08, 0x00, 0x08, 0x00, 0x04, 0x00, 0x10 } }, + + { .baseTimeRaw = 12408746, + .baseTimeMs = 1867901568, + .baseSequence = 1, + .packetStatusCount = 2, + .deltas = std::vector{ 35, 17 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, + 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 } }, + + { .baseTimeRaw = 818995, + .baseTimeMs = 1126157504, + .baseSequence = 930, + .packetStatusCount = 5, + .deltas = std::vector{ 62, 18, 5, 6, 19 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, + 0x8E, 0x50, 0x03, 0xA2, 0x00, 0x05, 0x0C, 0x7F, 0x33, 0x9F, + 0x20, 0x05, 0xF8, 0x48, 0x14, 0x18, 0x4C, 0x01 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 921, + .packetStatusCount = 7, + .deltas = std::vector{ 14, 5, 6, 6, 7, 14, 5 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x33, 0xB0, 0x4A, + 0xE8, 0x03, 0x99, 0x00, 0x07, 0x0C, 0x7F, 0x34, 0x9F, 0x20, 0x07, + 0x38, 0x14, 0x18, 0x18, 0x1C, 0x38, 0x14, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 935, + .packetStatusCount = 7, + .deltas = std::vector{ 57, 0, 6, 5, 5, 24, 0 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, 0x8E, + 0x50, 0x03, 0xA7, 0x00, 0x07, 0x0C, 0x7F, 0x34, 0xA0, 0x20, 0x07, + 0xE4, 0x00, 0x18, 0x14, 0x14, 0x60, 0x00, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 818996, + .baseTimeMs = 1126157568, + .baseSequence = 928, + .packetStatusCount = 5, + .deltas = std::vector{ 63, 11, 21, 6, 0 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x33, 0xB0, + 0x4A, 0xE8, 0x03, 0xA0, 0x00, 0x05, 0x0C, 0x7F, 0x34, 0xA0, + 0x20, 0x05, 0xFC, 0x2C, 0x54, 0x18, 0x00, 0x01 } }, + { .baseTimeRaw = 818997, + .baseTimeMs = 1126157632, + .baseSequence = 942, + .packetStatusCount = 6, + .deltas = std::vector{ 39, 13, 9, 5, 4, 13 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x26, 0x9E, + 0x8E, 0x50, 0x03, 0xAE, 0x00, 0x06, 0x0C, 0x7F, 0x35, 0xA1, + 0x20, 0x06, 0x9C, 0x34, 0x24, 0x14, 0x10, 0x34 } }, + { .baseTimeRaw = 821523, + .baseTimeMs = 1126319296, + .baseSequence = 10, + .packetStatusCount = 7, + .deltas = std::vector{ 25, 2, 2, 3, 1, 1, 3 }, + .buffer = + std::vector{ 0xAF, 0xCD, 0x00, 0x07, 0xFA, 0x17, 0xFA, 0x17, 0x00, 0x00, 0x04, + 0xD2, 0x00, 0x0A, 0x00, 0x07, 0x0C, 0x89, 0x13, 0x00, 0x20, 0x07, + 0x64, 0x08, 0x08, 0x0C, 0x04, 0x04, 0x0C, 0x00, 0x00, 0x03 } }, + { .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 2, + .deltas = std::vector{ 44, 18 }, + .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, + 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 } }, + { .baseTimeRaw = 821524, + .baseTimeMs = 1126319360, + .baseSequence = 17, + .packetStatusCount = 1, + .deltas = std::vector{ 62 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, + 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, + 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 } }, + { .baseTimeRaw = 821526, + .baseTimeMs = 1126319488, + .baseSequence = 19, + .packetStatusCount = 4, + .deltas = std::vector{ 4, 0, 4, 0 }, + .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x06, 0xFA, 0x17, 0xFA, 0x17, 0x08, 0xEB, + 0x06, 0xD7, 0x00, 0x13, 0x00, 0x04, 0x0C, 0x89, 0x16, 0x02, + 0x20, 0x04, 0x10, 0x00, 0x10, 0x00, 0x00, 0x02 } } + }; + + for (const auto& packetMeta : feedbackPacketsMeta) + { + auto buffer = packetMeta.buffer; + auto* feedback = FeedbackRtpTransportPacket::Parse(buffer.data(), buffer.size()); + + REQUIRE(feedback->GetReferenceTime() == packetMeta.baseTimeRaw); + REQUIRE(feedback->GetReferenceTimestamp() == packetMeta.baseTimeMs); + REQUIRE(feedback->GetBaseSequenceNumber() == packetMeta.baseSequence); + REQUIRE(feedback->GetPacketStatusCount() == packetMeta.packetStatusCount); + + auto packetsResults = feedback->GetPacketResults(); + + int deltasIt = 0; + for (auto const& delta : packetMeta.deltas) + { + auto resultDelta = packetsResults[deltasIt].delta; + REQUIRE(static_cast(resultDelta / 4) == delta); + deltasIt++; + } + } + } } From 3379c43d3a430ba43dd175f48735e434f41d4df5 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 6 Sep 2022 19:11:59 +0300 Subject: [PATCH 05/12] Get rid of negative reference_time, make reference timestamp to be represented as in libwebrtc. --- .../include/RTC/RTCP/FeedbackRtpTransport.hpp | 5 ++- worker/src/RTC/RTCP/FeedbackRtpTransport.cpp | 42 +------------------ 2 files changed, 5 insertions(+), 42 deletions(-) diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index d23ea69dec..67ef7abd16 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -41,6 +41,9 @@ namespace RTC { class FeedbackRtpTransportPacket : public FeedbackRtpPacket { + static constexpr int64_t kBaseTimeTick = 64; + static constexpr int64_t kTimeWrapPeriod = kBaseTimeTick * (1ll << 24); + public: struct PacketResult { @@ -239,7 +242,7 @@ namespace RTC } int64_t GetReferenceTimestamp() const // Reference time in ms. { - return static_cast(this->referenceTime) * 64; + return kTimeWrapPeriod + static_cast(this->referenceTime) * kBaseTimeTick; } uint8_t GetFeedbackPacketCount() const { diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index d310110094..e35823d82f 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp @@ -8,46 +8,6 @@ #include // std::numeric_limits() #include -// Code taken and adapted from libwebrtc (byte_io.h). -inline static int32_t parseReferenceTime(uint8_t* buffer) -{ - int32_t referenceTime; - uint32_t unsignedVal = Utils::Byte::Get3Bytes(buffer, 0); - - const auto msb = static_cast(unsignedVal >> ((3 - 1) * 8)); - - if ((msb & 0x80) != 0) - { - // Create a mask where all bits used by the 3 bytes are set to one, for - // instance 0x00FFFFFF. Bit-wise inverts that mask to 0xFF000000 and adds - // it to the input value. - static uint32_t usedBitMask = (1 << ((3 % sizeof(int32_t)) * 8)) - 1; - - unsignedVal = ~usedBitMask | unsignedVal; - } - - // An unsigned value with only the highest order bit set (ex 0x80). - static uint32_t unsignedHighestBitMask = static_cast(1) << ((sizeof(uint32_t) * 8) - 1); - - // A signed value with only the highest bit set. Since this is two's - // complement form, we can use the min value from std::numeric_limits. - static int32_t signedHighestBitMask = std::numeric_limits::min(); - - if ((unsignedVal & unsignedHighestBitMask) != 0) - { - // Casting is only safe when unsigned value can be represented in the - // signed target type, so mask out highest bit and mask it back manually. - referenceTime = static_cast(unsignedVal & ~unsignedHighestBitMask); - referenceTime |= signedHighestBitMask; - } - else - { - referenceTime = static_cast(unsignedVal); - } - - return referenceTime; -} - namespace RTC { namespace RTCP @@ -117,7 +77,7 @@ namespace RTC this->baseSequenceNumber = Utils::Byte::Get2Bytes(data, 0); this->packetStatusCount = Utils::Byte::Get2Bytes(data, 2); - this->referenceTime = parseReferenceTime(data + 4); + this->referenceTime = Utils::Byte::Get3Bytes(data, 4); this->feedbackPacketCount = Utils::Byte::Get1Byte(data, 7); this->size = len; From 2b5371440bed4300b25fb3774dc0b4ddb1484557 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 6 Sep 2022 21:12:05 +0300 Subject: [PATCH 06/12] Fix tests. Adopt changes to GetBaseDelta from https://github.com/versatica/mediasoup/pull/900 Add tests for GetBaseDelta Wraparound. --- .../libwebrtc/libwebrtc/mediasoup_helpers.h | 2 +- .../include/RTC/RTCP/FeedbackRtpTransport.hpp | 25 +++++++- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 58 +++++++++++++++---- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/worker/deps/libwebrtc/libwebrtc/mediasoup_helpers.h b/worker/deps/libwebrtc/libwebrtc/mediasoup_helpers.h index 4b6673a65f..cf84bf3974 100644 --- a/worker/deps/libwebrtc/libwebrtc/mediasoup_helpers.h +++ b/worker/deps/libwebrtc/libwebrtc/mediasoup_helpers.h @@ -39,7 +39,7 @@ namespace mediasoup_helpers int64_t GetBaseDeltaUs( const RTC::RTCP::FeedbackRtpTransportPacket* packet, int64_t prev_timestamp_us) { - return GetBaseTimeUs(packet) - prev_timestamp_us; + return packet->GetBaseDelta(prev_timestamp_us / 1000) * 1000; } } // namespace FeedbackRtpTransport } // namespace mediasoup_helpers diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 67ef7abd16..275fdd6b21 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -41,10 +41,9 @@ namespace RTC { class FeedbackRtpTransportPacket : public FeedbackRtpPacket { + public: static constexpr int64_t kBaseTimeTick = 64; static constexpr int64_t kTimeWrapPeriod = kBaseTimeTick * (1ll << 24); - - public: struct PacketResult { PacketResult(uint16_t sequenceNumber, bool received) @@ -240,10 +239,30 @@ namespace RTC { return this->referenceTime; } + // We only use this for testing purpose + void SetReferenceTime(uint64_t referenceTime) + { + this->referenceTime = (referenceTime % kTimeWrapPeriod) / kBaseTimeTick; + } int64_t GetReferenceTimestamp() const // Reference time in ms. { return kTimeWrapPeriod + static_cast(this->referenceTime) * kBaseTimeTick; } + int64_t GetBaseDelta(const int64_t previousTimestampMs) const + { + int64_t delta = GetReferenceTimestamp() - previousTimestampMs; + // Compensate for wrap around. + if (std::abs(delta - kTimeWrapPeriod) < std::abs(delta)) + { + delta -= kTimeWrapPeriod; + } + else if (std::abs(delta + kTimeWrapPeriod) < std::abs(delta)) + { + delta += kTimeWrapPeriod; + } + + return delta; + } uint8_t GetFeedbackPacketCount() const { return this->feedbackPacketCount; @@ -293,7 +312,7 @@ namespace RTC private: uint16_t baseSequenceNumber{ 0u }; - int32_t referenceTime{ 0 }; + uint32_t referenceTime{ 0 }; uint16_t latestSequenceNumber{ 0u }; // Just for locally generated packets. uint64_t latestTimestamp{ 0u }; // Just for locally generated packets. uint16_t packetStatusCount{ 0u }; diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index dbc62b0cf9..6ff08a0929 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -476,7 +476,10 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetBaseSequenceNumber() == 39); REQUIRE(packet->GetPacketStatusCount() == 13); REQUIRE(packet->GetReferenceTime() == 6275825); // 0x5FC2F1 (signed 24 bits) - REQUIRE(packet->GetReferenceTimestamp() == 6275825 * 64); + REQUIRE( + packet->GetReferenceTimestamp() == + FeedbackRtpTransportPacket::kTimeWrapPeriod + + static_cast(6275825) * FeedbackRtpTransportPacket::kBaseTimeTick); REQUIRE(packet->GetFeedbackPacketCount() == 3); SECTION("serialize packet") @@ -510,8 +513,11 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetSize() == sizeof(data)); REQUIRE(packet->GetBaseSequenceNumber() == 39); REQUIRE(packet->GetPacketStatusCount() == 0); - REQUIRE(packet->GetReferenceTime() == -2); // 0xFFFFFE (signed 24 bits) - REQUIRE(packet->GetReferenceTimestamp() == -2 * 64); + REQUIRE(packet->GetReferenceTime() == 16777214); // 0xFFFFFE (unsigned 24 bits) + REQUIRE( + packet->GetReferenceTimestamp() == + FeedbackRtpTransportPacket::kTimeWrapPeriod + + static_cast(16777214) * FeedbackRtpTransportPacket::kBaseTimeTick); REQUIRE(packet->GetFeedbackPacketCount() == 1); SECTION("serialize packet") @@ -546,18 +552,15 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetSize() == sizeof(data)); REQUIRE(packet->GetBaseSequenceNumber() == 1); REQUIRE(packet->GetPacketStatusCount() == 2); - REQUIRE(packet->GetReferenceTime() == -4368470); - REQUIRE(packet->GetReferenceTimestamp() == -4368470 * 64); + REQUIRE(packet->GetReferenceTime() == 12408746); + REQUIRE( + packet->GetReferenceTimestamp() == + FeedbackRtpTransportPacket::kTimeWrapPeriod + + static_cast(12408746) * FeedbackRtpTransportPacket::kBaseTimeTick); // Let's also test the reference time reported by Wireshark. int32_t wiresharkValue{ 12408746 }; - // Constraint it to signed 24 bits. - wiresharkValue = wiresharkValue << 8; - wiresharkValue = wiresharkValue >> 8; - - REQUIRE(packet->GetReferenceTime() == wiresharkValue); - REQUIRE(packet->GetReferenceTimestamp() == wiresharkValue * 64); REQUIRE(packet->GetFeedbackPacketCount() == 0); SECTION("serialize packet") @@ -729,6 +732,39 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(static_cast(resultDelta / 4) == delta); deltasIt++; } + delete feedback; } } + + SECTION("Check GetBaseDelta Wraparound") + { + auto kMaxBaseTime = + FeedbackRtpTransportPacket::kTimeWrapPeriod - FeedbackRtpTransportPacket::kBaseTimeTick; + auto* packet1 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); + auto* packet2 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); + auto* packet3 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); + + packet1->SetReferenceTime(kMaxBaseTime); + packet2->SetReferenceTime(kMaxBaseTime + FeedbackRtpTransportPacket::kBaseTimeTick); + packet3->SetReferenceTime( + kMaxBaseTime + FeedbackRtpTransportPacket::kBaseTimeTick + + FeedbackRtpTransportPacket::kBaseTimeTick); + + REQUIRE(packet1->GetReferenceTime() == 16777215); + REQUIRE(packet2->GetReferenceTime() == 0); + REQUIRE(packet3->GetReferenceTime() == 1); + + REQUIRE(packet1->GetReferenceTimestamp() == 2147483584); + REQUIRE(packet2->GetReferenceTimestamp() == 1073741824); + REQUIRE(packet3->GetReferenceTimestamp() == 1073741888); + + REQUIRE(packet1->GetBaseDelta(packet1->GetReferenceTimestamp()) == 0); + REQUIRE(packet2->GetBaseDelta(packet1->GetReferenceTimestamp()) == 64); + REQUIRE(packet3->GetBaseDelta(packet2->GetReferenceTimestamp()) == 64); + REQUIRE(packet3->GetBaseDelta(packet1->GetReferenceTimestamp()) == 128); + + delete packet1; + delete packet2; + delete packet3; + } } From b4b0ac8758ded1b6099b40ccb585d5a3ad4a8ad0 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 6 Sep 2022 21:14:40 +0300 Subject: [PATCH 07/12] Fix tests. Adopt changes to GetBaseDelta from https://github.com/versatica/mediasoup/pull/900 Add tests for GetBaseDelta Wraparound. --- .../remote_bitrate_estimator/inter_arrival.cc | 81 ++++++++++--------- .../remote_bitrate_estimator/inter_arrival.h | 12 +-- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc index 46e2df75dc..1d8b4ba06a 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc @@ -42,24 +42,32 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, MS_ASSERT(arrival_time_delta_ms != nullptr, "arrival_time_delta_ms is null"); MS_ASSERT(packet_size_delta != nullptr, "packet_size_delta is null"); bool calculated_deltas = false; - if (current_timestamp_group_.IsFirstPacket()) { - // We don't have enough data to update the filter, so we store it until we - // have two frames of data to process. - current_timestamp_group_.timestamp = timestamp; - current_timestamp_group_.first_timestamp = timestamp; - current_timestamp_group_.first_arrival_ms = arrival_time_ms; - } else if (!PacketInOrder(timestamp, arrival_time_ms)) { - return false; - } else if (NewTimestampGroup(arrival_time_ms, timestamp)) { - // First packet of a later frame, the previous frame sample is ready. - if (prev_timestamp_group_.complete_time_ms >= 0) { - *timestamp_delta = - current_timestamp_group_.timestamp - prev_timestamp_group_.timestamp; - *arrival_time_delta_ms = current_timestamp_group_.complete_time_ms - - prev_timestamp_group_.complete_time_ms; - MS_DEBUG_DEV("timestamp previous/current [%" PRIu32 "/%" PRIu32"] complete time previous/current [%" PRIi64 "/%" PRIi64 "]", - prev_timestamp_group_.timestamp, current_timestamp_group_.timestamp, - prev_timestamp_group_.complete_time_ms, current_timestamp_group_.complete_time_ms); + if (current_timestamp_group_.IsFirstPacket()) + { + // We don't have enough data to update the filter, so we store it until we + // have two frames of data to process. + current_timestamp_group_.timestamp = timestamp; + current_timestamp_group_.first_timestamp = timestamp; + current_timestamp_group_.first_arrival_ms = arrival_time_ms; + } + else if (!PacketInOrder(timestamp)) + { + return false; + } + else if (NewTimestampGroup(arrival_time_ms, timestamp)) + { + // First packet of a later frame, the previous frame sample is ready. + if (prev_timestamp_group_.complete_time_ms >= 0) + { + *timestamp_delta = current_timestamp_group_.timestamp - prev_timestamp_group_.timestamp; + *arrival_time_delta_ms = + current_timestamp_group_.complete_time_ms - prev_timestamp_group_.complete_time_ms; + MS_DEBUG_DEV( + "timestamp previous/current [%" PRIu32 "/%" PRIu32 + "] complete time previous/current [%" PRIi64 "/%" PRIi64 "]", + prev_timestamp_group_.timestamp, + current_timestamp_group_.timestamp, + prev_timestamp_group_.complete_time_ms, current_timestamp_group_.complete_time_ms); // Check system time differences to see if we have an unproportional jump // in arrival time. In that case reset the inter-arrival computations. int64_t system_time_delta_ms = @@ -115,27 +123,22 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, return calculated_deltas; } -bool InterArrival::PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms) { - if (current_timestamp_group_.IsFirstPacket()) { - return true; - } else if (arrival_time_ms < 0) { - // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 - // - // Sometimes we do get negative arrival time, which causes BelongsToBurst() - // to fail, which may cause anything that uses InterArrival to crash. - // - // Credits to @sspanak and @Ivaka. - return false; - } else { - // Assume that a diff which is bigger than half the timestamp interval - // (32 bits) must be due to reordering. This code is almost identical to - // that in IsNewerTimestamp() in module_common_types.h. - uint32_t timestamp_diff = - timestamp - current_timestamp_group_.first_timestamp; - - const static uint32_t int_middle = 0x80000000; - - if (timestamp_diff == int_middle) { +bool InterArrival::PacketInOrder(uint32_t timestamp) +{ + if (current_timestamp_group_.IsFirstPacket()) + { + return true; + } + else + { + // Assume that a diff which is bigger than half the timestamp interval + // (32 bits) must be due to reordering. This code is almost identical to + // that in IsNewerTimestamp() in module_common_types.h. + uint32_t timestamp_diff = timestamp - current_timestamp_group_.first_timestamp; + + const static uint32_t int_middle = 0x80000000; + + if (timestamp_diff == int_middle) { return timestamp > current_timestamp_group_.first_timestamp; } diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h index 1602a62bb7..504e11729d 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h @@ -70,14 +70,10 @@ class InterArrival { int64_t last_system_time_ms; }; - // Returns true if the packet with timestamp |timestamp| arrived in order. - // - // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 - // - // bool PacketInOrder(uint32_t timestamp); - bool PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms); - - // Returns true if the last packet was the end of the current batch and the + // Returns true if the packet with timestamp |timestamp| arrived in order. + bool PacketInOrder(uint32_t timestamp); + + // Returns true if the last packet was the end of the current batch and the // packet with |timestamp| is the first of a new batch. bool NewTimestampGroup(int64_t arrival_time_ms, uint32_t timestamp) const; From 39772536605832081ba66329c41dcae271843627 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 6 Sep 2022 21:24:28 +0300 Subject: [PATCH 08/12] Revert "Fix tests." This reverts commit b4b0ac8758ded1b6099b40ccb585d5a3ad4a8ad0. --- .../remote_bitrate_estimator/inter_arrival.cc | 81 +++++++++---------- .../remote_bitrate_estimator/inter_arrival.h | 12 ++- 2 files changed, 47 insertions(+), 46 deletions(-) diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc index 1d8b4ba06a..46e2df75dc 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc @@ -42,32 +42,24 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, MS_ASSERT(arrival_time_delta_ms != nullptr, "arrival_time_delta_ms is null"); MS_ASSERT(packet_size_delta != nullptr, "packet_size_delta is null"); bool calculated_deltas = false; - if (current_timestamp_group_.IsFirstPacket()) - { - // We don't have enough data to update the filter, so we store it until we - // have two frames of data to process. - current_timestamp_group_.timestamp = timestamp; - current_timestamp_group_.first_timestamp = timestamp; - current_timestamp_group_.first_arrival_ms = arrival_time_ms; - } - else if (!PacketInOrder(timestamp)) - { - return false; - } - else if (NewTimestampGroup(arrival_time_ms, timestamp)) - { - // First packet of a later frame, the previous frame sample is ready. - if (prev_timestamp_group_.complete_time_ms >= 0) - { - *timestamp_delta = current_timestamp_group_.timestamp - prev_timestamp_group_.timestamp; - *arrival_time_delta_ms = - current_timestamp_group_.complete_time_ms - prev_timestamp_group_.complete_time_ms; - MS_DEBUG_DEV( - "timestamp previous/current [%" PRIu32 "/%" PRIu32 - "] complete time previous/current [%" PRIi64 "/%" PRIi64 "]", - prev_timestamp_group_.timestamp, - current_timestamp_group_.timestamp, - prev_timestamp_group_.complete_time_ms, current_timestamp_group_.complete_time_ms); + if (current_timestamp_group_.IsFirstPacket()) { + // We don't have enough data to update the filter, so we store it until we + // have two frames of data to process. + current_timestamp_group_.timestamp = timestamp; + current_timestamp_group_.first_timestamp = timestamp; + current_timestamp_group_.first_arrival_ms = arrival_time_ms; + } else if (!PacketInOrder(timestamp, arrival_time_ms)) { + return false; + } else if (NewTimestampGroup(arrival_time_ms, timestamp)) { + // First packet of a later frame, the previous frame sample is ready. + if (prev_timestamp_group_.complete_time_ms >= 0) { + *timestamp_delta = + current_timestamp_group_.timestamp - prev_timestamp_group_.timestamp; + *arrival_time_delta_ms = current_timestamp_group_.complete_time_ms - + prev_timestamp_group_.complete_time_ms; + MS_DEBUG_DEV("timestamp previous/current [%" PRIu32 "/%" PRIu32"] complete time previous/current [%" PRIi64 "/%" PRIi64 "]", + prev_timestamp_group_.timestamp, current_timestamp_group_.timestamp, + prev_timestamp_group_.complete_time_ms, current_timestamp_group_.complete_time_ms); // Check system time differences to see if we have an unproportional jump // in arrival time. In that case reset the inter-arrival computations. int64_t system_time_delta_ms = @@ -123,22 +115,27 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, return calculated_deltas; } -bool InterArrival::PacketInOrder(uint32_t timestamp) -{ - if (current_timestamp_group_.IsFirstPacket()) - { - return true; - } - else - { - // Assume that a diff which is bigger than half the timestamp interval - // (32 bits) must be due to reordering. This code is almost identical to - // that in IsNewerTimestamp() in module_common_types.h. - uint32_t timestamp_diff = timestamp - current_timestamp_group_.first_timestamp; - - const static uint32_t int_middle = 0x80000000; - - if (timestamp_diff == int_middle) { +bool InterArrival::PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms) { + if (current_timestamp_group_.IsFirstPacket()) { + return true; + } else if (arrival_time_ms < 0) { + // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 + // + // Sometimes we do get negative arrival time, which causes BelongsToBurst() + // to fail, which may cause anything that uses InterArrival to crash. + // + // Credits to @sspanak and @Ivaka. + return false; + } else { + // Assume that a diff which is bigger than half the timestamp interval + // (32 bits) must be due to reordering. This code is almost identical to + // that in IsNewerTimestamp() in module_common_types.h. + uint32_t timestamp_diff = + timestamp - current_timestamp_group_.first_timestamp; + + const static uint32_t int_middle = 0x80000000; + + if (timestamp_diff == int_middle) { return timestamp > current_timestamp_group_.first_timestamp; } diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h index 504e11729d..1602a62bb7 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h @@ -70,10 +70,14 @@ class InterArrival { int64_t last_system_time_ms; }; - // Returns true if the packet with timestamp |timestamp| arrived in order. - bool PacketInOrder(uint32_t timestamp); - - // Returns true if the last packet was the end of the current batch and the + // Returns true if the packet with timestamp |timestamp| arrived in order. + // + // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 + // + // bool PacketInOrder(uint32_t timestamp); + bool PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms); + + // Returns true if the last packet was the end of the current batch and the // packet with |timestamp| is the first of a new batch. bool NewTimestampGroup(int64_t arrival_time_ms, uint32_t timestamp) const; From 31229cee609b8b1357862788a0b6ede3be2e5542 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Tue, 6 Sep 2022 21:29:17 +0300 Subject: [PATCH 09/12] Fix tests. Adopt changes to GetBaseDelta from https://github.com/versatica/mediasoup/pull/900 Add tests for GetBaseDelta Wraparound. --- .../remote_bitrate_estimator/inter_arrival.cc | 12 ++---------- .../modules/remote_bitrate_estimator/inter_arrival.h | 6 +----- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc index 46e2df75dc..daa9c861d2 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.cc @@ -48,7 +48,7 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, current_timestamp_group_.timestamp = timestamp; current_timestamp_group_.first_timestamp = timestamp; current_timestamp_group_.first_arrival_ms = arrival_time_ms; - } else if (!PacketInOrder(timestamp, arrival_time_ms)) { + } else if (!PacketInOrder(timestamp)) { return false; } else if (NewTimestampGroup(arrival_time_ms, timestamp)) { // First packet of a later frame, the previous frame sample is ready. @@ -115,17 +115,9 @@ bool InterArrival::ComputeDeltas(uint32_t timestamp, return calculated_deltas; } -bool InterArrival::PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms) { +bool InterArrival::PacketInOrder(uint32_t timestamp) { if (current_timestamp_group_.IsFirstPacket()) { return true; - } else if (arrival_time_ms < 0) { - // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 - // - // Sometimes we do get negative arrival time, which causes BelongsToBurst() - // to fail, which may cause anything that uses InterArrival to crash. - // - // Credits to @sspanak and @Ivaka. - return false; } else { // Assume that a diff which is bigger than half the timestamp interval // (32 bits) must be due to reordering. This code is almost identical to diff --git a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h index 1602a62bb7..8ce1c145d1 100644 --- a/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h +++ b/worker/deps/libwebrtc/libwebrtc/modules/remote_bitrate_estimator/inter_arrival.h @@ -71,11 +71,7 @@ class InterArrival { }; // Returns true if the packet with timestamp |timestamp| arrived in order. - // - // NOTE: Change related to https://github.com/versatica/mediasoup/issues/357 - // - // bool PacketInOrder(uint32_t timestamp); - bool PacketInOrder(uint32_t timestamp, int64_t arrival_time_ms); + bool PacketInOrder(uint32_t timestamp); // Returns true if the last packet was the end of the current batch and the // packet with |timestamp| is the first of a new batch. From 29dbc7d1293d0a62279ab39332f43c88f2f26d34 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Wed, 7 Sep 2022 12:49:35 +0300 Subject: [PATCH 10/12] Cosmetics. --- .../include/RTC/RTCP/FeedbackRtpTransport.hpp | 20 +++++++++------- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 24 +++++++++---------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 275fdd6b21..6b4f173358 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -42,8 +42,10 @@ namespace RTC class FeedbackRtpTransportPacket : public FeedbackRtpPacket { public: - static constexpr int64_t kBaseTimeTick = 64; - static constexpr int64_t kTimeWrapPeriod = kBaseTimeTick * (1ll << 24); + static constexpr int64_t BaseTimeTick = 64; + static constexpr int64_t TimeWrapPeriod = BaseTimeTick * (1ll << 24); + + public: struct PacketResult { PacketResult(uint16_t sequenceNumber, bool received) @@ -239,26 +241,26 @@ namespace RTC { return this->referenceTime; } - // We only use this for testing purpose + // We only use this for testing purpose. void SetReferenceTime(uint64_t referenceTime) { - this->referenceTime = (referenceTime % kTimeWrapPeriod) / kBaseTimeTick; + this->referenceTime = (referenceTime % TimeWrapPeriod) / BaseTimeTick; } int64_t GetReferenceTimestamp() const // Reference time in ms. { - return kTimeWrapPeriod + static_cast(this->referenceTime) * kBaseTimeTick; + return TimeWrapPeriod + static_cast(this->referenceTime) * BaseTimeTick; } int64_t GetBaseDelta(const int64_t previousTimestampMs) const { int64_t delta = GetReferenceTimestamp() - previousTimestampMs; // Compensate for wrap around. - if (std::abs(delta - kTimeWrapPeriod) < std::abs(delta)) + if (std::abs(delta - TimeWrapPeriod) < std::abs(delta)) { - delta -= kTimeWrapPeriod; + delta -= TimeWrapPeriod; } - else if (std::abs(delta + kTimeWrapPeriod) < std::abs(delta)) + else if (std::abs(delta + TimeWrapPeriod) < std::abs(delta)) { - delta += kTimeWrapPeriod; + delta += TimeWrapPeriod; } return delta; diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 6ff08a0929..79a47a8dee 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -478,8 +478,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetReferenceTime() == 6275825); // 0x5FC2F1 (signed 24 bits) REQUIRE( packet->GetReferenceTimestamp() == - FeedbackRtpTransportPacket::kTimeWrapPeriod + - static_cast(6275825) * FeedbackRtpTransportPacket::kBaseTimeTick); + FeedbackRtpTransportPacket::TimeWrapPeriod + + static_cast(6275825) * FeedbackRtpTransportPacket::BaseTimeTick); REQUIRE(packet->GetFeedbackPacketCount() == 3); SECTION("serialize packet") @@ -516,8 +516,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetReferenceTime() == 16777214); // 0xFFFFFE (unsigned 24 bits) REQUIRE( packet->GetReferenceTimestamp() == - FeedbackRtpTransportPacket::kTimeWrapPeriod + - static_cast(16777214) * FeedbackRtpTransportPacket::kBaseTimeTick); + FeedbackRtpTransportPacket::TimeWrapPeriod + + static_cast(16777214) * FeedbackRtpTransportPacket::BaseTimeTick); REQUIRE(packet->GetFeedbackPacketCount() == 1); SECTION("serialize packet") @@ -555,8 +555,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" REQUIRE(packet->GetReferenceTime() == 12408746); REQUIRE( packet->GetReferenceTimestamp() == - FeedbackRtpTransportPacket::kTimeWrapPeriod + - static_cast(12408746) * FeedbackRtpTransportPacket::kBaseTimeTick); + FeedbackRtpTransportPacket::TimeWrapPeriod + + static_cast(12408746) * FeedbackRtpTransportPacket::BaseTimeTick); // Let's also test the reference time reported by Wireshark. int32_t wiresharkValue{ 12408746 }; @@ -738,17 +738,17 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" SECTION("Check GetBaseDelta Wraparound") { - auto kMaxBaseTime = - FeedbackRtpTransportPacket::kTimeWrapPeriod - FeedbackRtpTransportPacket::kBaseTimeTick; + auto MaxBaseTime = + FeedbackRtpTransportPacket::TimeWrapPeriod - FeedbackRtpTransportPacket::BaseTimeTick; auto* packet1 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); auto* packet2 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); auto* packet3 = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); - packet1->SetReferenceTime(kMaxBaseTime); - packet2->SetReferenceTime(kMaxBaseTime + FeedbackRtpTransportPacket::kBaseTimeTick); + packet1->SetReferenceTime(MaxBaseTime); + packet2->SetReferenceTime(MaxBaseTime + FeedbackRtpTransportPacket::BaseTimeTick); packet3->SetReferenceTime( - kMaxBaseTime + FeedbackRtpTransportPacket::kBaseTimeTick + - FeedbackRtpTransportPacket::kBaseTimeTick); + MaxBaseTime + FeedbackRtpTransportPacket::BaseTimeTick + + FeedbackRtpTransportPacket::BaseTimeTick); REQUIRE(packet1->GetReferenceTime() == 16777215); REQUIRE(packet2->GetReferenceTime() == 0); From 8543bc824d3b44c1ba156c5a17c0cc3b1c4b5fa5 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Mon, 19 Sep 2022 13:34:13 +0300 Subject: [PATCH 11/12] Cosmetics. --- worker/include/RTC/RTCP/FeedbackRtpTransport.hpp | 8 ++++---- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 6b4f173358..58cbc6f151 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -42,8 +42,8 @@ namespace RTC class FeedbackRtpTransportPacket : public FeedbackRtpPacket { public: - static constexpr int64_t BaseTimeTick = 64; - static constexpr int64_t TimeWrapPeriod = BaseTimeTick * (1ll << 24); + static constexpr int64_t BaseTimeTick = 64; + static constexpr int64_t TimeWrapPeriod = BaseTimeTick * (1ll << 24); public: struct PacketResult @@ -241,8 +241,7 @@ namespace RTC { return this->referenceTime; } - // We only use this for testing purpose. - void SetReferenceTime(uint64_t referenceTime) + void SetReferenceTime(uint64_t referenceTime) // We only use this for testing purpose. { this->referenceTime = (referenceTime % TimeWrapPeriod) / BaseTimeTick; } @@ -253,6 +252,7 @@ namespace RTC int64_t GetBaseDelta(const int64_t previousTimestampMs) const { int64_t delta = GetReferenceTimestamp() - previousTimestampMs; + // Compensate for wrap around. if (std::abs(delta - TimeWrapPeriod) < std::abs(delta)) { diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 79a47a8dee..0ea02a642d 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -596,8 +596,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" .packetStatusCount = 1, .deltas = std::vector{ 57 }, .buffer = std::vector{ 0xaf, 0xcd, 0x00, 0x05, 0xfa, 0x17, 0xfa, 0x17, - 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, - 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 } }, + 0x00, 0x00, 0x04, 0xd2, 0x00, 0x0d, 0x00, 0x01, + 0x00, 0x8A, 0xB0, 0x00, 0x20, 0x01, 0xE4, 0x01 } }, { .baseTimeRaw = 35504, .baseTimeMs = 1076014080, .baseSequence = 14, @@ -633,8 +633,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" .packetStatusCount = 2, .deltas = std::vector{ 35, 17 }, .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, - 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 } }, + 0x39, 0xE9, 0x42, 0x38, 0x00, 0x01, 0x00, 0x02, + 0xBD, 0x57, 0xAA, 0x00, 0x20, 0x02, 0x8C, 0x44 } }, { .baseTimeRaw = 818995, .baseTimeMs = 1126157504, @@ -693,16 +693,16 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" .packetStatusCount = 2, .deltas = std::vector{ 44, 18 }, .buffer = std::vector{ 0x8F, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, - 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 } }, + 0x08, 0xEB, 0x06, 0xD7, 0x00, 0x11, 0x00, 0x02, + 0x0C, 0x89, 0x14, 0x01, 0x20, 0x02, 0xB0, 0x48 } }, { .baseTimeRaw = 821524, .baseTimeMs = 1126319360, .baseSequence = 17, .packetStatusCount = 1, .deltas = std::vector{ 62 }, .buffer = std::vector{ 0xAF, 0xCD, 0x00, 0x05, 0xFA, 0x17, 0xFA, 0x17, - 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, - 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 } }, + 0x20, 0x92, 0x5E, 0xB7, 0x00, 0x11, 0x00, 0x01, + 0x0C, 0x89, 0x14, 0x00, 0x20, 0x01, 0xF8, 0x01 } }, { .baseTimeRaw = 821526, .baseTimeMs = 1126319488, .baseSequence = 19, From a66167f89b311dc1d58aafa87322aabeedaf2d05 Mon Sep 17 00:00:00 2001 From: Eugene Voityuk Date: Mon, 19 Sep 2022 15:47:33 +0300 Subject: [PATCH 12/12] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf869b341e..9d3be8f907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * New C++ `ChannelMessageHandlers` class (PR #894). * Fix Rust support after recent changes (PR #898). * Update NPM deps. +* Modify `FeedbackRtpTransport` and tests to be compliant with latest libwebrtc code, make reference time to be unsigned (PR #899 by @penguinol and @sarumjanuch). ### 3.10.5