Avoid using legacy rtp parser in neteq test::Packet
Bug: None
Change-Id: I9184954d9c99f0a34ae335d03843171864071e5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222648
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34316}
diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index 445b314..28d30d3 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -1058,6 +1058,7 @@
deps = [
":default_neteq_factory",
":neteq",
+ "../../api:array_view",
"../../api:neteq_simulator_api",
"../../api:rtp_headers",
"../../api/audio:audio_frame_api",
@@ -1068,7 +1069,6 @@
"../../rtc_base:checks",
"../../rtc_base:rtc_base_approved",
"../../system_wrappers",
- "../rtp_rtcp",
"../rtp_rtcp:rtp_rtcp_format",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
diff --git a/modules/audio_coding/acm2/acm_send_test.cc b/modules/audio_coding/acm2/acm_send_test.cc
index b3e1e1e..6f395d6 100644
--- a/modules/audio_coding/acm2/acm_send_test.cc
+++ b/modules/audio_coding/acm2/acm_send_test.cc
@@ -140,8 +140,9 @@
std::unique_ptr<Packet> AcmSendTestOldApi::CreatePacket() {
const size_t kRtpHeaderSize = 12;
- size_t allocated_bytes = last_payload_vec_.size() + kRtpHeaderSize;
- uint8_t* packet_memory = new uint8_t[allocated_bytes];
+ rtc::CopyOnWriteBuffer packet_buffer(last_payload_vec_.size() +
+ kRtpHeaderSize);
+ uint8_t* packet_memory = packet_buffer.MutableData();
// Populate the header bytes.
packet_memory[0] = 0x80;
packet_memory[1] = static_cast<uint8_t>(payload_type_);
@@ -162,8 +163,8 @@
// Copy the payload data.
memcpy(packet_memory + kRtpHeaderSize, &last_payload_vec_[0],
last_payload_vec_.size());
- std::unique_ptr<Packet> packet(
- new Packet(packet_memory, allocated_bytes, clock_.TimeInMilliseconds()));
+ auto packet = std::make_unique<Packet>(std::move(packet_buffer),
+ clock_.TimeInMilliseconds());
RTC_DCHECK(packet);
RTC_DCHECK(packet->valid_header());
return packet;
diff --git a/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc b/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
index 6b325b6..6cbba20 100644
--- a/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
+++ b/modules/audio_coding/neteq/tools/constant_pcm_packet_source.cc
@@ -37,14 +37,15 @@
std::unique_ptr<Packet> ConstantPcmPacketSource::NextPacket() {
RTC_CHECK_GT(packet_len_bytes_, kHeaderLenBytes);
- uint8_t* packet_memory = new uint8_t[packet_len_bytes_];
+ rtc::CopyOnWriteBuffer packet_buffer(packet_len_bytes_);
+ uint8_t* packet_memory = packet_buffer.MutableData();
// Fill the payload part of the packet memory with the pre-encoded value.
for (unsigned i = 0; i < 2 * payload_len_samples_; ++i)
packet_memory[kHeaderLenBytes + i] = encoded_sample_[i % 2];
WriteHeader(packet_memory);
// |packet| assumes ownership of |packet_memory|.
- std::unique_ptr<Packet> packet(
- new Packet(packet_memory, packet_len_bytes_, next_arrival_time_ms_));
+ auto packet =
+ std::make_unique<Packet>(std::move(packet_buffer), next_arrival_time_ms_);
next_arrival_time_ms_ += payload_len_samples_ / samples_per_ms_;
return packet;
}
diff --git a/modules/audio_coding/neteq/tools/packet.cc b/modules/audio_coding/neteq/tools/packet.cc
index 48959e4..e540173 100644
--- a/modules/audio_coding/neteq/tools/packet.cc
+++ b/modules/audio_coding/neteq/tools/packet.cc
@@ -10,30 +10,22 @@
#include "modules/audio_coding/neteq/tools/packet.h"
-#include <string.h>
-
-#include <memory>
-
-#include "modules/rtp_rtcp/source/rtp_utility.h"
+#include "api/array_view.h"
+#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "rtc_base/checks.h"
+#include "rtc_base/copy_on_write_buffer.h"
namespace webrtc {
namespace test {
-using webrtc::RtpUtility::RtpHeaderParser;
-
-Packet::Packet(uint8_t* packet_memory,
- size_t allocated_bytes,
+Packet::Packet(rtc::CopyOnWriteBuffer packet,
size_t virtual_packet_length_bytes,
double time_ms,
- const RtpUtility::RtpHeaderParser& parser,
- const RtpHeaderExtensionMap* extension_map /*= nullptr*/)
- : payload_memory_(packet_memory),
- packet_length_bytes_(allocated_bytes),
+ const RtpHeaderExtensionMap* extension_map)
+ : packet_(std::move(packet)),
virtual_packet_length_bytes_(virtual_packet_length_bytes),
- virtual_payload_length_bytes_(0),
time_ms_(time_ms),
- valid_header_(ParseHeader(parser, extension_map)) {}
+ valid_header_(ParseHeader(extension_map)) {}
Packet::Packet(const RTPHeader& header,
size_t virtual_packet_length_bytes,
@@ -45,23 +37,6 @@
time_ms_(time_ms),
valid_header_(true) {}
-Packet::Packet(uint8_t* packet_memory, size_t allocated_bytes, double time_ms)
- : 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)
- : Packet(packet_memory,
- allocated_bytes,
- virtual_packet_length_bytes,
- time_ms,
- RtpUtility::RtpHeaderParser(packet_memory, allocated_bytes)) {}
-
Packet::~Packet() = default;
bool Packet::ExtractRedHeaders(std::list<RTPHeader*>* headers) const {
@@ -77,9 +52,8 @@
// +-+-+-+-+-+-+-+-+
//
- RTC_DCHECK(payload_);
- const uint8_t* payload_ptr = payload_;
- const uint8_t* payload_end_ptr = payload_ptr + payload_length_bytes_;
+ const uint8_t* payload_ptr = payload();
+ const uint8_t* payload_end_ptr = payload_ptr + payload_length_bytes();
// Find all RED headers with the extension bit set to 1. That is, all headers
// but the last one.
@@ -111,27 +85,43 @@
}
}
-bool Packet::ParseHeader(const RtpHeaderParser& parser,
- const RtpHeaderExtensionMap* extension_map) {
- bool valid_header = parser.Parse(&header_, extension_map);
+bool Packet::ParseHeader(const RtpHeaderExtensionMap* extension_map) {
+ // Use RtpPacketReceived instead of RtpPacket because former already has a
+ // converter into legacy RTPHeader.
+ webrtc::RtpPacketReceived rtp_packet(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.
- const bool header_only_with_padding =
- (header_.headerLength == packet_length_bytes_ &&
- header_.paddingLength > 0);
- if (!valid_header && !header_only_with_padding) {
- return false;
+ // Because of the special case of dummy packets that have padding marked in
+ // the RTP header, but do not have rtp payload with the padding size, handle
+ // padding manually. Regular RTP packet parser reports failure, but it is fine
+ // in this context.
+ bool padding = (packet_[0] & 0b0010'0000);
+ size_t padding_size = 0;
+ if (padding) {
+ // Clear the padding bit to prevent failure when rtp payload is omited.
+ rtc::CopyOnWriteBuffer packet(packet_);
+ packet.MutableData()[0] &= ~0b0010'0000;
+ if (!rtp_packet.Parse(std::move(packet))) {
+ return false;
+ }
+ if (rtp_packet.payload_size() > 0) {
+ padding_size = rtp_packet.data()[rtp_packet.size() - 1];
+ }
+ if (padding_size > rtp_packet.payload_size()) {
+ return false;
+ }
+ } else {
+ if (!rtp_packet.Parse(packet_)) {
+ return false;
+ }
}
- RTC_DCHECK_LE(header_.headerLength, packet_length_bytes_);
- payload_ = &payload_memory_[header_.headerLength];
- RTC_DCHECK_GE(packet_length_bytes_, header_.headerLength);
- payload_length_bytes_ = packet_length_bytes_ - header_.headerLength;
- RTC_CHECK_GE(virtual_packet_length_bytes_, packet_length_bytes_);
- RTC_DCHECK_GE(virtual_packet_length_bytes_, header_.headerLength);
+ rtp_payload_ = rtc::MakeArrayView(packet_.data() + rtp_packet.headers_size(),
+ rtp_packet.payload_size() - padding_size);
+ rtp_packet.GetHeader(&header_);
+
+ RTC_CHECK_GE(virtual_packet_length_bytes_, rtp_packet.size());
+ RTC_DCHECK_GE(virtual_packet_length_bytes_, rtp_packet.headers_size());
virtual_payload_length_bytes_ =
- virtual_packet_length_bytes_ - header_.headerLength;
+ virtual_packet_length_bytes_ - rtp_packet.headers_size();
return true;
}
diff --git a/modules/audio_coding/neteq/tools/packet.h b/modules/audio_coding/neteq/tools/packet.h
index f4189aa..ef118d9 100644
--- a/modules/audio_coding/neteq/tools/packet.h
+++ b/modules/audio_coding/neteq/tools/packet.h
@@ -12,62 +12,46 @@
#define MODULES_AUDIO_CODING_NETEQ_TOOLS_PACKET_H_
#include <list>
-#include <memory>
-#include "api/rtp_headers.h" // NOLINT(build/include)
+#include "api/array_view.h"
+#include "api/rtp_headers.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "rtc_base/constructor_magic.h"
+#include "rtc_base/copy_on_write_buffer.h"
namespace webrtc {
-
-namespace RtpUtility {
-class RtpHeaderParser;
-} // namespace RtpUtility
-
namespace test {
// Class for handling RTP packets in test applications.
class Packet {
public:
// Creates a packet, with the packet payload (including header bytes) in
- // |packet_memory|. The length of |packet_memory| is |allocated_bytes|.
- // The new object assumes ownership of |packet_memory| and will delete it
- // 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|.
- // |virtual_packet_length_bytes| is typically used when reading RTP dump files
+ // `packet`. The `time_ms` is an extra time associated with this packet,
+ // typically used to denote arrival time.
+ // `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,
+ // RTP light). The `virtual_packet_length_bytes` tells what size the packet
+ // had on wire, including the now discarded payload.
+ Packet(rtc::CopyOnWriteBuffer packet,
size_t virtual_packet_length_bytes,
double time_ms,
- const RtpUtility::RtpHeaderParser& parser,
const RtpHeaderExtensionMap* extension_map = nullptr);
+ Packet(rtc::CopyOnWriteBuffer packet,
+ double time_ms,
+ const RtpHeaderExtensionMap* extension_map = nullptr)
+ : Packet(packet, packet.size(), time_ms, extension_map) {}
+
// 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
- // RTP headers, and no payload. The |virtual_packet_length_bytes| tells what
+ // RTP headers, and no payload. The `virtual_packet_length_bytes` tells what
// size the packet had on wire, including the now discarded payload,
- // The |virtual_payload_length_bytes| tells the size of the payload.
+ // The `virtual_payload_length_bytes` tells the size of the payload.
Packet(const RTPHeader& header,
size_t virtual_packet_length_bytes,
size_t virtual_payload_length_bytes,
double time_ms);
- // The following constructors are the same as the first two, but without a
- // parser. Note that when the object is constructed using any of these
- // methods, the header will be parsed using a default RtpHeaderParser object.
- // In particular, RTP header extensions won't be parsed.
- Packet(uint8_t* packet_memory, size_t allocated_bytes, double time_ms);
-
- Packet(uint8_t* packet_memory,
- size_t allocated_bytes,
- size_t virtual_packet_length_bytes,
- double time_ms);
-
virtual ~Packet();
// Parses the first bytes of the RTP payload, interpreting them as RED headers
@@ -80,11 +64,11 @@
// itself.
static void DeleteRedHeaders(std::list<RTPHeader*>* headers);
- const uint8_t* payload() const { return payload_; }
+ const uint8_t* payload() const { return rtp_payload_.data(); }
- size_t packet_length_bytes() const { return packet_length_bytes_; }
+ size_t packet_length_bytes() const { return packet_.size(); }
- size_t payload_length_bytes() const { return payload_length_bytes_; }
+ size_t payload_length_bytes() const { return rtp_payload_.size(); }
size_t virtual_packet_length_bytes() const {
return virtual_packet_length_bytes_;
@@ -100,21 +84,17 @@
bool valid_header() const { return valid_header_; }
private:
- bool ParseHeader(const webrtc::RtpUtility::RtpHeaderParser& parser,
- const RtpHeaderExtensionMap* extension_map);
+ bool ParseHeader(const RtpHeaderExtensionMap* extension_map);
void CopyToHeader(RTPHeader* destination) const;
RTPHeader header_;
- 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.
+ const rtc::CopyOnWriteBuffer packet_;
+ rtc::ArrayView<const uint8_t> rtp_payload_; // Empty 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_ = 0;
const double time_ms_; // Used to denote a packet's arrival time.
- const bool valid_header_; // Set by the RtpHeaderParser.
+ const bool valid_header_;
RTC_DISALLOW_COPY_AND_ASSIGN(Packet);
};
diff --git a/modules/audio_coding/neteq/tools/packet_unittest.cc b/modules/audio_coding/neteq/tools/packet_unittest.cc
index 7f3d663..7cc9a48 100644
--- a/modules/audio_coding/neteq/tools/packet_unittest.cc
+++ b/modules/audio_coding/neteq/tools/packet_unittest.cc
@@ -42,16 +42,15 @@
TEST(TestPacket, RegularPacket) {
const size_t kPacketLengthBytes = 100;
- uint8_t* packet_memory = new uint8_t[kPacketLengthBytes];
+ rtc::CopyOnWriteBuffer packet_memory(kPacketLengthBytes);
const uint8_t kPayloadType = 17;
const uint16_t kSequenceNumber = 4711;
const uint32_t kTimestamp = 47114711;
const uint32_t kSsrc = 0x12345678;
MakeRtpHeader(kPayloadType, kSequenceNumber, kTimestamp, kSsrc,
- packet_memory);
+ packet_memory.MutableData());
const double kPacketTime = 1.0;
- // Hand over ownership of |packet_memory| to |packet|.
- Packet packet(packet_memory, kPacketLengthBytes, kPacketTime);
+ Packet packet(std::move(packet_memory), kPacketTime);
ASSERT_TRUE(packet.valid_header());
EXPECT_EQ(kPayloadType, packet.header().payloadType);
EXPECT_EQ(kSequenceNumber, packet.header().sequenceNumber);
@@ -70,16 +69,44 @@
TEST(TestPacket, DummyPacket) {
const size_t kPacketLengthBytes = kHeaderLengthBytes; // Only RTP header.
const size_t kVirtualPacketLengthBytes = 100;
- uint8_t* packet_memory = new uint8_t[kPacketLengthBytes];
+ rtc::CopyOnWriteBuffer packet_memory(kPacketLengthBytes);
const uint8_t kPayloadType = 17;
const uint16_t kSequenceNumber = 4711;
const uint32_t kTimestamp = 47114711;
const uint32_t kSsrc = 0x12345678;
MakeRtpHeader(kPayloadType, kSequenceNumber, kTimestamp, kSsrc,
- packet_memory);
+ packet_memory.MutableData());
const double kPacketTime = 1.0;
- // Hand over ownership of |packet_memory| to |packet|.
- Packet packet(packet_memory, kPacketLengthBytes, kVirtualPacketLengthBytes,
+ Packet packet(std::move(packet_memory), kVirtualPacketLengthBytes,
+ kPacketTime);
+ ASSERT_TRUE(packet.valid_header());
+ EXPECT_EQ(kPayloadType, packet.header().payloadType);
+ EXPECT_EQ(kSequenceNumber, packet.header().sequenceNumber);
+ EXPECT_EQ(kTimestamp, packet.header().timestamp);
+ EXPECT_EQ(kSsrc, packet.header().ssrc);
+ EXPECT_EQ(0, packet.header().numCSRCs);
+ EXPECT_EQ(kPacketLengthBytes, packet.packet_length_bytes());
+ EXPECT_EQ(kPacketLengthBytes - kHeaderLengthBytes,
+ packet.payload_length_bytes());
+ EXPECT_EQ(kVirtualPacketLengthBytes, packet.virtual_packet_length_bytes());
+ EXPECT_EQ(kVirtualPacketLengthBytes - kHeaderLengthBytes,
+ packet.virtual_payload_length_bytes());
+ EXPECT_EQ(kPacketTime, packet.time_ms());
+}
+
+TEST(TestPacket, DummyPaddingPacket) {
+ const size_t kPacketLengthBytes = kHeaderLengthBytes; // Only RTP header.
+ const size_t kVirtualPacketLengthBytes = 100;
+ rtc::CopyOnWriteBuffer packet_memory(kPacketLengthBytes);
+ const uint8_t kPayloadType = 17;
+ const uint16_t kSequenceNumber = 4711;
+ const uint32_t kTimestamp = 47114711;
+ const uint32_t kSsrc = 0x12345678;
+ MakeRtpHeader(kPayloadType, kSequenceNumber, kTimestamp, kSsrc,
+ packet_memory.MutableData());
+ packet_memory.MutableData()[0] |= 0b0010'0000; // Set the padding bit.
+ const double kPacketTime = 1.0;
+ Packet packet(std::move(packet_memory), kVirtualPacketLengthBytes,
kPacketTime);
ASSERT_TRUE(packet.valid_header());
EXPECT_EQ(kPayloadType, packet.header().payloadType);
@@ -133,19 +160,19 @@
TEST(TestPacket, RED) {
const size_t kPacketLengthBytes = 100;
- uint8_t* packet_memory = new uint8_t[kPacketLengthBytes];
+ rtc::CopyOnWriteBuffer packet_memory(kPacketLengthBytes);
const uint8_t kRedPayloadType = 17;
const uint16_t kSequenceNumber = 4711;
const uint32_t kTimestamp = 47114711;
const uint32_t kSsrc = 0x12345678;
MakeRtpHeader(kRedPayloadType, kSequenceNumber, kTimestamp, kSsrc,
- packet_memory);
+ packet_memory.MutableData());
// Create four RED headers.
// Payload types are just the same as the block index the offset is 100 times
// the block index.
const int kRedBlocks = 4;
- uint8_t* payload_ptr =
- &packet_memory[kHeaderLengthBytes]; // First byte after header.
+ uint8_t* payload_ptr = packet_memory.MutableData() +
+ kHeaderLengthBytes; // First byte after header.
for (int i = 0; i < kRedBlocks; ++i) {
int payload_type = i;
// Offset value is not used for the last block.
diff --git a/modules/audio_coding/neteq/tools/rtp_file_source.cc b/modules/audio_coding/neteq/tools/rtp_file_source.cc
index 7852330..16b225e 100644
--- a/modules/audio_coding/neteq/tools/rtp_file_source.cc
+++ b/modules/audio_coding/neteq/tools/rtp_file_source.cc
@@ -62,12 +62,9 @@
// Read the next one.
continue;
}
- std::unique_ptr<uint8_t[]> packet_memory(new uint8_t[temp_packet.length]);
- memcpy(packet_memory.get(), temp_packet.data, temp_packet.length);
- RtpUtility::RtpHeaderParser parser(packet_memory.get(), temp_packet.length);
auto packet = std::make_unique<Packet>(
- packet_memory.release(), temp_packet.length,
- temp_packet.original_length, temp_packet.time_ms, parser,
+ rtc::CopyOnWriteBuffer(temp_packet.data, temp_packet.length),
+ temp_packet.original_length, temp_packet.time_ms,
&rtp_header_extension_map_);
if (!packet->valid_header()) {
continue;