Revert "Revert of Parse FlexFEC RTP headers in Call and add integration with BWE. (patchset #17 id:460001 of https://codereview.webrtc.org/2553863003/ )"

Problem fixed: RTP header extensions were not properly set in tests.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2593963003
Cr-Commit-Position: refs/heads/master@{#15741}
This commit is contained in:
brandtr
2016-12-21 06:37:18 -08:00
committed by Commit bot
parent 9ba94baa9c
commit b29e652b10
18 changed files with 306 additions and 166 deletions

View File

@ -10,11 +10,8 @@
#include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h"
#include <utility>
#include "webrtc/base/logging.h"
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
namespace webrtc {
@ -31,13 +28,14 @@ constexpr int kPacketLogIntervalMs = 10000;
} // namespace
FlexfecReceiver::FlexfecReceiver(uint32_t ssrc,
uint32_t protected_media_ssrc,
RecoveredPacketReceiver* callback)
FlexfecReceiver::FlexfecReceiver(
uint32_t ssrc,
uint32_t protected_media_ssrc,
RecoveredPacketReceiver* recovered_packet_receiver)
: ssrc_(ssrc),
protected_media_ssrc_(protected_media_ssrc),
erasure_code_(ForwardErrorCorrection::CreateFlexfec()),
callback_(callback),
recovered_packet_receiver_(recovered_packet_receiver),
clock_(Clock::GetRealTimeClock()),
last_recovered_packet_ms_(-1) {
// It's OK to create this object on a different thread/task queue than
@ -47,10 +45,9 @@ FlexfecReceiver::FlexfecReceiver(uint32_t ssrc,
FlexfecReceiver::~FlexfecReceiver() = default;
bool FlexfecReceiver::AddAndProcessReceivedPacket(const uint8_t* packet,
size_t packet_length) {
bool FlexfecReceiver::AddAndProcessReceivedPacket(RtpPacketReceived packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
if (!AddReceivedPacket(packet, packet_length)) {
if (!AddReceivedPacket(std::move(packet))) {
return false;
}
return ProcessReceivedPackets();
@ -61,42 +58,32 @@ FecPacketCounter FlexfecReceiver::GetPacketCounter() const {
return packet_counter_;
}
bool FlexfecReceiver::AddReceivedPacket(const uint8_t* packet,
size_t packet_length) {
bool FlexfecReceiver::AddReceivedPacket(RtpPacketReceived packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
// RTP packets with a full base header (12 bytes), but without payload,
// could conceivably be useful in the decoding. Therefore we check
// with a strict inequality here.
if (packet_length < kRtpHeaderSize) {
LOG(LS_WARNING) << "Truncated packet, discarding.";
return false;
}
// TODO(brandtr): Consider how to handle received FlexFEC packets and
// the bandwidth estimator.
RtpPacketReceived parsed_packet;
if (!parsed_packet.Parse(packet, packet_length)) {
return false;
}
// with a non-strict inequality here.
RTC_DCHECK_GE(packet.size(), kRtpHeaderSize);
// Demultiplex based on SSRC, and insert into erasure code decoder.
std::unique_ptr<ReceivedPacket> received_packet(new ReceivedPacket());
received_packet->seq_num = parsed_packet.SequenceNumber();
received_packet->ssrc = parsed_packet.Ssrc();
received_packet->seq_num = packet.SequenceNumber();
received_packet->ssrc = packet.Ssrc();
if (received_packet->ssrc == ssrc_) {
// This is a FEC packet belonging to this FlexFEC stream.
if (parsed_packet.payload_size() < kMinFlexfecHeaderSize) {
// This is a FlexFEC packet.
if (packet.payload_size() < kMinFlexfecHeaderSize) {
LOG(LS_WARNING) << "Truncated FlexFEC packet, discarding.";
return false;
}
received_packet->is_fec = true;
++packet_counter_.num_fec_packets;
// Insert packet payload into erasure code.
// TODO(brandtr): Remove this memcpy when the FEC packet classes
// are using COW buffers internally.
received_packet->pkt = rtc::scoped_refptr<Packet>(new Packet());
auto payload = parsed_packet.payload();
auto payload = packet.payload();
memcpy(received_packet->pkt->data, payload.data(), payload.size());
received_packet->pkt->length = payload.size();
} else {
@ -106,13 +93,14 @@ bool FlexfecReceiver::AddReceivedPacket(const uint8_t* packet,
return false;
}
received_packet->is_fec = false;
// Insert entire packet into erasure code.
// TODO(brandtr): Remove this memcpy too.
received_packet->pkt = rtc::scoped_refptr<Packet>(new Packet());
memcpy(received_packet->pkt->data, parsed_packet.data(),
parsed_packet.size());
received_packet->pkt->length = parsed_packet.size();
memcpy(received_packet->pkt->data, packet.data(), packet.size());
received_packet->pkt->length = packet.size();
}
received_packets_.push_back(std::move(received_packet));
++packet_counter_.num_packets;
@ -144,8 +132,8 @@ bool FlexfecReceiver::ProcessReceivedPackets() {
continue;
}
++packet_counter_.num_recovered_packets;
if (!callback_->OnRecoveredPacket(recovered_packet->pkt->data,
recovered_packet->pkt->length)) {
if (!recovered_packet_receiver_->OnRecoveredPacket(
recovered_packet->pkt->data, recovered_packet->pkt->length)) {
return false;
}
recovered_packet->returned = true;

View File

@ -16,6 +16,7 @@
#include "webrtc/modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h"
#include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h"
#include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h"
#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h"
#include "webrtc/test/gmock.h"
#include "webrtc/test/gtest.h"
@ -36,6 +37,12 @@ constexpr size_t kPayloadLength = 500;
constexpr uint32_t kFlexfecSsrc = 42984;
constexpr uint32_t kMediaSsrc = 8353;
RtpPacketReceived ParsePacket(const Packet& packet) {
RtpPacketReceived parsed_packet;
EXPECT_TRUE(parsed_packet.Parse(packet.data, packet.length));
return parsed_packet;
}
} // namespace
class FlexfecReceiverTest : public ::testing::Test {
@ -93,21 +100,8 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaPacket) {
std::unique_ptr<Packet> media_packet(
packet_generator_.NextPacket(0, kPayloadLength));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
}
TEST_F(FlexfecReceiverTest, FailsOnTruncatedMediaPacket) {
const size_t kNoPayload = 0;
packet_generator_.NewFrame(1);
std::unique_ptr<Packet> media_packet(
packet_generator_.NextPacket(0, kNoPayload));
// Simulate truncated media packet.
media_packet->length = kRtpHeaderSize - 1;
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
}
TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) {
@ -117,13 +111,12 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) {
PacketList media_packets;
PacketizeFrame(kNumMediaPackets, 0, &media_packets);
std::list<Packet*> fec_packets = EncodeFec(media_packets, kNumFecPackets);
auto media_packet = media_packets.front().get();
const auto& media_packet = media_packets.front();
auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front());
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(fec_packet->data,
fec_packet->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet)));
}
TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) {
@ -133,15 +126,14 @@ TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) {
PacketList media_packets;
PacketizeFrame(kNumMediaPackets, 0, &media_packets);
std::list<Packet*> fec_packets = EncodeFec(media_packets, kNumFecPackets);
auto media_packet = media_packets.front().get();
const auto& media_packet = media_packets.front();
// Simulate truncated FlexFEC payload.
fec_packets.front()->length = 1;
auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front());
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(fec_packet->data,
fec_packet->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet)));
}
TEST_F(FlexfecReceiverTest, FailsOnUnknownMediaSsrc) {
@ -149,15 +141,15 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownMediaSsrc) {
PacketList media_packets;
PacketizeFrame(kNumMediaPackets, 0, &media_packets);
auto media_packet = media_packets.front().get();
auto& media_packet = media_packets.front();
// Corrupt the SSRC.
media_packet->data[8] = 0;
media_packet->data[9] = 1;
media_packet->data[10] = 2;
media_packet->data[11] = 3;
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_FALSE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
}
TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) {
@ -167,7 +159,7 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) {
PacketList media_packets;
PacketizeFrame(kNumMediaPackets, 0, &media_packets);
std::list<Packet*> fec_packets = EncodeFec(media_packets, kNumFecPackets);
auto media_packet = media_packets.front().get();
const auto& media_packet = media_packets.front();
auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front());
// Corrupt the SSRC.
fec_packet->data[8] = 4;
@ -175,10 +167,9 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) {
fec_packet->data[10] = 6;
fec_packet->data[11] = 7;
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(fec_packet->data,
fec_packet->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet)));
}
TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) {
@ -191,8 +182,8 @@ TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) {
// Receive all media packets.
for (const auto& media_packet : media_packets) {
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data,
media_packet->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
}
// Receive FEC packet.
@ -200,7 +191,7 @@ TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) {
std::unique_ptr<Packet> packet_with_rtp_header =
packet_generator_.BuildFlexfecPacket(*fec_packet);
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
}
TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {
@ -213,8 +204,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@ -227,7 +217,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
}
TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
@ -251,7 +241,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
// Receive second FEC packet and recover second lost media packet.
fec_it++;
@ -263,7 +253,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
}
TEST_F(FlexfecReceiverTest, DoesNotRecoverFromMediaAndFecLoss) {
@ -276,8 +266,7 @@ TEST_F(FlexfecReceiverTest, DoesNotRecoverFromMediaAndFecLoss) {
// Receive first media packet.
auto media_it = media_packets.begin();
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
// Drop second media packet and FEC packet. Do not expect call back.
}
@ -292,8 +281,7 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) {
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@ -306,11 +294,11 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
// Receive FEC packet again.
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
// Do not call back again.
}
@ -332,8 +320,7 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) {
// Drop every second media packet.
auto media_it = media_packets.begin();
while (media_it != media_packets.end()) {
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
++media_it;
if (media_it == media_packets.end()) {
break;
@ -356,7 +343,7 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) {
ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
fec_packet_with_rtp_header->data, fec_packet_with_rtp_header->length));
ParsePacket(*fec_packet_with_rtp_header)));
++media_it;
}
}
@ -383,8 +370,7 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) {
// Receive all other media packets.
while (media_it != media_packets.end()) {
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
++media_it;
}
@ -399,7 +385,7 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
}
TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) {
@ -424,8 +410,7 @@ TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) {
// Receive all other media packets.
while (media_it != media_packets.end()) {
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
++media_it;
}
@ -434,7 +419,7 @@ TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) {
std::unique_ptr<Packet> packet_with_rtp_header =
packet_generator_.BuildFlexfecPacket(**fec_it);
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
// Do not expect a call back.
}
@ -455,14 +440,14 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) {
auto media_packet3 = media_it++;
auto media_packet4 = media_it++;
auto media_packet5 = media_it++;
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet5)->data,
(*media_packet5)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet2)->data,
(*media_packet2)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet3)->data,
(*media_packet3)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet0)->data,
(*media_packet0)->length));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet5)));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet2)));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet3)));
EXPECT_TRUE(
receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet0)));
// Expect to recover lost media packets.
EXPECT_CALL(recovered_packet_receiver_,
@ -482,7 +467,7 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) {
while (fec_it != fec_packets.end()) {
packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it);
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
++fec_it;
}
}
@ -497,8 +482,7 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) {
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data,
(*media_it)->length));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@ -511,7 +495,7 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) {
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
packet_with_rtp_header->data, packet_with_rtp_header->length));
ParsePacket(*packet_with_rtp_header)));
// Check stats calculations.
FecPacketCounter packet_counter = receiver_.GetPacketCounter();

View File

@ -86,6 +86,10 @@ bool Packet::Parse(const uint8_t* buffer, size_t buffer_size) {
return true;
}
bool Packet::Parse(rtc::ArrayView<const uint8_t> packet) {
return Parse(packet.data(), packet.size());
}
bool Packet::Parse(rtc::CopyOnWriteBuffer buffer) {
if (!ParseBuffer(buffer.cdata(), buffer.size())) {
Clear();

View File

@ -31,6 +31,7 @@ class Packet {
// Parse and copy given buffer into Packet.
bool Parse(const uint8_t* buffer, size_t size);
bool Parse(rtc::ArrayView<const uint8_t> packet);
// Parse and move given buffer into Packet.
bool Parse(rtc::CopyOnWriteBuffer packet);