Make packet info optional and only set for primary packets in NetEq.
Header metadata such as audio level and capture time doesn't make sense
for redundant payloads (i.e. RED and inband-FEC).
It is assumed that one of the parsed payload timestamps will correspond
to the RTP header timestamp.
This fixes a bug where capture time and CSRCs were not set after
parsing RED packets.
CreateRedPayload test function is adapted from red_payload_splitter_unittest.cc
Bug: webrtc:15185
Change-Id: Iba58772499b6d760f516854999b60511896b053c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305700
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40240}
diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 113215d..52e8cba 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -506,14 +506,13 @@
PacketList packet_list;
// Insert packet in a packet list.
- packet_list.push_back([&rtp_header, &payload, &receive_time] {
+ packet_list.push_back([&rtp_header, &payload] {
// Convert to Packet.
Packet packet;
packet.payload_type = rtp_header.payloadType;
packet.sequence_number = rtp_header.sequenceNumber;
packet.timestamp = rtp_header.timestamp;
packet.payload.SetData(payload.data(), payload.size());
- packet.packet_info = RtpPacketInfo(rtp_header, receive_time);
// Waiting time will be set upon inserting the packet in the buffer.
RTC_DCHECK(!packet.waiting_time);
return packet;
@@ -630,10 +629,9 @@
parsed_packet_list.splice(parsed_packet_list.end(), packet_list,
packet_list.begin());
} else {
- const auto sequence_number = packet.sequence_number;
- const auto payload_type = packet.payload_type;
+ const uint16_t sequence_number = packet.sequence_number;
+ const uint8_t payload_type = packet.payload_type;
const Packet::Priority original_priority = packet.priority;
- const auto& packet_info = packet.packet_info;
auto packet_from_result = [&](AudioDecoder::ParseResult& result) {
Packet new_packet;
new_packet.sequence_number = sequence_number;
@@ -641,7 +639,10 @@
new_packet.timestamp = result.timestamp;
new_packet.priority.codec_level = result.priority;
new_packet.priority.red_level = original_priority.red_level;
- new_packet.packet_info = packet_info;
+ // Only associate the header information with the primary packet.
+ if (new_packet.timestamp == rtp_header.timestamp) {
+ new_packet.packet_info = RtpPacketInfo(rtp_header, receive_time);
+ }
new_packet.frame = std::move(result.frame);
return new_packet;
};
@@ -1474,8 +1475,9 @@
auto opt_result = packet_list->front().frame->Decode(
rtc::ArrayView<int16_t>(&decoded_buffer_[*decoded_length],
decoded_buffer_length_ - *decoded_length));
- last_decoded_packet_infos_.push_back(
- std::move(packet_list->front().packet_info));
+ if (packet_list->front().packet_info) {
+ last_decoded_packet_infos_.push_back(*packet_list->front().packet_info);
+ }
packet_list->pop_front();
if (opt_result) {
const auto& result = *opt_result;
diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc
index f631937..e61cd52 100644
--- a/modules/audio_coding/neteq/neteq_impl_unittest.cc
+++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -725,6 +725,86 @@
}
}
+std::vector<uint8_t> CreateRedPayload(size_t num_payloads,
+ int payload_type,
+ int payload_size,
+ int timestamp_offset) {
+ constexpr int kRedHeaderLength = 4;
+ const size_t size =
+ payload_size + 1 + (num_payloads - 1) * (payload_size + kRedHeaderLength);
+ std::vector<uint8_t> payload(size, 0);
+ uint8_t* payload_ptr = payload.data();
+ for (size_t i = 0; i < num_payloads; ++i) {
+ // Write the RED headers.
+ if (i == num_payloads - 1) {
+ // Special case for last payload.
+ *payload_ptr = payload_type & 0x7F; // F = 0;
+ ++payload_ptr;
+ break;
+ }
+ *payload_ptr = payload_type & 0x7F;
+ // Not the last block; set F = 1.
+ *payload_ptr |= 0x80;
+ ++payload_ptr;
+ const int this_offset =
+ rtc::checked_cast<int>((num_payloads - i - 1) * timestamp_offset);
+ *payload_ptr = this_offset >> 6;
+ ++payload_ptr;
+ RTC_DCHECK_LE(payload_size, 1023); // Max length described by 10 bits.
+ *payload_ptr = ((this_offset & 0x3F) << 2) | (payload_size >> 8);
+ ++payload_ptr;
+ *payload_ptr = payload_size & 0xFF;
+ ++payload_ptr;
+ }
+ return payload;
+}
+
+TEST_F(NetEqImplTest, InsertRedPayload) {
+ UseNoMocks();
+ CreateInstance();
+ constexpr int kRedPayloadType = 7;
+ neteq_->RegisterPayloadType(kRedPayloadType, SdpAudioFormat("red", 8000, 1));
+ constexpr int kPayloadType = 8;
+ neteq_->RegisterPayloadType(kPayloadType, SdpAudioFormat("l16", 8000, 1));
+ size_t frame_size = 80; // 10 ms.
+ size_t payload_size = frame_size * 2;
+ std::vector<uint8_t> payload =
+ CreateRedPayload(3, kPayloadType, payload_size, frame_size);
+ RTPHeader header;
+ header.payloadType = kRedPayloadType;
+ header.sequenceNumber = 0x1234;
+ header.timestamp = 0x12345678;
+ header.ssrc = 0x87654321;
+ AbsoluteCaptureTime capture_time;
+ capture_time.absolute_capture_timestamp = 1234;
+ header.extension.absolute_capture_time = capture_time;
+ header.extension.audioLevel = 12;
+ header.extension.hasAudioLevel = true;
+ header.numCSRCs = 1;
+ header.arrOfCSRCs[0] = 123;
+ neteq_->InsertPacket(header, payload);
+ AudioFrame frame;
+ bool muted;
+ neteq_->GetAudio(&frame, &muted);
+ // TODO(jakobi): Find a better way to test that the correct packet is decoded
+ // than using the timestamp. The fixed NetEq delay is an implementation
+ // detail that should not be tested.
+ constexpr int kNetEqFixedDelay = 5;
+ EXPECT_EQ(frame.timestamp_,
+ header.timestamp - frame_size * 2 - kNetEqFixedDelay);
+ EXPECT_TRUE(frame.packet_infos_.empty());
+ neteq_->GetAudio(&frame, &muted);
+ EXPECT_EQ(frame.timestamp_, header.timestamp - frame_size - kNetEqFixedDelay);
+ EXPECT_TRUE(frame.packet_infos_.empty());
+ neteq_->GetAudio(&frame, &muted);
+ EXPECT_EQ(frame.timestamp_, header.timestamp - kNetEqFixedDelay);
+ EXPECT_EQ(frame.packet_infos_.size(), 1u);
+ EXPECT_EQ(frame.packet_infos_.front().absolute_capture_time(), capture_time);
+ EXPECT_EQ(frame.packet_infos_.front().audio_level(),
+ header.extension.audioLevel);
+ EXPECT_EQ(frame.packet_infos_.front().csrcs()[0], header.arrOfCSRCs[0]);
+}
+
// This test verifies that audio interruption is not logged for the initial
// PLC period before the first packet is deocoded.
// TODO(henrik.lundin) Maybe move this test to neteq_network_stats_unittest.cc.
diff --git a/modules/audio_coding/neteq/packet.h b/modules/audio_coding/neteq/packet.h
index 0c6f204..795c36d 100644
--- a/modules/audio_coding/neteq/packet.h
+++ b/modules/audio_coding/neteq/packet.h
@@ -74,7 +74,7 @@
// Datagram excluding RTP header and header extension.
rtc::Buffer payload;
Priority priority;
- RtpPacketInfo packet_info;
+ absl::optional<RtpPacketInfo> packet_info;
std::unique_ptr<TickTimer::Stopwatch> waiting_time;
std::unique_ptr<AudioDecoder::EncodedAudioFrame> frame;
diff --git a/modules/audio_coding/neteq/red_payload_splitter.cc b/modules/audio_coding/neteq/red_payload_splitter.cc
index cec9f2f..992cd28 100644
--- a/modules/audio_coding/neteq/red_payload_splitter.cc
+++ b/modules/audio_coding/neteq/red_payload_splitter.cc
@@ -39,7 +39,7 @@
bool ret = true;
PacketList::iterator it = packet_list->begin();
while (it != packet_list->end()) {
- const Packet& red_packet = *it;
+ Packet& red_packet = *it;
RTC_DCHECK(!red_packet.payload.empty());
const uint8_t* payload_ptr = red_packet.payload.data();
size_t payload_length = red_packet.payload.size();
@@ -132,13 +132,6 @@
new_packet.priority.red_level =
rtc::dchecked_cast<int>((new_headers.size() - 1) - i);
new_packet.payload.SetData(payload_ptr, payload_length);
- new_packet.packet_info = RtpPacketInfo(
- /*ssrc=*/red_packet.packet_info.ssrc(),
- /*csrcs=*/std::vector<uint32_t>(),
- /*rtp_timestamp=*/new_packet.timestamp,
- /*receive_time=*/red_packet.packet_info.receive_time());
- new_packet.packet_info.set_audio_level(
- red_packet.packet_info.audio_level());
new_packets.push_front(std::move(new_packet));
payload_ptr += payload_length;
}
