Delete RtpData::OnRecoveredPacket, use RecoveredPacketReceiver instead.

BUG=webrtc:7135

Review-Url: https://codereview.webrtc.org/2886813002
Cr-Commit-Position: refs/heads/master@{#18305}
This commit is contained in:
nisse 2017-05-29 08:16:37 -07:00 committed by Commit Bot
parent 69bf1b3f3a
commit 30e8931ea7
9 changed files with 74 additions and 84 deletions

View File

@ -23,16 +23,6 @@
namespace webrtc {
// Callback interface for packets recovered by FlexFEC. The implementation
// should be able to demultiplex the recovered RTP packets based on SSRC.
class RecoveredPacketReceiver {
public:
virtual void OnRecoveredPacket(const uint8_t* packet, size_t length) = 0;
protected:
virtual ~RecoveredPacketReceiver() = default;
};
class FlexfecReceiver {
public:
FlexfecReceiver(uint32_t ssrc,

View File

@ -195,9 +195,17 @@ class RtpData {
virtual int32_t OnReceivedPayloadData(const uint8_t* payload_data,
size_t payload_size,
const WebRtcRTPHeader* rtp_header) = 0;
};
virtual bool OnRecoveredPacket(const uint8_t* packet,
size_t packet_length) = 0;
// Callback interface for packets recovered by FlexFEC or ULPFEC. In
// the FlexFEC case, the implementation should be able to demultiplex
// the recovered RTP packets based on SSRC.
class RecoveredPacketReceiver {
public:
virtual void OnRecoveredPacket(const uint8_t* packet, size_t length) = 0;
protected:
virtual ~RecoveredPacketReceiver() = default;
};
class RtpFeedback {
@ -400,10 +408,6 @@ class NullRtpData : public RtpData {
const WebRtcRTPHeader* rtp_header) override {
return 0;
}
bool OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override {
return true;
}
};
// Statistics about packet loss for a single directional connection. All values

View File

@ -31,7 +31,7 @@ struct FecPacketCounter {
class UlpfecReceiver {
public:
static UlpfecReceiver* Create(RtpData* callback);
static UlpfecReceiver* Create(RecoveredPacketReceiver* callback);
virtual ~UlpfecReceiver() {}

View File

@ -21,11 +21,11 @@
namespace webrtc {
UlpfecReceiver* UlpfecReceiver::Create(RtpData* callback) {
UlpfecReceiver* UlpfecReceiver::Create(RecoveredPacketReceiver* callback) {
return new UlpfecReceiverImpl(callback);
}
UlpfecReceiverImpl::UlpfecReceiverImpl(RtpData* callback)
UlpfecReceiverImpl::UlpfecReceiverImpl(RecoveredPacketReceiver* callback)
: recovered_packet_callback_(callback),
fec_(ForwardErrorCorrection::CreateUlpfec()) {}
@ -212,10 +212,8 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
if (!received_packets_.front()->is_fec) {
ForwardErrorCorrection::Packet* packet = received_packets_.front()->pkt;
crit_sect_.Leave();
if (!recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length)) {
return -1;
}
recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length);
crit_sect_.Enter();
}
if (fec_->DecodeFec(&received_packets_, &recovered_packets_) != 0) {
@ -233,10 +231,8 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() {
ForwardErrorCorrection::Packet* packet = recovered_packet->pkt;
++packet_counter_.num_recovered_packets;
crit_sect_.Leave();
if (!recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length)) {
return -1;
}
recovered_packet_callback_->OnRecoveredPacket(packet->data,
packet->length);
crit_sect_.Enter();
recovered_packet->returned = true;
}

View File

@ -23,7 +23,7 @@ namespace webrtc {
class UlpfecReceiverImpl : public UlpfecReceiver {
public:
explicit UlpfecReceiverImpl(RtpData* callback);
explicit UlpfecReceiverImpl(RecoveredPacketReceiver* callback);
virtual ~UlpfecReceiverImpl();
int32_t AddReceivedRedPacket(const RTPHeader& rtp_header,
@ -37,7 +37,7 @@ class UlpfecReceiverImpl : public UlpfecReceiver {
private:
rtc::CriticalSection crit_sect_;
RtpData* recovered_packet_callback_;
RecoveredPacketReceiver* recovered_packet_callback_;
std::unique_ptr<ForwardErrorCorrection> fec_;
// TODO(holmer): In the current version |received_packets_| is never more
// than one packet, since we process FEC every time a new packet

View File

@ -16,6 +16,7 @@
#include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h"
#include "webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h"
#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"
#include "webrtc/modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
#include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h"
#include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h"
@ -36,13 +37,19 @@ using test::fec::UlpfecPacketGenerator;
constexpr int kFecPayloadType = 96;
constexpr uint32_t kMediaSsrc = 835424;
class NullRecoveredPacketReceiver : public RecoveredPacketReceiver {
public:
void OnRecoveredPacket(const uint8_t* packet, size_t length) override {}
};
} // namespace
class UlpfecReceiverTest : public ::testing::Test {
protected:
UlpfecReceiverTest()
: fec_(ForwardErrorCorrection::CreateUlpfec()),
receiver_fec_(UlpfecReceiver::Create(&rtp_data_callback_)),
receiver_fec_(UlpfecReceiver::Create(&recovered_packet_receiver_)),
packet_generator_(kMediaSsrc) {}
// Generates |num_fec_packets| FEC packets, given |media_packets|.
@ -64,7 +71,7 @@ class UlpfecReceiverTest : public ::testing::Test {
// to the receiver.
void BuildAndAddRedFecPacket(Packet* packet);
// Ensure that |rtp_data_callback_| will be called correctly
// Ensure that |recovered_packet_receiver_| will be called correctly
// and that the recovered packet will be identical to the lost packet.
void VerifyReconstructedMediaPacket(const AugmentedPacket& packet,
size_t times);
@ -75,7 +82,7 @@ class UlpfecReceiverTest : public ::testing::Test {
size_t length,
uint8_t ulpfec_payload_type);
MockRtpData rtp_data_callback_;
MockRecoveredPacketReceiver recovered_packet_receiver_;
std::unique_ptr<ForwardErrorCorrection> fec_;
std::unique_ptr<UlpfecReceiver> receiver_fec_;
UlpfecPacketGenerator packet_generator_;
@ -134,15 +141,13 @@ void UlpfecReceiverTest::VerifyReconstructedMediaPacket(
// Verify that the content of the reconstructed packet is equal to the
// content of |packet|, and that the same content is received |times| number
// of times in a row.
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, packet.length))
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, packet.length))
.With(Args<0, 1>(ElementsAreArray(packet.data, packet.length)))
.Times(times)
.WillRepeatedly(Return(true));
.Times(times);
}
void UlpfecReceiverTest::InjectGarbagePacketLength(size_t fec_garbage_offset) {
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _));
const size_t kNumFecPackets = 1;
std::list<AugmentedPacket*> augmented_media_packets;
@ -172,7 +177,7 @@ void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data,
std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
ASSERT_TRUE(parser->Parse(data, length, &header));
NullRtpData null_callback;
NullRecoveredPacketReceiver null_callback;
std::unique_ptr<UlpfecReceiver> receiver_fec(
UlpfecReceiver::Create(&null_callback));
@ -345,9 +350,8 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) {
EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front());
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
delayed_fec = fec_packets.front();
@ -362,17 +366,15 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) {
for (auto it = augmented_media_packets_batch2.begin();
it != augmented_media_packets_batch2.end(); ++it) {
BuildAndAddRedMediaPacket(*it);
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}
// Add the delayed FEC packet. One packet should be reconstructed.
BuildAndAddRedFecPacket(delayed_fec);
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}
@ -390,9 +392,8 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) {
EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front());
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
delayed_fec = fec_packets.front();
@ -407,16 +408,15 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) {
for (auto it = augmented_media_packets_batch2.begin();
it != augmented_media_packets_batch2.end(); ++it) {
BuildAndAddRedMediaPacket(*it);
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}
// Add the delayed FEC packet. No packet should be reconstructed since the
// first media packet of that frame has been dropped due to being too old.
BuildAndAddRedFecPacket(delayed_fec);
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)).Times(0);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}
@ -435,7 +435,7 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) {
for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) {
// Only FEC packets inserted. No packets recoverable at this time.
BuildAndAddRedFecPacket(*it);
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)).Times(0);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}
// Move unique_ptr's to media_packets for lifetime management.
@ -450,9 +450,8 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) {
// and should have been dropped. Only the media packet we inserted will be
// returned.
BuildAndAddRedMediaPacket(augmented_media_packets.front());
EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _))
.Times(1)
.WillRepeatedly(Return(true));
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _))
.Times(1);
EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
}

View File

@ -277,17 +277,17 @@ int32_t RtpStreamReceiver::OnReceivedPayloadData(
}
// TODO(nisse): Try to delete this method. Obstacles: It is used by
// ParseAndHandleEncapsulatingHeader, for handling Rtx packets. And
// it's part of the RtpData interface which we implement.
bool RtpStreamReceiver::OnRecoveredPacket(const uint8_t* rtp_packet,
// ParseAndHandleEncapsulatingHeader, for handling Rtx packets, and
// for callbacks from |ulpfec_receiver_|.
void RtpStreamReceiver::OnRecoveredPacket(const uint8_t* rtp_packet,
size_t rtp_packet_length) {
RTPHeader header;
if (!rtp_header_parser_->Parse(rtp_packet, rtp_packet_length, &header)) {
return false;
return;
}
header.payload_type_frequency = kVideoPayloadTypeFrequency;
bool in_order = IsPacketInOrder(header);
return ReceivePacket(rtp_packet, rtp_packet_length, header, in_order);
ReceivePacket(rtp_packet, rtp_packet_length, header, in_order);
}
// TODO(pbos): Remove as soon as audio can handle a changing payload type
@ -428,13 +428,13 @@ rtc::Optional<int64_t> RtpStreamReceiver::LastReceivedKeyframePacketMs() const {
return packet_buffer_->LastReceivedKeyframePacketMs();
}
// TODO(nisse): Drop return value.
bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
void RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header,
bool in_order) {
if (rtp_payload_registry_.IsEncapsulated(header)) {
return ParseAndHandleEncapsulatingHeader(packet, packet_length, header);
ParseAndHandleEncapsulatingHeader(packet, packet_length, header);
return;
}
const uint8_t* payload = packet + header.headerLength;
assert(packet_length >= header.headerLength);
@ -442,13 +442,13 @@ bool RtpStreamReceiver::ReceivePacket(const uint8_t* packet,
PayloadUnion payload_specific;
if (!rtp_payload_registry_.GetPayloadSpecifics(header.payloadType,
&payload_specific)) {
return false;
return;
}
return rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
payload_specific, in_order);
rtp_receiver_->IncomingRtpPacket(header, payload, payload_length,
payload_specific, in_order);
}
bool RtpStreamReceiver::ParseAndHandleEncapsulatingHeader(
void RtpStreamReceiver::ParseAndHandleEncapsulatingHeader(
const uint8_t* packet, size_t packet_length, const RTPHeader& header) {
if (rtp_payload_registry_.IsRed(header)) {
int8_t ulpfec_pt = rtp_payload_registry_.ulpfec_payload_type();
@ -460,24 +460,24 @@ bool RtpStreamReceiver::ParseAndHandleEncapsulatingHeader(
}
if (ulpfec_receiver_->AddReceivedRedPacket(header, packet, packet_length,
ulpfec_pt) != 0) {
return false;
return;
}
return ulpfec_receiver_->ProcessReceivedFec() == 0;
ulpfec_receiver_->ProcessReceivedFec();
} else if (rtp_payload_registry_.IsRtx(header)) {
if (header.headerLength + header.paddingLength == packet_length) {
// This is an empty packet and should be silently dropped before trying to
// parse the RTX header.
return true;
return;
}
// Remove the RTX header and parse the original RTP header.
if (packet_length < header.headerLength)
return false;
return;
if (packet_length > sizeof(restored_packet_))
return false;
return;
rtc::CritScope lock(&receive_cs_);
if (restored_packet_in_use_) {
LOG(LS_WARNING) << "Multiple RTX headers detected, dropping packet.";
return false;
return;
}
if (!rtp_payload_registry_.RestoreOriginalPacket(
restored_packet_, packet, &packet_length, rtp_receiver_->SSRC(),
@ -485,14 +485,12 @@ bool RtpStreamReceiver::ParseAndHandleEncapsulatingHeader(
LOG(LS_WARNING) << "Incoming RTX packet: Invalid RTP header ssrc: "
<< header.ssrc << " payload type: "
<< static_cast<int>(header.payloadType);
return false;
return;
}
restored_packet_in_use_ = true;
bool ret = OnRecoveredPacket(restored_packet_, packet_length);
OnRecoveredPacket(restored_packet_, packet_length);
restored_packet_in_use_ = false;
return ret;
}
return false;
}
void RtpStreamReceiver::NotifyReceiverOfFecPacket(const RTPHeader& header) {

View File

@ -56,6 +56,7 @@ class VideoReceiver;
} // namespace vcm
class RtpStreamReceiver : public RtpData,
public RecoveredPacketReceiver,
public RtpFeedback,
public VCMFrameTypeCallback,
public VCMPacketRequestCallback,
@ -102,7 +103,8 @@ class RtpStreamReceiver : public RtpData,
int32_t OnReceivedPayloadData(const uint8_t* payload_data,
size_t payload_size,
const WebRtcRTPHeader* rtp_header) override;
bool OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override;
// Implements RecoveredPacketReceiver.
void OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override;
// Implements RtpFeedback.
int32_t OnInitializeDecoder(int8_t payload_type,
@ -140,13 +142,13 @@ class RtpStreamReceiver : public RtpData,
private:
bool AddReceiveCodec(const VideoCodec& video_codec);
bool ReceivePacket(const uint8_t* packet,
void ReceivePacket(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header,
bool in_order);
// Parses and handles for instance RTX and RED headers.
// This function assumes that it's being called from only one thread.
bool ParseAndHandleEncapsulatingHeader(const uint8_t* packet,
void ParseAndHandleEncapsulatingHeader(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header);
void NotifyReceiverOfFecPacket(const RTPHeader& header);

View File

@ -316,7 +316,6 @@ class Channel
int32_t OnReceivedPayloadData(const uint8_t* payloadData,
size_t payloadSize,
const WebRtcRTPHeader* rtpHeader) override;
bool OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override;
// From RtpFeedback in the RTP/RTCP module
int32_t OnInitializeDecoder(int8_t payloadType,
@ -417,6 +416,8 @@ class Channel
bool OnRtpPacketWithHeader(const uint8_t* received_packet,
size_t length,
RTPHeader *header);
bool OnRecoveredPacket(const uint8_t* packet, size_t packet_length);
bool ReceivePacket(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header,