Make the RtpHeaderParserImpl available to tests and tools only.
There are a few reasons for making this test only:
* The code is only used by tests and utilities.
* The pure interface has only a single implementation so an interface isn't really needed.
(a followup change could remove it altogether)
* The implementation always incorporates locking regardless of how the class gets used.
See e.g. previous use in the Packet class.
* The implementation is a layer on top of RtpUtility::RtpHeaderParser which is
sufficient for most production cases.
Change-Id: Ide6d50567cf8ae5127a2eb04cceeb10cf317ec36
Bug: none
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150658
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29010}
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index 06ed9bd..24ed0d2 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -129,7 +129,6 @@
"include/flexfec_sender.h",
"include/receive_statistics.h",
"include/remote_ntp_time_estimator.h",
- "include/rtp_header_parser.h",
"include/rtp_rtcp.h",
"include/ulpfec_receiver.h",
"source/absolute_capture_time_receiver.cc",
@@ -175,7 +174,6 @@
"source/rtp_format_vp9.h",
"source/rtp_header_extension_size.cc",
"source/rtp_header_extension_size.h",
- "source/rtp_header_parser.cc",
"source/rtp_packet_history.cc",
"source/rtp_packet_history.h",
"source/rtp_rtcp_config.h",
diff --git a/modules/rtp_rtcp/include/rtp_header_parser.h b/modules/rtp_rtcp/include/rtp_header_parser.h
deleted file mode 100644
index 0afcb71..0000000
--- a/modules/rtp_rtcp/include/rtp_header_parser.h
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * Copyright (c) 2013 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_INCLUDE_RTP_HEADER_PARSER_H_
-#define MODULES_RTP_RTCP_INCLUDE_RTP_HEADER_PARSER_H_
-
-#include "api/rtp_parameters.h"
-#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
-
-namespace webrtc {
-
-struct RTPHeader;
-
-class RtpHeaderParser {
- public:
- static RtpHeaderParser* Create();
- virtual ~RtpHeaderParser() {}
-
- // Returns true if the packet is an RTCP packet, false otherwise.
- static bool IsRtcp(const uint8_t* packet, size_t length);
- static absl::optional<uint32_t> GetSsrc(const uint8_t* packet, size_t length);
-
- // Parses the packet and stores the parsed packet in |header|. Returns true on
- // success, false otherwise.
- // This method is thread-safe in the sense that it can parse multiple packets
- // at once.
- virtual bool Parse(const uint8_t* packet,
- size_t length,
- RTPHeader* header) const = 0;
-
- // Registers an RTP header extension and binds it to |id|.
- virtual bool RegisterRtpHeaderExtension(RTPExtensionType type,
- uint8_t id) = 0;
-
- // Registers an RTP header extension.
- virtual bool RegisterRtpHeaderExtension(RtpExtension extension) = 0;
-
- // De-registers an RTP header extension.
- virtual bool DeregisterRtpHeaderExtension(RTPExtensionType type) = 0;
-
- // De-registers an RTP header extension.
- virtual bool DeregisterRtpHeaderExtension(RtpExtension extension) = 0;
-};
-} // namespace webrtc
-#endif // MODULES_RTP_RTCP_INCLUDE_RTP_HEADER_PARSER_H_
diff --git a/modules/rtp_rtcp/source/rtp_header_parser.cc b/modules/rtp_rtcp/source/rtp_header_parser.cc
deleted file mode 100644
index 65431d1..0000000
--- a/modules/rtp_rtcp/source/rtp_header_parser.cc
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright (c) 2013 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/include/rtp_header_parser.h"
-
-#include <string.h>
-
-#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
-#include "modules/rtp_rtcp/source/rtp_utility.h"
-#include "rtc_base/critical_section.h"
-#include "rtc_base/thread_annotations.h"
-
-namespace webrtc {
-
-class RtpHeaderParserImpl : public RtpHeaderParser {
- public:
- RtpHeaderParserImpl();
- ~RtpHeaderParserImpl() override = default;
-
- bool Parse(const uint8_t* packet,
- size_t length,
- RTPHeader* header) const override;
-
- bool RegisterRtpHeaderExtension(RTPExtensionType type, uint8_t id) override;
- bool RegisterRtpHeaderExtension(RtpExtension extension) override;
-
- bool DeregisterRtpHeaderExtension(RTPExtensionType type) override;
- bool DeregisterRtpHeaderExtension(RtpExtension extension) override;
-
- private:
- rtc::CriticalSection critical_section_;
- RtpHeaderExtensionMap rtp_header_extension_map_
- RTC_GUARDED_BY(critical_section_);
-};
-
-RtpHeaderParser* RtpHeaderParser::Create() {
- return new RtpHeaderParserImpl;
-}
-
-RtpHeaderParserImpl::RtpHeaderParserImpl() {}
-
-bool RtpHeaderParser::IsRtcp(const uint8_t* packet, size_t length) {
- RtpUtility::RtpHeaderParser rtp_parser(packet, length);
- return rtp_parser.RTCP();
-}
-
-absl::optional<uint32_t> RtpHeaderParser::GetSsrc(const uint8_t* packet,
- size_t length) {
- RtpUtility::RtpHeaderParser rtp_parser(packet, length);
- RTPHeader header;
- if (rtp_parser.Parse(&header, nullptr)) {
- return header.ssrc;
- }
- return absl::nullopt;
-}
-
-bool RtpHeaderParserImpl::Parse(const uint8_t* packet,
- size_t length,
- RTPHeader* header) const {
- RtpUtility::RtpHeaderParser rtp_parser(packet, length);
- *header = RTPHeader();
-
- RtpHeaderExtensionMap map;
- {
- rtc::CritScope cs(&critical_section_);
- map = rtp_header_extension_map_;
- }
-
- const bool valid_rtpheader = rtp_parser.Parse(header, &map);
- if (!valid_rtpheader) {
- return false;
- }
- return true;
-}
-bool RtpHeaderParserImpl::RegisterRtpHeaderExtension(RtpExtension extension) {
- rtc::CritScope cs(&critical_section_);
- return rtp_header_extension_map_.RegisterByUri(extension.id, extension.uri);
-}
-
-bool RtpHeaderParserImpl::RegisterRtpHeaderExtension(RTPExtensionType type,
- uint8_t id) {
- rtc::CritScope cs(&critical_section_);
- return rtp_header_extension_map_.RegisterByType(id, type);
-}
-
-bool RtpHeaderParserImpl::DeregisterRtpHeaderExtension(RtpExtension extension) {
- rtc::CritScope cs(&critical_section_);
- return rtp_header_extension_map_.Deregister(
- rtp_header_extension_map_.GetType(extension.id));
-}
-
-bool RtpHeaderParserImpl::DeregisterRtpHeaderExtension(RTPExtensionType type) {
- rtc::CritScope cs(&critical_section_);
- return rtp_header_extension_map_.Deregister(type) == 0;
-}
-} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index e6f8db1..f55e4f8 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -17,7 +17,6 @@
#include "absl/memory/memory.h"
#include "api/transport/field_trial_based_config.h"
#include "api/video_codecs/video_codec.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/playout_delay_oracle.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
@@ -28,6 +27,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/rtcp_packet_parser.h"
+#include "test/rtp_header_parser.h"
using ::testing::_;
using ::testing::ElementsAre;
@@ -72,7 +72,7 @@
size_t len,
const PacketOptions& options) override {
RTPHeader header;
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
+ std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::CreateForTest());
EXPECT_TRUE(parser->Parse(static_cast<const uint8_t*>(data), len, &header));
++rtp_packets_sent_;
last_rtp_header_ = header;
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 125a0b8..ad501df 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -21,7 +21,6 @@
#include "logging/rtc_event_log/mock/mock_rtc_event_log.h"
#include "modules/rtp_rtcp/include/rtp_cvo.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "modules/rtp_rtcp/include/rtp_packet_sender.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
@@ -39,6 +38,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/mock_transport.h"
+#include "test/rtp_header_parser.h"
namespace webrtc {
diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
index cd1798b..32f3bbb 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc
@@ -15,7 +15,6 @@
#include <list>
#include <memory>
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h"
#include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"
#include "modules/rtp_rtcp/source/byte_io.h"
@@ -23,6 +22,7 @@
#include "modules/rtp_rtcp/source/forward_error_correction.h"
#include "test/gmock.h"
#include "test/gtest.h"
+#include "test/rtp_header_parser.h"
namespace webrtc {
@@ -177,7 +177,7 @@
size_t length,
uint8_t ulpfec_payload_type) {
RTPHeader header;
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
+ std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::CreateForTest());
ASSERT_TRUE(parser->Parse(data, length, &header));
NullRecoveredPacketReceiver null_callback;