Skip to content

Commit 70c09bd

Browse files
author
Henrik Lundin
committed
Reland of Change NetEq::InsertPacket to take an RTPHeader (patchset #1 id:1 of https://codereview.webrtc.org/2812933002/ )
Reason for revert: Downstream roadblock should be cleared by now. Relanding original patch. Original issue's description: > Revert of Change NetEq::InsertPacket to take an RTPHeader (patchset #2 id:20001 of https://codereview.webrtc.org/2807273004/ ) > > Reason for revert: > Broke downstream dependencies. > > Original issue's description: > > Change NetEq::InsertPacket to take an RTPHeader > > > > It used to take a WebRtcRTPHeader as input, which has an RTPHeader as > > a member. None of the other member in WebRtcRTPHeader where used in > > NetEq. > > > > This CL adapts the production code; tests and tools will be converted > > in a follow-up CL. > > > > BUG=webrtc:7467 > > > > Review-Url: https://codereview.webrtc.org/2807273004 > > Cr-Commit-Position: refs/heads/master@{#17652} > > Committed: https://chromium.googlesource.com/external/webrtc/+/4d027576a6f7420fc4ec6be7f4f991cfad34b826 > > [email protected] > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=webrtc:7467 > > Review-Url: https://codereview.webrtc.org/2812933002 > Cr-Commit-Position: refs/heads/master@{#17657} > Committed: https://chromium.googlesource.com/external/webrtc/+/10d095d4f743bc16f8e486e156c48a6d023b32c5 [email protected] # Not skipping CQ checks because original CL landed more than 1 days ago. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng BUG=webrtc:7467 Review-Url: https://codereview.webrtc.org/2835093002 . Cr-Commit-Position: refs/heads/master@{#17843}
1 parent 7a7b336 commit 70c09bd

12 files changed

+99
-97
lines changed

‎webrtc/modules/audio_coding/acm2/acm_receiver.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ int AcmReceiver::InsertPacket(const WebRtcRTPHeader& rtp_header,
104104
}
105105
} // |crit_sect_| is released.
106106

107-
if (neteq_->InsertPacket(rtp_header, incoming_payload, receive_timestamp) <
108-
0) {
107+
if (neteq_->InsertPacket(rtp_header.header, incoming_payload,
108+
receive_timestamp) < 0) {
109109
LOG(LERROR) << "AcmReceiver::InsertPacket "
110110
<< static_cast<int>(header->payloadType)
111111
<< " Failed to insert packet";

‎webrtc/modules/audio_coding/neteq/include/neteq.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ namespace webrtc {
2626

2727
// Forward declarations.
2828
class AudioFrame;
29-
struct WebRtcRTPHeader;
3029
class AudioDecoderFactory;
3130

3231
struct NetEqNetworkStatistics {
@@ -141,7 +140,7 @@ class NetEq {
141140
// of the time when the packet was received, and should be measured with
142141
// the same tick rate as the RTP timestamp of the current payload.
143142
// Returns 0 on success, -1 on failure.
144-
virtual int InsertPacket(const WebRtcRTPHeader& rtp_header,
143+
virtual int InsertPacket(const RTPHeader& rtp_header,
145144
rtc::ArrayView<const uint8_t> payload,
146145
uint32_t receive_timestamp) = 0;
147146

‎webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,8 @@ class NetEqExternalVsInternalDecoderTest : public NetEqExternalDecoderUnitTest,
210210
rtc::ArrayView<const uint8_t> payload,
211211
uint32_t receive_timestamp) override {
212212
// Insert packet in internal decoder.
213-
ASSERT_EQ(NetEq::kOK, neteq_internal_->InsertPacket(rtp_header, payload,
214-
receive_timestamp));
213+
ASSERT_EQ(NetEq::kOK, neteq_internal_->InsertPacket(
214+
rtp_header.header, payload, receive_timestamp));
215215

216216
// Insert packet in external decoder instance.
217217
NetEqExternalDecoderUnitTest::InsertPacket(rtp_header, payload,

‎webrtc/modules/audio_coding/neteq/neteq_impl.cc

+15-15
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ NetEqImpl::NetEqImpl(const NetEq::Config& config,
131131

132132
NetEqImpl::~NetEqImpl() = default;
133133

134-
int NetEqImpl::InsertPacket(const WebRtcRTPHeader& rtp_header,
134+
int NetEqImpl::InsertPacket(const RTPHeader& rtp_header,
135135
rtc::ArrayView<const uint8_t> payload,
136136
uint32_t receive_timestamp) {
137137
rtc::MsanCheckInitialized(payload);
@@ -581,7 +581,7 @@ Operations NetEqImpl::last_operation_for_test() const {
581581

582582
// Methods below this line are private.
583583

584-
int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
584+
int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header,
585585
rtc::ArrayView<const uint8_t> payload,
586586
uint32_t receive_timestamp) {
587587
if (payload.empty()) {
@@ -594,24 +594,24 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
594594
packet_list.push_back([&rtp_header, &payload] {
595595
// Convert to Packet.
596596
Packet packet;
597-
packet.payload_type = rtp_header.header.payloadType;
598-
packet.sequence_number = rtp_header.header.sequenceNumber;
599-
packet.timestamp = rtp_header.header.timestamp;
597+
packet.payload_type = rtp_header.payloadType;
598+
packet.sequence_number = rtp_header.sequenceNumber;
599+
packet.timestamp = rtp_header.timestamp;
600600
packet.payload.SetData(payload.data(), payload.size());
601601
// Waiting time will be set upon inserting the packet in the buffer.
602602
RTC_DCHECK(!packet.waiting_time);
603603
return packet;
604604
}());
605605

606-
bool update_sample_rate_and_channels = first_packet_ ||
607-
(rtp_header.header.ssrc != ssrc_);
606+
bool update_sample_rate_and_channels =
607+
first_packet_ || (rtp_header.ssrc != ssrc_);
608608

609609
if (update_sample_rate_and_channels) {
610610
// Reset timestamp scaling.
611611
timestamp_scaler_->Reset();
612612
}
613613

614-
if (!decoder_database_->IsRed(rtp_header.header.payloadType)) {
614+
if (!decoder_database_->IsRed(rtp_header.payloadType)) {
615615
// Scale timestamp to internal domain (only for some codecs).
616616
timestamp_scaler_->ToInternal(&packet_list);
617617
}
@@ -627,14 +627,14 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
627627
// Note: |first_packet_| will be cleared further down in this method, once
628628
// the packet has been successfully inserted into the packet buffer.
629629

630-
rtcp_.Init(rtp_header.header.sequenceNumber);
630+
rtcp_.Init(rtp_header.sequenceNumber);
631631

632632
// Flush the packet buffer and DTMF buffer.
633633
packet_buffer_->Flush();
634634
dtmf_buffer_->Flush();
635635

636636
// Store new SSRC.
637-
ssrc_ = rtp_header.header.ssrc;
637+
ssrc_ = rtp_header.ssrc;
638638

639639
// Update audio buffer timestamp.
640640
sync_buffer_->IncreaseEndTimestamp(main_timestamp - timestamp_);
@@ -644,19 +644,19 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
644644
}
645645

646646
// Update RTCP statistics, only for regular packets.
647-
rtcp_.Update(rtp_header.header, receive_timestamp);
647+
rtcp_.Update(rtp_header, receive_timestamp);
648648

649649
if (nack_enabled_) {
650650
RTC_DCHECK(nack_);
651651
if (update_sample_rate_and_channels) {
652652
nack_->Reset();
653653
}
654-
nack_->UpdateLastReceivedPacket(rtp_header.header.sequenceNumber,
655-
rtp_header.header.timestamp);
654+
nack_->UpdateLastReceivedPacket(rtp_header.sequenceNumber,
655+
rtp_header.timestamp);
656656
}
657657

658658
// Check for RED payload type, and separate payloads into several packets.
659-
if (decoder_database_->IsRed(rtp_header.header.payloadType)) {
659+
if (decoder_database_->IsRed(rtp_header.payloadType)) {
660660
if (!red_payload_splitter_->SplitRed(&packet_list)) {
661661
return kRedundancySplitError;
662662
}
@@ -675,7 +675,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
675675

676676
// Update main_timestamp, if new packets appear in the list
677677
// after RED splitting.
678-
if (decoder_database_->IsRed(rtp_header.header.payloadType)) {
678+
if (decoder_database_->IsRed(rtp_header.payloadType)) {
679679
timestamp_scaler_->ToInternal(&packet_list);
680680
main_timestamp = packet_list.front().timestamp;
681681
main_payload_type = packet_list.front().payload_type;

‎webrtc/modules/audio_coding/neteq/neteq_impl.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class NetEqImpl : public webrtc::NetEq {
105105
// of the time when the packet was received, and should be measured with
106106
// the same tick rate as the RTP timestamp of the current payload.
107107
// Returns 0 on success, -1 on failure.
108-
int InsertPacket(const WebRtcRTPHeader& rtp_header,
108+
int InsertPacket(const RTPHeader& rtp_header,
109109
rtc::ArrayView<const uint8_t> payload,
110110
uint32_t receive_timestamp) override;
111111

@@ -222,7 +222,7 @@ class NetEqImpl : public webrtc::NetEq {
222222
// Inserts a new packet into NetEq. This is used by the InsertPacket method
223223
// above. Returns 0 on success, otherwise an error code.
224224
// TODO(hlundin): Merge this with InsertPacket above?
225-
int InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
225+
int InsertPacketInternal(const RTPHeader& rtp_header,
226226
rtc::ArrayView<const uint8_t> payload,
227227
uint32_t receive_timestamp)
228228
EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);

‎webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc

+21-21
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ class NetEqImplTest : public ::testing::Test {
193193

194194
// Insert first packet.
195195
EXPECT_EQ(NetEq::kOK,
196-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
196+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
197197

198198
// Pull audio once.
199199
const size_t kMaxOutputSize =
@@ -384,12 +384,12 @@ TEST_F(NetEqImplTest, InsertPacket) {
384384
}
385385

386386
// Insert first packet.
387-
neteq_->InsertPacket(rtp_header, payload, kFirstReceiveTime);
387+
neteq_->InsertPacket(rtp_header.header, payload, kFirstReceiveTime);
388388

389389
// Insert second packet.
390390
rtp_header.header.timestamp += 160;
391391
rtp_header.header.sequenceNumber += 1;
392-
neteq_->InsertPacket(rtp_header, payload, kFirstReceiveTime + 155);
392+
neteq_->InsertPacket(rtp_header.header, payload, kFirstReceiveTime + 155);
393393
}
394394

395395
TEST_F(NetEqImplTest, InsertPacketsUntilBufferIsFull) {
@@ -413,7 +413,7 @@ TEST_F(NetEqImplTest, InsertPacketsUntilBufferIsFull) {
413413
// Insert packets. The buffer should not flush.
414414
for (size_t i = 1; i <= config_.max_packets_in_buffer; ++i) {
415415
EXPECT_EQ(NetEq::kOK,
416-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
416+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
417417
rtp_header.header.timestamp += kPayloadLengthSamples;
418418
rtp_header.header.sequenceNumber += 1;
419419
EXPECT_EQ(i, packet_buffer_->NumPacketsInBuffer());
@@ -422,7 +422,7 @@ TEST_F(NetEqImplTest, InsertPacketsUntilBufferIsFull) {
422422
// Insert one more packet and make sure the buffer got flushed. That is, it
423423
// should only hold one single packet.
424424
EXPECT_EQ(NetEq::kOK,
425-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
425+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
426426
EXPECT_EQ(1u, packet_buffer_->NumPacketsInBuffer());
427427
const Packet* test_packet = packet_buffer_->PeekNextPacket();
428428
EXPECT_EQ(rtp_header.header.timestamp, test_packet->timestamp);
@@ -502,7 +502,7 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) {
502502

503503
// Insert one packet.
504504
EXPECT_EQ(NetEq::kOK,
505-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
505+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
506506

507507
// Pull audio once.
508508
const size_t kMaxOutputSize = static_cast<size_t>(10 * kSampleRateHz / 1000);
@@ -583,7 +583,7 @@ TEST_F(NetEqImplTest, ReorderedPacket) {
583583

584584
// Insert one packet.
585585
EXPECT_EQ(NetEq::kOK,
586-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
586+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
587587

588588
// Pull audio once.
589589
const size_t kMaxOutputSize = static_cast<size_t>(10 * kSampleRateHz / 1000);
@@ -600,12 +600,12 @@ TEST_F(NetEqImplTest, ReorderedPacket) {
600600
rtp_header.header.timestamp -= kPayloadLengthSamples;
601601
payload[0] = 1;
602602
EXPECT_EQ(NetEq::kOK,
603-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
603+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
604604
rtp_header.header.sequenceNumber += 2;
605605
rtp_header.header.timestamp += 2 * kPayloadLengthSamples;
606606
payload[0] = 2;
607607
EXPECT_EQ(NetEq::kOK,
608-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
608+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
609609

610610
// Expect only the second packet to be decoded (the one with "2" as the first
611611
// payload byte).
@@ -651,7 +651,7 @@ TEST_F(NetEqImplTest, FirstPacketUnknown) {
651651
// Insert one packet. Note that we have not registered any payload type, so
652652
// this packet will be rejected.
653653
EXPECT_EQ(NetEq::kFail,
654-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
654+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
655655
EXPECT_EQ(NetEq::kUnknownRtpPayloadType, neteq_->LastError());
656656

657657
// Pull audio once.
@@ -673,7 +673,7 @@ TEST_F(NetEqImplTest, FirstPacketUnknown) {
673673
rtp_header.header.sequenceNumber++;
674674
rtp_header.header.timestamp += kPayloadLengthSamples;
675675
EXPECT_EQ(NetEq::kOK,
676-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
676+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
677677
EXPECT_EQ(i + 1, packet_buffer_->NumPacketsInBuffer());
678678
}
679679

@@ -760,14 +760,14 @@ TEST_F(NetEqImplTest, CodecInternalCng) {
760760

761761
// Insert one packet (decoder will return speech).
762762
EXPECT_EQ(NetEq::kOK,
763-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
763+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
764764

765765
// Insert second packet (decoder will return CNG).
766766
payload[0] = 1;
767767
rtp_header.header.sequenceNumber++;
768768
rtp_header.header.timestamp += kPayloadLengthSamples;
769769
EXPECT_EQ(NetEq::kOK,
770-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
770+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
771771

772772
const size_t kMaxOutputSize = static_cast<size_t>(10 * kSampleRateKhz);
773773
AudioFrame output;
@@ -818,7 +818,7 @@ TEST_F(NetEqImplTest, CodecInternalCng) {
818818
rtp_header.header.sequenceNumber += 2;
819819
rtp_header.header.timestamp += 2 * kPayloadLengthSamples;
820820
EXPECT_EQ(NetEq::kOK,
821-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
821+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
822822

823823
for (size_t i = 6; i < 8; ++i) {
824824
ASSERT_EQ(kMaxOutputSize, output.samples_per_channel_);
@@ -896,7 +896,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) {
896896
// Insert one packet.
897897
payload[0] = kFirstPayloadValue; // This will make Decode() fail.
898898
EXPECT_EQ(NetEq::kOK,
899-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
899+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
900900

901901
// Insert another packet.
902902
payload[0] = kSecondPayloadValue; // This will make Decode() successful.
@@ -905,7 +905,7 @@ TEST_F(NetEqImplTest, UnsupportedDecoder) {
905905
// the second packet get decoded.
906906
rtp_header.header.timestamp += 3 * kPayloadLengthSamples;
907907
EXPECT_EQ(NetEq::kOK,
908-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
908+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
909909

910910
AudioFrame output;
911911
bool muted;
@@ -957,7 +957,7 @@ TEST_F(NetEqImplTest, FloodBufferAndGetNetworkStats) {
957957
for (size_t i = 0; i <= config_.max_packets_in_buffer; ++i) {
958958
EXPECT_EQ(i, packet_buffer_->NumPacketsInBuffer());
959959
EXPECT_EQ(NetEq::kOK,
960-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
960+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
961961
rtp_header.header.timestamp +=
962962
rtc::checked_cast<uint32_t>(kPayloadLengthSamples);
963963
++rtp_header.header.sequenceNumber;
@@ -1013,7 +1013,7 @@ TEST_F(NetEqImplTest, DecodedPayloadTooShort) {
10131013

10141014
// Insert one packet.
10151015
EXPECT_EQ(NetEq::kOK,
1016-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
1016+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
10171017

10181018
EXPECT_EQ(5u, neteq_->sync_buffer_for_test()->FutureLength());
10191019

@@ -1109,7 +1109,7 @@ TEST_F(NetEqImplTest, DecodingError) {
11091109
rtp_header.header.sequenceNumber += 1;
11101110
rtp_header.header.timestamp += kFrameLengthSamples;
11111111
EXPECT_EQ(NetEq::kOK,
1112-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
1112+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
11131113
}
11141114

11151115
// Pull audio.
@@ -1221,7 +1221,7 @@ TEST_F(NetEqImplTest, DecodingErrorDuringInternalCng) {
12211221
rtp_header.header.sequenceNumber += 1;
12221222
rtp_header.header.timestamp += kFrameLengthSamples;
12231223
EXPECT_EQ(NetEq::kOK,
1224-
neteq_->InsertPacket(rtp_header, payload, kReceiveTime));
1224+
neteq_->InsertPacket(rtp_header.header, payload, kReceiveTime));
12251225
}
12261226

12271227
// Pull audio.
@@ -1341,7 +1341,7 @@ class NetEqImplTest120ms : public NetEqImplTest {
13411341
rtp_header.header.ssrc = 15;
13421342
const size_t kPayloadLengthBytes = 1; // This can be arbitrary.
13431343
uint8_t payload[kPayloadLengthBytes] = {0};
1344-
EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload, 10));
1344+
EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header.header, payload, 10));
13451345
sequence_number_++;
13461346
}
13471347

‎webrtc/modules/audio_coding/neteq/neteq_stereo_unittest.cc

+7-6
Original file line numberDiff line numberDiff line change
@@ -197,16 +197,17 @@ class NetEqStereoTest : public ::testing::TestWithParam<TestParameters> {
197197
while (time_now >= next_arrival_time) {
198198
// Insert packet in mono instance.
199199
ASSERT_EQ(NetEq::kOK,
200-
neteq_mono_->InsertPacket(rtp_header_mono_,
200+
neteq_mono_->InsertPacket(rtp_header_mono_.header,
201201
rtc::ArrayView<const uint8_t>(
202202
encoded_, payload_size_bytes_),
203203
next_arrival_time));
204204
// Insert packet in multi-channel instance.
205-
ASSERT_EQ(NetEq::kOK, neteq_->InsertPacket(
206-
rtp_header_, rtc::ArrayView<const uint8_t>(
207-
encoded_multi_channel_,
208-
multi_payload_size_bytes_),
209-
next_arrival_time));
205+
ASSERT_EQ(NetEq::kOK,
206+
neteq_->InsertPacket(
207+
rtp_header_.header,
208+
rtc::ArrayView<const uint8_t>(encoded_multi_channel_,
209+
multi_payload_size_bytes_),
210+
next_arrival_time));
210211
// Get next input packets (mono and multi-channel).
211212
do {
212213
next_send_time = GetNewPackets();

0 commit comments

Comments
 (0)