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/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index e28964d..7d72a43 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -1099,6 +1099,7 @@
"../../test:rtp_test_utils",
"../rtp_rtcp",
"../rtp_rtcp:rtp_rtcp_format",
+ "//third_party/abseil-cpp/absl/memory:memory",
"//third_party/abseil-cpp/absl/types:optional",
]
diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc
index 8545f7a..d029c60 100644
--- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc
+++ b/modules/audio_coding/acm2/audio_coding_module_unittest.cc
@@ -71,12 +71,12 @@
const uint8_t kPayloadType = 111;
} // namespace
-class RtpUtility {
+class RtpData {
public:
- RtpUtility(int samples_per_packet, uint8_t payload_type)
+ RtpData(int samples_per_packet, uint8_t payload_type)
: samples_per_packet_(samples_per_packet), payload_type_(payload_type) {}
- virtual ~RtpUtility() {}
+ virtual ~RtpData() {}
void Populate(RTPHeader* rtp_header) {
rtp_header->sequenceNumber = 0xABCD;
@@ -163,7 +163,7 @@
class AudioCodingModuleTestOldApi : public ::testing::Test {
protected:
AudioCodingModuleTestOldApi()
- : rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)),
+ : rtp_utility_(new RtpData(kFrameSizeSamples, kPayloadType)),
clock_(Clock::GetRealTimeClock()) {}
~AudioCodingModuleTestOldApi() {}
@@ -239,7 +239,7 @@
VerifyEncoding();
}
- std::unique_ptr<RtpUtility> rtp_utility_;
+ std::unique_ptr<RtpData> rtp_utility_;
std::unique_ptr<AudioCodingModule> acm_;
PacketizationCallbackStubOldApi packet_cb_;
RTPHeader rtp_header_;
diff --git a/modules/audio_coding/neteq/tools/packet.cc b/modules/audio_coding/neteq/tools/packet.cc
index 4e2102d..6ed6f98 100644
--- a/modules/audio_coding/neteq/tools/packet.cc
+++ b/modules/audio_coding/neteq/tools/packet.cc
@@ -14,81 +14,53 @@
#include <memory>
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
+#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/checks.h"
namespace webrtc {
namespace test {
-Packet::Packet(uint8_t* packet_memory,
- size_t allocated_bytes,
- double time_ms,
- const RtpHeaderParser& parser)
- : payload_memory_(packet_memory),
- payload_(NULL),
- packet_length_bytes_(allocated_bytes),
- payload_length_bytes_(0),
- virtual_packet_length_bytes_(allocated_bytes),
- virtual_payload_length_bytes_(0),
- time_ms_(time_ms) {
- valid_header_ = ParseHeader(parser);
-}
+using webrtc::RtpUtility::RtpHeaderParser;
Packet::Packet(uint8_t* packet_memory,
size_t allocated_bytes,
size_t virtual_packet_length_bytes,
double time_ms,
- const RtpHeaderParser& parser)
+ const RtpUtility::RtpHeaderParser& parser,
+ const RtpHeaderExtensionMap* extension_map /*= nullptr*/)
: payload_memory_(packet_memory),
- payload_(NULL),
packet_length_bytes_(allocated_bytes),
- payload_length_bytes_(0),
virtual_packet_length_bytes_(virtual_packet_length_bytes),
virtual_payload_length_bytes_(0),
- time_ms_(time_ms) {
- valid_header_ = ParseHeader(parser);
-}
+ time_ms_(time_ms),
+ valid_header_(ParseHeader(parser, extension_map)) {}
Packet::Packet(const RTPHeader& header,
size_t virtual_packet_length_bytes,
size_t virtual_payload_length_bytes,
double time_ms)
: header_(header),
- payload_memory_(),
- payload_(NULL),
- packet_length_bytes_(0),
- payload_length_bytes_(0),
virtual_packet_length_bytes_(virtual_packet_length_bytes),
virtual_payload_length_bytes_(virtual_payload_length_bytes),
time_ms_(time_ms),
valid_header_(true) {}
Packet::Packet(uint8_t* packet_memory, size_t allocated_bytes, double time_ms)
- : payload_memory_(packet_memory),
- payload_(NULL),
- packet_length_bytes_(allocated_bytes),
- payload_length_bytes_(0),
- virtual_packet_length_bytes_(allocated_bytes),
- virtual_payload_length_bytes_(0),
- time_ms_(time_ms) {
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
- valid_header_ = ParseHeader(*parser);
-}
+ : Packet(packet_memory,
+ allocated_bytes,
+ allocated_bytes,
+ time_ms,
+ RtpUtility::RtpHeaderParser(packet_memory, allocated_bytes)) {}
Packet::Packet(uint8_t* packet_memory,
size_t allocated_bytes,
size_t virtual_packet_length_bytes,
double time_ms)
- : payload_memory_(packet_memory),
- payload_(NULL),
- packet_length_bytes_(allocated_bytes),
- payload_length_bytes_(0),
- virtual_packet_length_bytes_(virtual_packet_length_bytes),
- virtual_payload_length_bytes_(0),
- time_ms_(time_ms) {
- std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
- valid_header_ = ParseHeader(*parser);
-}
+ : Packet(packet_memory,
+ allocated_bytes,
+ virtual_packet_length_bytes,
+ time_ms,
+ RtpUtility::RtpHeaderParser(packet_memory, allocated_bytes)) {}
Packet::~Packet() = default;
@@ -139,9 +111,10 @@
}
}
-bool Packet::ParseHeader(const RtpHeaderParser& parser) {
- bool valid_header = parser.Parse(
- payload_memory_.get(), static_cast<int>(packet_length_bytes_), &header_);
+bool Packet::ParseHeader(const RtpHeaderParser& parser,
+ const RtpHeaderExtensionMap* extension_map) {
+ bool valid_header = parser.Parse(&header_, extension_map);
+
// Special case for dummy packets that have padding marked in the RTP header.
// This causes the RTP header parser to report failure, but is fine in this
// context.
diff --git a/modules/audio_coding/neteq/tools/packet.h b/modules/audio_coding/neteq/tools/packet.h
index 5748ba2..f4189aa 100644
--- a/modules/audio_coding/neteq/tools/packet.h
+++ b/modules/audio_coding/neteq/tools/packet.h
@@ -15,11 +15,14 @@
#include <memory>
#include "api/rtp_headers.h" // NOLINT(build/include)
+#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "rtc_base/constructor_magic.h"
namespace webrtc {
+namespace RtpUtility {
class RtpHeaderParser;
+} // namespace RtpUtility
namespace test {
@@ -32,22 +35,17 @@
// when the Packet object is deleted. The |time_ms| is an extra time
// associated with this packet, typically used to denote arrival time.
// The first bytes in |packet_memory| will be parsed using |parser|.
- Packet(uint8_t* packet_memory,
- size_t allocated_bytes,
- double time_ms,
- const RtpHeaderParser& parser);
-
- // Same as above, but with the extra argument |virtual_packet_length_bytes|.
- // This is typically used when reading RTP dump files that only contain the
- // RTP headers, and no payload (a.k.a RTP dummy files or RTP light). The
- // |virtual_packet_length_bytes| tells what size the packet had on wire,
- // including the now discarded payload, whereas |allocated_bytes| is the
- // length of the remaining payload (typically only the RTP header).
+ // |virtual_packet_length_bytes| is typically used when reading RTP dump files
+ // that only contain the RTP headers, and no payload (a.k.a RTP dummy files or
+ // RTP light). The |virtual_packet_length_bytes| tells what size the packet
+ // had on wire, including the now discarded payload, whereas |allocated_bytes|
+ // is the length of the remaining payload (typically only the RTP header).
Packet(uint8_t* packet_memory,
size_t allocated_bytes,
size_t virtual_packet_length_bytes,
double time_ms,
- const RtpHeaderParser& parser);
+ const RtpUtility::RtpHeaderParser& parser,
+ const RtpHeaderExtensionMap* extension_map = nullptr);
// Same as above, but creates the packet from an already parsed RTPHeader.
// This is typically used when reading RTP dump files that only contain the
@@ -98,25 +96,25 @@
const RTPHeader& header() const { return header_; }
- void set_time_ms(double time) { time_ms_ = time; }
double time_ms() const { return time_ms_; }
bool valid_header() const { return valid_header_; }
private:
- bool ParseHeader(const RtpHeaderParser& parser);
+ bool ParseHeader(const webrtc::RtpUtility::RtpHeaderParser& parser,
+ const RtpHeaderExtensionMap* extension_map);
void CopyToHeader(RTPHeader* destination) const;
RTPHeader header_;
- std::unique_ptr<uint8_t[]> payload_memory_;
- const uint8_t* payload_; // First byte after header.
- const size_t packet_length_bytes_; // Total length of packet.
- size_t payload_length_bytes_; // Length of the payload, after RTP header.
- // Zero for dummy RTP packets.
+ const std::unique_ptr<uint8_t[]> payload_memory_;
+ const uint8_t* payload_ = nullptr; // First byte after header.
+ const size_t packet_length_bytes_ = 0; // Total length of packet.
+ size_t payload_length_bytes_ = 0; // Length of the payload, after RTP header.
+ // Zero for dummy RTP packets.
// Virtual lengths are used when parsing RTP header files (dummy RTP files).
const size_t virtual_packet_length_bytes_;
- size_t virtual_payload_length_bytes_;
- double time_ms_; // Used to denote a packet's arrival time.
- bool valid_header_; // Set by the RtpHeaderParser.
+ size_t virtual_payload_length_bytes_ = 0;
+ const double time_ms_; // Used to denote a packet's arrival time.
+ const bool valid_header_; // Set by the RtpHeaderParser.
RTC_DISALLOW_COPY_AND_ASSIGN(Packet);
};
diff --git a/modules/audio_coding/neteq/tools/rtp_file_source.cc b/modules/audio_coding/neteq/tools/rtp_file_source.cc
index eda2b3e..410af27 100644
--- a/modules/audio_coding/neteq/tools/rtp_file_source.cc
+++ b/modules/audio_coding/neteq/tools/rtp_file_source.cc
@@ -18,8 +18,8 @@
#include <memory>
+#include "absl/memory/memory.h"
#include "modules/audio_coding/neteq/tools/packet.h"
-#include "modules/rtp_rtcp/include/rtp_header_parser.h"
#include "rtc_base/checks.h"
#include "test/rtp_file_reader.h"
@@ -49,8 +49,7 @@
bool RtpFileSource::RegisterRtpHeaderExtension(RTPExtensionType type,
uint8_t id) {
- assert(parser_.get());
- return parser_->RegisterRtpHeaderExtension(type, id);
+ return rtp_header_extension_map_.RegisterByType(id, type);
}
std::unique_ptr<Packet> RtpFileSource::NextPacket() {
@@ -66,9 +65,11 @@
}
std::unique_ptr<uint8_t[]> packet_memory(new uint8_t[temp_packet.length]);
memcpy(packet_memory.get(), temp_packet.data, temp_packet.length);
- std::unique_ptr<Packet> packet(new Packet(
+ RtpUtility::RtpHeaderParser parser(packet_memory.get(), temp_packet.length);
+ auto packet = absl::make_unique<Packet>(
packet_memory.release(), temp_packet.length,
- temp_packet.original_length, temp_packet.time_ms, *parser_.get()));
+ temp_packet.original_length, temp_packet.time_ms, parser,
+ &rtp_header_extension_map_);
if (!packet->valid_header()) {
continue;
}
@@ -83,7 +84,6 @@
RtpFileSource::RtpFileSource(absl::optional<uint32_t> ssrc_filter)
: PacketSource(),
- parser_(RtpHeaderParser::Create()),
ssrc_filter_(ssrc_filter) {}
bool RtpFileSource::OpenFile(const std::string& file_name) {
diff --git a/modules/audio_coding/neteq/tools/rtp_file_source.h b/modules/audio_coding/neteq/tools/rtp_file_source.h
index 77e435a..953e2fa 100644
--- a/modules/audio_coding/neteq/tools/rtp_file_source.h
+++ b/modules/audio_coding/neteq/tools/rtp_file_source.h
@@ -19,12 +19,11 @@
#include "absl/types/optional.h"
#include "modules/audio_coding/neteq/tools/packet_source.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtp_utility.h"
#include "rtc_base/constructor_magic.h"
namespace webrtc {
-class RtpHeaderParser;
-
namespace test {
class RtpFileReader;
@@ -58,8 +57,8 @@
bool OpenFile(const std::string& file_name);
std::unique_ptr<RtpFileReader> rtp_reader_;
- std::unique_ptr<RtpHeaderParser> parser_;
const absl::optional<uint32_t> ssrc_filter_;
+ RtpHeaderExtensionMap rtp_header_extension_map_;
RTC_DISALLOW_COPY_AND_ASSIGN(RtpFileSource);
};