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}
This commit is contained in:
Tommi
2019-08-29 16:39:05 +02:00
committed by Commit Bot
parent 022a7c8d49
commit 25eb47ccf1
37 changed files with 152 additions and 159 deletions

View File

@ -1099,6 +1099,7 @@ rtc_source_set("neteq_test_tools") {
"../../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",
]

View File

@ -71,12 +71,12 @@ const int kPayloadSizeBytes = kFrameSizeSamples * sizeof(int16_t);
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 PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
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 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
VerifyEncoding();
}
std::unique_ptr<RtpUtility> rtp_utility_;
std::unique_ptr<RtpData> rtp_utility_;
std::unique_ptr<AudioCodingModule> acm_;
PacketizationCallbackStubOldApi packet_cb_;
RTPHeader rtp_header_;

View File

@ -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 @@ void Packet::DeleteRedHeaders(std::list<RTPHeader*>* headers) {
}
}
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.

View File

@ -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 @@ class Packet {
// 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 @@ class Packet {
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);
};

View File

@ -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 @@ RtpFileSource::~RtpFileSource() {}
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<Packet> RtpFileSource::NextPacket() {
}
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 @@ std::unique_ptr<Packet> RtpFileSource::NextPacket() {
RtpFileSource::RtpFileSource(absl::optional<uint32_t> ssrc_filter)
: PacketSource(),
parser_(RtpHeaderParser::Create()),
ssrc_filter_(ssrc_filter) {}
bool RtpFileSource::OpenFile(const std::string& file_name) {

View File

@ -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 @@ class RtpFileSource : public PacketSource {
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);
};