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/test/BUILD.gn b/test/BUILD.gn
index bc56d89..74c5268 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -131,15 +131,19 @@
"rtp_file_reader.h",
"rtp_file_writer.cc",
"rtp_file_writer.h",
+ "rtp_header_parser.cc",
+ "rtp_header_parser.h",
]
deps = [
"../api:array_view",
+ "../api:rtp_parameters",
"../modules/rtp_rtcp",
"../modules/rtp_rtcp:rtp_rtcp_format",
"../rtc_base:checks",
"../rtc_base:rtc_base_approved",
"../rtc_base/system:arch",
+ "//third_party/abseil-cpp/absl/memory",
]
}
@@ -597,6 +601,7 @@
"direct_transport.h",
]
deps = [
+ ":rtp_test_utils",
"../api:simulated_network_api",
"../api:transport_api",
"../call:call_interfaces",
diff --git a/test/direct_transport.cc b/test/direct_transport.cc
index 4638652..7ca5bb1 100644
--- a/test/direct_transport.cc
+++ b/test/direct_transport.cc
@@ -12,8 +12,8 @@
#include "absl/memory/memory.h"
#include "call/call.h"
#include "call/fake_network_pipe.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/time_utils.h"
+#include "test/rtp_header_parser.h"
#include "test/single_threaded_task_queue.h"
namespace webrtc {
diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn
index cd8e217..3618303 100644
--- a/test/fuzzers/BUILD.gn
+++ b/test/fuzzers/BUILD.gn
@@ -616,7 +616,7 @@
"rtp_header_parser_fuzzer.cc",
]
deps = [
- "../../modules/rtp_rtcp",
+ "../:rtp_test_utils",
]
}
@@ -625,7 +625,7 @@
"rtp_header_parser_fuzzer.cc",
]
deps = [
- "../../modules/rtp_rtcp",
+ "../:rtp_test_utils",
]
}
diff --git a/test/fuzzers/rtp_header_parser_fuzzer.cc b/test/fuzzers/rtp_header_parser_fuzzer.cc
index 6d95fdc..d6af5ca 100644
--- a/test/fuzzers/rtp_header_parser_fuzzer.cc
+++ b/test/fuzzers/rtp_header_parser_fuzzer.cc
@@ -15,7 +15,7 @@
#include <memory>
#include <string>
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
+#include "test/rtp_header_parser.h"
namespace webrtc {
@@ -24,7 +24,8 @@
RtpHeaderParser::GetSsrc(data, size);
RTPHeader rtp_header;
- std::unique_ptr<RtpHeaderParser> rtp_header_parser(RtpHeaderParser::Create());
+ std::unique_ptr<RtpHeaderParser> rtp_header_parser(
+ RtpHeaderParser::CreateForTest());
rtp_header_parser->Parse(data, size, &rtp_header);
for (int i = 1; i < kRtpExtensionNumberOfExtensions; ++i) {
diff --git a/test/fuzzers/utils/BUILD.gn b/test/fuzzers/utils/BUILD.gn
index 307cbe1..007c750 100644
--- a/test/fuzzers/utils/BUILD.gn
+++ b/test/fuzzers/utils/BUILD.gn
@@ -23,7 +23,6 @@
"../../../call:call_interfaces",
"../../../common_video",
"../../../media:rtc_internal_video_codecs",
- "../../../modules/rtp_rtcp",
"../../../rtc_base:checks",
"../../../rtc_base:rtc_base_approved",
"../../../rtc_base:rtc_json",
diff --git a/test/fuzzers/utils/rtp_replayer.cc b/test/fuzzers/utils/rtp_replayer.cc
index e430d40..0656f4c 100644
--- a/test/fuzzers/utils/rtp_replayer.cc
+++ b/test/fuzzers/utils/rtp_replayer.cc
@@ -16,7 +16,6 @@
#include "absl/memory/memory.h"
#include "api/task_queue/default_task_queue_factory.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/strings/json.h"
#include "system_wrappers/include/clock.h"
#include "system_wrappers/include/sleep.h"
@@ -24,6 +23,7 @@
#include "test/encoder_settings.h"
#include "test/fake_decoder.h"
#include "test/rtp_file_reader.h"
+#include "test/rtp_header_parser.h"
namespace webrtc {
namespace test {
@@ -158,7 +158,8 @@
break;
case PacketReceiver::DELIVERY_UNKNOWN_SSRC: {
RTPHeader header;
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
+ std::unique_ptr<RtpHeaderParser> parser(
+ RtpHeaderParser::CreateForTest());
parser->Parse(packet.data, packet.length, &header);
if (unknown_packets[header.ssrc] == 0) {
@@ -171,7 +172,8 @@
RTC_LOG(LS_ERROR)
<< "Packet error, corrupt packets or incorrect setup?";
RTPHeader header;
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
+ std::unique_ptr<RtpHeaderParser> parser(
+ RtpHeaderParser::CreateForTest());
parser->Parse(packet.data, packet.length, &header);
RTC_LOG(LS_ERROR) << "Packet packet_length=" << packet.length
<< " payload_type=" << header.payloadType
diff --git a/test/rtp_header_parser.cc b/test/rtp_header_parser.cc
new file mode 100644
index 0000000..1a4ba42
--- /dev/null
+++ b/test/rtp_header_parser.cc
@@ -0,0 +1,103 @@
+/*
+ * 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 "test/rtp_header_parser.h"
+
+#include <memory>
+
+#include "absl/memory/memory.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_);
+};
+
+std::unique_ptr<RtpHeaderParser> RtpHeaderParser::CreateForTest() {
+ return absl::make_unique<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/test/rtp_header_parser.h b/test/rtp_header_parser.h
new file mode 100644
index 0000000..851ccf3
--- /dev/null
+++ b/test/rtp_header_parser.h
@@ -0,0 +1,53 @@
+/*
+ * 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 TEST_RTP_HEADER_PARSER_H_
+#define TEST_RTP_HEADER_PARSER_H_
+
+#include <memory>
+
+#include "api/rtp_parameters.h"
+#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+
+namespace webrtc {
+
+struct RTPHeader;
+
+class RtpHeaderParser {
+ public:
+ static std::unique_ptr<RtpHeaderParser> CreateForTest();
+ 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 // TEST_RTP_HEADER_PARSER_H_
diff --git a/test/rtp_rtcp_observer.h b/test/rtp_rtcp_observer.h
index 830c2f1..5763039 100644
--- a/test/rtp_rtcp_observer.h
+++ b/test/rtp_rtcp_observer.h
@@ -18,12 +18,12 @@
#include "api/test/simulated_network.h"
#include "call/simulated_packet_receiver.h"
#include "call/video_send_stream.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/event.h"
#include "system_wrappers/include/field_trial.h"
#include "test/direct_transport.h"
#include "test/gtest.h"
+#include "test/rtp_header_parser.h"
namespace {
const int kShortTimeoutMs = 500;
@@ -71,7 +71,8 @@
protected:
RtpRtcpObserver() : RtpRtcpObserver(0) {}
explicit RtpRtcpObserver(int event_timeout_ms)
- : parser_(RtpHeaderParser::Create()), timeout_ms_(event_timeout_ms) {}
+ : parser_(RtpHeaderParser::CreateForTest()),
+ timeout_ms_(event_timeout_ms) {}
rtc::Event observation_complete_;
const std::unique_ptr<RtpHeaderParser> parser_;
diff --git a/test/scenario/BUILD.gn b/test/scenario/BUILD.gn
index 9792271..dce7775 100644
--- a/test/scenario/BUILD.gn
+++ b/test/scenario/BUILD.gn
@@ -76,6 +76,7 @@
":column_printer",
"../:fake_video_codecs",
"../:fileutils",
+ "../:rtp_test_utils",
"../:test_common",
"../:test_support",
"../:video_test_common",
diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc
index 31435bb..1654afc 100644
--- a/test/scenario/call_client.cc
+++ b/test/scenario/call_client.cc
@@ -203,7 +203,7 @@
clock_(time_controller->GetClock()),
log_writer_factory_(std::move(log_writer_factory)),
network_controller_factory_(log_writer_factory_.get(), config.transport),
- header_parser_(RtpHeaderParser::Create()),
+ header_parser_(RtpHeaderParser::CreateForTest()),
task_queue_(time_controller->GetTaskQueueFactory()->CreateTaskQueue(
"CallClient",
TaskQueueFactory::Priority::NORMAL)) {
diff --git a/test/scenario/call_client.h b/test/scenario/call_client.h
index d2603a8..78c302d 100644
--- a/test/scenario/call_client.h
+++ b/test/scenario/call_client.h
@@ -20,11 +20,11 @@
#include "call/call.h"
#include "modules/audio_device/include/test_audio_device.h"
#include "modules/congestion_controller/goog_cc/test/goog_cc_printer.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/task_queue_for_test.h"
#include "test/logging/log_writer.h"
#include "test/network/network_emulation.h"
+#include "test/rtp_header_parser.h"
#include "test/scenario/column_printer.h"
#include "test/scenario/network_node.h"
#include "test/scenario/scenario_config.h"