diff options
author | Danil Chapovalov <danilchap@webrtc.org> | 2022-07-21 13:01:08 +0200 |
---|---|---|
committer | WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2022-07-27 14:53:05 +0000 |
commit | 300a230f16193ea9a312f5dba6fcdbc0c1797cef (patch) | |
tree | 699ec9767c01653fc2c4240586ac44e068364583 | |
parent | 23370f22c188d45a0e2788b48e1f04615780b0d2 (diff) | |
download | webrtc-300a230f16193ea9a312f5dba6fcdbc0c1797cef.tar.gz |
Delete inter arrival jitter rtcp packet as unused
WebRTC doesn't produces such packet and ignores it when receive.
Bug: None
Change-Id: I4af8cb3308cb2422808bdfc420a85fa175085bfb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269181
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37627}
-rw-r--r-- | logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc | 7 | ||||
-rw-r--r-- | logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc | 7 | ||||
-rw-r--r-- | modules/rtp_rtcp/BUILD.gn | 3 | ||||
-rw-r--r-- | modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.cc | 101 | ||||
-rw-r--r-- | modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h | 54 | ||||
-rw-r--r-- | modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report_unittest.cc | 76 | ||||
-rw-r--r-- | modules/rtp_rtcp/source/rtcp_receiver_unittest.cc | 11 | ||||
-rw-r--r-- | test/rtcp_packet_parser.cc | 3 | ||||
-rw-r--r-- | test/rtcp_packet_parser.h | 3 |
9 files changed, 4 insertions, 261 deletions
diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc index add42ad15b..422e2c4195 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc @@ -44,7 +44,6 @@ #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/psfb.h" #include "modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" @@ -713,15 +712,13 @@ std::string RtcEventLogEncoderLegacy::EncodeRtcpPacket( uint32_t block_size = next_block - block_begin; switch (header.type()) { case rtcp::Bye::kPacketType: - case rtcp::ExtendedJitterReport::kPacketType: case rtcp::ExtendedReports::kPacketType: case rtcp::Psfb::kPacketType: case rtcp::ReceiverReport::kPacketType: case rtcp::Rtpfb::kPacketType: case rtcp::SenderReport::kPacketType: - // We log sender reports, receiver reports, bye messages - // inter-arrival jitter, third-party loss reports, payload-specific - // feedback and extended reports. + // We log sender reports, receiver reports, bye messages, third-party + // loss reports, payload-specific feedback and extended reports. memcpy(buffer.data() + buffer_length, block_begin, block_size); buffer_length += block_size; break; diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc index d88f124f9e..9d791d1aca 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc @@ -49,7 +49,6 @@ #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/psfb.h" #include "modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" @@ -306,15 +305,13 @@ size_t RemoveNonAllowlistedRtcpBlocks(const rtc::Buffer& packet, size_t block_size = next_block - block_begin; switch (header.type()) { case rtcp::Bye::kPacketType: - case rtcp::ExtendedJitterReport::kPacketType: case rtcp::ExtendedReports::kPacketType: case rtcp::Psfb::kPacketType: case rtcp::ReceiverReport::kPacketType: case rtcp::Rtpfb::kPacketType: case rtcp::SenderReport::kPacketType: - // We log sender reports, receiver reports, bye messages - // inter-arrival jitter, third-party loss reports, payload-specific - // feedback and extended reports. + // We log sender reports, receiver reports, bye messages, third-party + // loss reports, payload-specific feedback and extended reports. // TODO(terelius): As an optimization, don't copy anything if all blocks // in the packet are allowlisted types. memcpy(buffer + buffer_length, block_begin, block_size); diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index b2a01aafac..19dbbabf9c 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -24,7 +24,6 @@ rtc_library("rtp_rtcp_format") { "source/rtcp_packet/common_header.h", "source/rtcp_packet/compound_packet.h", "source/rtcp_packet/dlrr.h", - "source/rtcp_packet/extended_jitter_report.h", "source/rtcp_packet/extended_reports.h", "source/rtcp_packet/fir.h", "source/rtcp_packet/loss_notification.h", @@ -64,7 +63,6 @@ rtc_library("rtp_rtcp_format") { "source/rtcp_packet/common_header.cc", "source/rtcp_packet/compound_packet.cc", "source/rtcp_packet/dlrr.cc", - "source/rtcp_packet/extended_jitter_report.cc", "source/rtcp_packet/extended_reports.cc", "source/rtcp_packet/fir.cc", "source/rtcp_packet/loss_notification.cc", @@ -538,7 +536,6 @@ if (rtc_include_tests) { "source/rtcp_packet/common_header_unittest.cc", "source/rtcp_packet/compound_packet_unittest.cc", "source/rtcp_packet/dlrr_unittest.cc", - "source/rtcp_packet/extended_jitter_report_unittest.cc", "source/rtcp_packet/extended_reports_unittest.cc", "source/rtcp_packet/fir_unittest.cc", "source/rtcp_packet/loss_notification_unittest.cc", diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.cc deleted file mode 100644 index 5e7dadd1f4..0000000000 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.cc +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" - -#include <cstdint> -#include <utility> - -#include "modules/rtp_rtcp/source/byte_io.h" -#include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" -#include "rtc_base/checks.h" -#include "rtc_base/logging.h" - -namespace webrtc { -namespace rtcp { -constexpr uint8_t ExtendedJitterReport::kPacketType; -// Transmission Time Offsets in RTP Streams (RFC 5450). -// -// 0 1 2 3 -// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// hdr |V=2|P| RC | PT=IJ=195 | length | -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// | inter-arrival jitter | -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// . . -// . . -// . . -// | inter-arrival jitter | -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// -// If present, this RTCP packet must be placed after a receiver report -// (inside a compound RTCP packet), and MUST have the same value for RC -// (reception report count) as the receiver report. - -ExtendedJitterReport::ExtendedJitterReport() = default; - -ExtendedJitterReport::~ExtendedJitterReport() = default; - -bool ExtendedJitterReport::Parse(const CommonHeader& packet) { - RTC_DCHECK_EQ(packet.type(), kPacketType); - - const uint8_t number_of_jitters = packet.count(); - - if (packet.payload_size_bytes() < number_of_jitters * kJitterSizeBytes) { - RTC_LOG(LS_WARNING) << "Packet is too small to contain all the jitter."; - return false; - } - - inter_arrival_jitters_.resize(number_of_jitters); - for (size_t index = 0; index < number_of_jitters; ++index) { - inter_arrival_jitters_[index] = ByteReader<uint32_t>::ReadBigEndian( - &packet.payload()[index * kJitterSizeBytes]); - } - - return true; -} - -bool ExtendedJitterReport::SetJitterValues(std::vector<uint32_t> values) { - if (values.size() > kMaxNumberOfJitterValues) { - RTC_LOG(LS_WARNING) << "Too many inter-arrival jitter items."; - return false; - } - inter_arrival_jitters_ = std::move(values); - return true; -} - -size_t ExtendedJitterReport::BlockLength() const { - return kHeaderLength + kJitterSizeBytes * inter_arrival_jitters_.size(); -} - -bool ExtendedJitterReport::Create(uint8_t* packet, - size_t* index, - size_t max_length, - PacketReadyCallback callback) const { - while (*index + BlockLength() > max_length) { - if (!OnBufferFull(packet, index, callback)) - return false; - } - const size_t index_end = *index + BlockLength(); - size_t length = inter_arrival_jitters_.size(); - CreateHeader(length, kPacketType, length, packet, index); - - for (uint32_t jitter : inter_arrival_jitters_) { - ByteWriter<uint32_t>::WriteBigEndian(packet + *index, jitter); - *index += kJitterSizeBytes; - } - // Sanity check. - RTC_DCHECK_EQ(index_end, *index); - return true; -} - -} // namespace rtcp -} // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h b/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h deleted file mode 100644 index c28b9d9dbd..0000000000 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_EXTENDED_JITTER_REPORT_H_ -#define MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_EXTENDED_JITTER_REPORT_H_ - -#include <vector> - -#include "modules/rtp_rtcp/source/rtcp_packet.h" - -namespace webrtc { -namespace rtcp { -class CommonHeader; - -class ExtendedJitterReport : public RtcpPacket { - public: - static constexpr uint8_t kPacketType = 195; - static constexpr size_t kMaxNumberOfJitterValues = 0x1f; - - ExtendedJitterReport(); - ~ExtendedJitterReport() override; - - // Parse assumes header is already parsed and validated. - bool Parse(const CommonHeader& packet); - - bool SetJitterValues(std::vector<uint32_t> jitter_values); - - const std::vector<uint32_t>& jitter_values() { - return inter_arrival_jitters_; - } - - size_t BlockLength() const override; - - bool Create(uint8_t* packet, - size_t* index, - size_t max_length, - PacketReadyCallback callback) const override; - - private: - static constexpr size_t kJitterSizeBytes = 4; - - std::vector<uint32_t> inter_arrival_jitters_; -}; - -} // namespace rtcp -} // namespace webrtc -#endif // MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_EXTENDED_JITTER_REPORT_H_ diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report_unittest.cc deleted file mode 100644 index 3fd7d20397..0000000000 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report_unittest.cc +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" - -#include "test/gmock.h" -#include "test/gtest.h" -#include "test/rtcp_packet_parser.h" - -using ::testing::ElementsAre; -using ::testing::IsEmpty; -using webrtc::rtcp::ExtendedJitterReport; - -namespace webrtc { -namespace { -constexpr uint32_t kJitter1 = 0x11121314; -constexpr uint32_t kJitter2 = 0x22242628; -} // namespace - -TEST(RtcpPacketExtendedJitterReportTest, CreateAndParseWithoutItems) { - ExtendedJitterReport ij; - rtc::Buffer raw = ij.Build(); - - ExtendedJitterReport parsed; - EXPECT_TRUE(test::ParseSinglePacket(raw, &parsed)); - - EXPECT_THAT(parsed.jitter_values(), IsEmpty()); -} - -TEST(RtcpPacketExtendedJitterReportTest, CreateAndParseWithOneItem) { - ExtendedJitterReport ij; - EXPECT_TRUE(ij.SetJitterValues({kJitter1})); - rtc::Buffer raw = ij.Build(); - - ExtendedJitterReport parsed; - EXPECT_TRUE(test::ParseSinglePacket(raw, &parsed)); - - EXPECT_THAT(parsed.jitter_values(), ElementsAre(kJitter1)); -} - -TEST(RtcpPacketExtendedJitterReportTest, CreateAndParseWithTwoItems) { - ExtendedJitterReport ij; - EXPECT_TRUE(ij.SetJitterValues({kJitter1, kJitter2})); - rtc::Buffer raw = ij.Build(); - - ExtendedJitterReport parsed; - EXPECT_TRUE(test::ParseSinglePacket(raw, &parsed)); - - EXPECT_THAT(parsed.jitter_values(), ElementsAre(kJitter1, kJitter2)); -} - -TEST(RtcpPacketExtendedJitterReportTest, CreateWithTooManyItems) { - ExtendedJitterReport ij; - const int kMaxItems = ExtendedJitterReport::kMaxNumberOfJitterValues; - EXPECT_FALSE( - ij.SetJitterValues(std::vector<uint32_t>(kMaxItems + 1, kJitter1))); - EXPECT_TRUE(ij.SetJitterValues(std::vector<uint32_t>(kMaxItems, kJitter1))); -} - -TEST(RtcpPacketExtendedJitterReportTest, ParseFailsWithTooManyItems) { - ExtendedJitterReport ij; - ij.SetJitterValues({kJitter1}); - rtc::Buffer raw = ij.Build(); - raw[0]++; // Damage packet: increase jitter count by 1. - ExtendedJitterReport parsed; - EXPECT_FALSE(test::ParseSinglePacket(raw, &parsed)); -} - -} // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index a3446c43f0..ff39ccca9c 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -26,7 +26,6 @@ #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/fir.h" #include "modules/rtp_rtcp/source/rtcp_packet/nack.h" @@ -593,16 +592,6 @@ TEST(RtcpReceiverTest, GetRtt) { EXPECT_EQ(0, receiver.RTT(kSenderSsrc, nullptr, nullptr, nullptr, nullptr)); } -// Ij packets are ignored. -TEST(RtcpReceiverTest, InjectIjWithNoItem) { - ReceiverMocks mocks; - RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - - rtcp::ExtendedJitterReport ij; - receiver.IncomingPacket(ij.Build()); -} - // App packets are ignored. TEST(RtcpReceiverTest, InjectApp) { ReceiverMocks mocks; diff --git a/test/rtcp_packet_parser.cc b/test/rtcp_packet_parser.cc index 94f500fe57..e286ec5a36 100644 --- a/test/rtcp_packet_parser.cc +++ b/test/rtcp_packet_parser.cc @@ -47,9 +47,6 @@ bool RtcpPacketParser::Parse(const void* data, size_t length) { case rtcp::ExtendedReports::kPacketType: xr_.Parse(header, &sender_ssrc_); break; - case rtcp::ExtendedJitterReport::kPacketType: - ij_.Parse(header); - break; case rtcp::Psfb::kPacketType: switch (header.fmt()) { case rtcp::Fir::kFeedbackMessageType: diff --git a/test/rtcp_packet_parser.h b/test/rtcp_packet_parser.h index 4916195822..9e8c9685e9 100644 --- a/test/rtcp_packet_parser.h +++ b/test/rtcp_packet_parser.h @@ -19,7 +19,6 @@ #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h" -#include "modules/rtp_rtcp/source/rtcp_packet/extended_jitter_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "modules/rtp_rtcp/source/rtcp_packet/fir.h" #include "modules/rtp_rtcp/source/rtcp_packet/loss_notification.h" @@ -84,7 +83,6 @@ class RtcpPacketParser { PacketCounter<rtcp::App>* app() { return &app_; } PacketCounter<rtcp::Bye>* bye() { return &bye_; } - PacketCounter<rtcp::ExtendedJitterReport>* ij() { return &ij_; } PacketCounter<rtcp::ExtendedReports>* xr() { return &xr_; } PacketCounter<rtcp::Fir>* fir() { return &fir_; } PacketCounter<rtcp::Nack>* nack() { return &nack_; } @@ -110,7 +108,6 @@ class RtcpPacketParser { private: PacketCounter<rtcp::App> app_; PacketCounter<rtcp::Bye> bye_; - PacketCounter<rtcp::ExtendedJitterReport> ij_; PacketCounter<rtcp::ExtendedReports> xr_; PacketCounter<rtcp::Fir> fir_; PacketCounter<rtcp::Nack> nack_; |