admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Use correct RTP header length in RED generation for ULPFEC packets.

Browse Source 
Prior to this change, in certain circumstances the RTP header length
used when creating a RedPacket was incorrect. This was due to an
assumption that a new media packet would _always_ be added to the
UlpfecGenerator's internal media packet buffer. This is not correct,
and the fix is to keep track of whatever RTP header length that is
currently correct.

Bug: webrtc:8767
Change-Id: I6d61429a19d4693dde9330f0469d13c5dfbeac52
Reviewed-on: https://webrtc-review.googlesource.com/40600
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21720}
This commit is contained in:
Rasmus Brandt 2018-01-22 12:52:36 +01:00 committed by Commit Bot
parent 75db552b33
commit 393e266470
5 changed files with 61 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
modules/rtp_rtcp/source/rtp_sender_video.cc
View File

@ -145,8 +145,7 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec(
uint16_t first_fec_sequence_number =
rtp_sender_->AllocateSequenceNumber(num_fec_packets);
fec_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(
red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number,
media_packet->headers_size());
red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number);
RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
if (retransmission_settings_ & kRetransmitFECPackets)
fec_storage = kAllowRetransmission;

19
modules/rtp_rtcp/source/ulpfec_generator.cc
View File

@ -13,6 +13,7 @@
#include <memory>
#include <utility>
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/byte_io.h"
#include "modules/rtp_rtcp/source/forward_error_correction.h"
#include "modules/rtp_rtcp/source/rtp_utility.h"
@ -105,6 +106,7 @@ UlpfecGenerator::UlpfecGenerator()
UlpfecGenerator::UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec)
: fec_(std::move(fec)),
last_media_packet_rtp_header_length_(0),
num_protected_frames_(0),
min_num_media_packets_(1) {
memset(&params_, 0, sizeof(params_));
@ -142,6 +144,10 @@ int UlpfecGenerator::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer,
packet->length = payload_length + rtp_header_length;
memcpy(packet->data, data_buffer, packet->length);
media_packets_.push_back(std::move(packet));
// Keep track of the RTP header length, so we can copy the RTP header
// from |packet| to newly generated ULPFEC+RED packets.
RTC_DCHECK_GE(rtp_header_length, kRtpHeaderSize);
last_media_packet_rtp_header_length_ = rtp_header_length;
}
if (marker_bit) {
++num_protected_frames_;
@ -200,8 +206,7 @@ size_t UlpfecGenerator::MaxPacketOverhead() const {
std::vector<std::unique_ptr<RedPacket>> UlpfecGenerator::GetUlpfecPacketsAsRed(
int red_payload_type,
int ulpfec_payload_type,
uint16_t first_seq_num,
size_t rtp_header_length) {
uint16_t first_seq_num) {
std::vector<std::unique_ptr<RedPacket>> red_packets;
red_packets.reserve(generated_fec_packets_.size());
RTC_DCHECK(!media_packets_.empty());
@ -212,9 +217,12 @@ std::vector<std::unique_ptr<RedPacket>> UlpfecGenerator::GetUlpfecPacketsAsRed(
// Wrap FEC packet (including FEC headers) in a RED packet. Since the
// FEC packets in |generated_fec_packets_| don't have RTP headers, we
// reuse the header from the last media packet.
std::unique_ptr<RedPacket> red_packet(new RedPacket(
fec_packet->length + kRedForFecHeaderLength + rtp_header_length));
red_packet->CreateHeader(last_media_packet->data, rtp_header_length,
RTC_DCHECK_GT(last_media_packet_rtp_header_length_, 0);
std::unique_ptr<RedPacket> red_packet(
new RedPacket(last_media_packet_rtp_header_length_ +
kRedForFecHeaderLength + fec_packet->length));
red_packet->CreateHeader(last_media_packet->data,
last_media_packet_rtp_header_length_,
red_payload_type, ulpfec_payload_type);
red_packet->SetSeqNum(seq_num++);
red_packet->ClearMarkerBit();
@ -237,6 +245,7 @@ int UlpfecGenerator::Overhead() const {
void UlpfecGenerator::ResetState() {
media_packets_.clear();
last_media_packet_rtp_header_length_ = 0;
generated_fec_packets_.clear();
num_protected_frames_ = 0;
}

4
modules/rtp_rtcp/source/ulpfec_generator.h
View File

@ -69,8 +69,7 @@ class UlpfecGenerator {
std::vector<std::unique_ptr<RedPacket>> GetUlpfecPacketsAsRed(
int red_payload_type,
int ulpfec_payload_type,
uint16_t first_seq_num,
size_t rtp_header_length);
uint16_t first_seq_num);
private:
explicit UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec);
@ -98,6 +97,7 @@ class UlpfecGenerator {
std::unique_ptr<ForwardErrorCorrection> fec_;
ForwardErrorCorrection::PacketList media_packets_;
size_t last_media_packet_rtp_header_length_;
std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
int num_protected_frames_;
int min_num_media_packets_;

49
modules/rtp_rtcp/source/ulpfec_generator_unittest.cc
View File

@ -95,8 +95,8 @@ TEST_F(UlpfecGeneratorTest, NoEmptyFecWithSeqNumGaps) {
size_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets();
if (num_fec_packets > 0) {
std::vector<std::unique_ptr<RedPacket>> fec_packets =
ulpfec_generator_.GetUlpfecPacketsAsRed(
kRedPayloadType, kFecPayloadType, 100, p.header_size);
ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType,
kFecPayloadType, 100);
EXPECT_EQ(num_fec_packets, fec_packets.size());
}
}
@ -122,10 +122,10 @@ TEST_F(UlpfecGeneratorTest, OneFrameFec) {
last_timestamp = packet->header.header.timestamp;
}
EXPECT_TRUE(ulpfec_generator_.FecAvailable());
uint16_t seq_num = packet_generator_.NextPacketSeqNum();
const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
std::vector<std::unique_ptr<RedPacket>> red_packets =
ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
seq_num, kRtpHeaderSize);
seq_num);
EXPECT_FALSE(ulpfec_generator_.FecAvailable());
ASSERT_EQ(1u, red_packets.size());
VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
@ -157,14 +157,51 @@ TEST_F(UlpfecGeneratorTest, TwoFrameFec) {
}
}
EXPECT_TRUE(ulpfec_generator_.FecAvailable());
uint16_t seq_num = packet_generator_.NextPacketSeqNum();
const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
std::vector<std::unique_ptr<RedPacket>> red_packets =
ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
seq_num, kRtpHeaderSize);
seq_num);
EXPECT_FALSE(ulpfec_generator_.FecAvailable());
ASSERT_EQ(1u, red_packets.size());
VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
red_packets.front().get(), false);
}
TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) {
constexpr size_t kShortRtpHeaderLength = 12;
constexpr size_t kLongRtpHeaderLength = 16;
// Only one frame required to generate FEC.
FecProtectionParams params = {127, 1, kFecMaskRandom};
ulpfec_generator_.SetFecParameters(params);
// Fill up internal buffer with media packets with short RTP header length.
packet_generator_.NewFrame(kUlpfecMaxMediaPackets + 1);
for (size_t i = 0; i < kUlpfecMaxMediaPackets; ++i) {
std::unique_ptr<AugmentedPacket> packet =
packet_generator_.NextPacket(i, 10);
EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(
packet->data, packet->length, kShortRtpHeaderLength));
EXPECT_FALSE(ulpfec_generator_.FecAvailable());
}
// Kick off FEC generation with media packet with long RTP header length.
// Since the internal buffer is full, this packet will not be protected.
std::unique_ptr<AugmentedPacket> packet =
packet_generator_.NextPacket(kUlpfecMaxMediaPackets, 10);
EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(
packet->data, packet->length, kLongRtpHeaderLength));
EXPECT_TRUE(ulpfec_generator_.FecAvailable());
// Ensure that the RED header is placed correctly, i.e. the correct
// RTP header length was used in the RED packet creation.
const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
std::vector<std::unique_ptr<RedPacket>> red_packets =
ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
seq_num);
for (const auto& red_packet : red_packets) {
EXPECT_EQ(kFecPayloadType, red_packet->data()[kShortRtpHeaderLength]);
}
}
} // namespace webrtc

3
test/fuzzers/ulpfec_generator_fuzzer.cc
View File

@ -51,8 +51,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
const size_t num_fec_packets = generator.NumAvailableFecPackets();
if (num_fec_packets > 0) {
std::vector<std::unique_ptr<RedPacket>> fec_packets =
generator.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100,
rtp_header_length);
generator.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100);
RTC_CHECK_EQ(num_fec_packets, fec_packets.size());
}
}