Make recv_deltas optional in TransportFeedback packets

Bug: webrtc:10263
Change-Id: I49c4a4710a5c34a62b53080e708c310a8484831b
Reviewed-on: https://webrtc-review.googlesource.com/c/122543
Commit-Queue: Johannes Kron <kron@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26687}
This commit is contained in:
Johannes Kron 2019-02-14 11:08:54 +01:00 committed by Commit Bot
parent 69fb6c8510
commit 8e847ee96a
3 changed files with 131 additions and 62 deletions

View File

@ -262,10 +262,14 @@ void TransportFeedback::LastChunk::DecodeRunLength(uint16_t chunk,
}
TransportFeedback::TransportFeedback()
: TransportFeedback(/*include_timestamps=*/true) {}
TransportFeedback::TransportFeedback(bool include_timestamps)
: base_seq_no_(0),
num_seq_no_(0),
base_time_ticks_(0),
feedback_seq_(0),
include_timestamps_(include_timestamps),
last_timestamp_us_(0),
size_bytes_(kTransportFeedbackHeaderSizeBytes) {}
@ -276,6 +280,7 @@ TransportFeedback::TransportFeedback(TransportFeedback&& other)
num_seq_no_(other.num_seq_no_),
base_time_ticks_(other.base_time_ticks_),
feedback_seq_(other.feedback_seq_),
include_timestamps_(other.include_timestamps_),
last_timestamp_us_(other.last_timestamp_us_),
packets_(std::move(other.packets_)),
encoded_chunks_(std::move(other.encoded_chunks_)),
@ -301,19 +306,25 @@ void TransportFeedback::SetFeedbackSequenceNumber(uint8_t feedback_sequence) {
bool TransportFeedback::AddReceivedPacket(uint16_t sequence_number,
int64_t timestamp_us) {
// Convert to ticks and round.
int64_t delta_full = (timestamp_us - last_timestamp_us_) % kTimeWrapPeriodUs;
if (delta_full > kTimeWrapPeriodUs / 2)
delta_full -= kTimeWrapPeriodUs;
delta_full +=
delta_full < 0 ? -(kDeltaScaleFactor / 2) : kDeltaScaleFactor / 2;
delta_full /= kDeltaScaleFactor;
// Set delta to zero if timestamps are not included, this will simplify the
// encoding process.
int16_t delta = 0;
if (include_timestamps_) {
// Convert to ticks and round.
int64_t delta_full =
(timestamp_us - last_timestamp_us_) % kTimeWrapPeriodUs;
if (delta_full > kTimeWrapPeriodUs / 2)
delta_full -= kTimeWrapPeriodUs;
delta_full +=
delta_full < 0 ? -(kDeltaScaleFactor / 2) : kDeltaScaleFactor / 2;
delta_full /= kDeltaScaleFactor;
int16_t delta = static_cast<int16_t>(delta_full);
// If larger than 16bit signed, we can't represent it - need new fb packet.
if (delta != delta_full) {
RTC_LOG(LS_WARNING) << "Delta value too large ( >= 2^16 ticks )";
return false;
delta = static_cast<int16_t>(delta_full);
// If larger than 16bit signed, we can't represent it - need new fb packet.
if (delta != delta_full) {
RTC_LOG(LS_WARNING) << "Delta value too large ( >= 2^16 ticks )";
return false;
}
}
uint16_t next_seq_no = base_seq_no_ + num_seq_no_;
@ -332,7 +343,9 @@ bool TransportFeedback::AddReceivedPacket(uint16_t sequence_number,
packets_.emplace_back(sequence_number, delta);
last_timestamp_us_ += delta * kDeltaScaleFactor;
size_bytes_ += delta_size;
if (include_timestamps_) {
size_bytes_ += delta_size;
}
return true;
}
@ -400,38 +413,57 @@ bool TransportFeedback::Parse(const CommonHeader& packet) {
num_seq_no_ = status_count;
uint16_t seq_no = base_seq_no_;
size_t recv_delta_size = 0;
for (size_t delta_size : delta_sizes) {
if (index + delta_size > end_index) {
RTC_LOG(LS_WARNING) << "Buffer overflow while parsing packet.";
Clear();
return false;
}
switch (delta_size) {
case 0:
break;
case 1: {
int16_t delta = payload[index];
packets_.emplace_back(seq_no, delta);
last_timestamp_us_ += delta * kDeltaScaleFactor;
index += delta_size;
break;
}
case 2: {
int16_t delta = ByteReader<int16_t>::ReadBigEndian(&payload[index]);
packets_.emplace_back(seq_no, delta);
last_timestamp_us_ += delta * kDeltaScaleFactor;
index += delta_size;
break;
}
case 3:
recv_delta_size += delta_size;
}
// Determine if timestamps, that is, recv_delta are included in the packet.
if (end_index >= index + recv_delta_size) {
for (size_t delta_size : delta_sizes) {
if (index + delta_size > end_index) {
RTC_LOG(LS_WARNING) << "Buffer overflow while parsing packet.";
Clear();
RTC_LOG(LS_WARNING) << "Invalid delta_size for seq_no " << seq_no;
return false;
default:
RTC_NOTREACHED();
break;
}
switch (delta_size) {
case 0:
break;
case 1: {
int16_t delta = payload[index];
packets_.emplace_back(seq_no, delta);
last_timestamp_us_ += delta * kDeltaScaleFactor;
index += delta_size;
break;
}
case 2: {
int16_t delta = ByteReader<int16_t>::ReadBigEndian(&payload[index]);
packets_.emplace_back(seq_no, delta);
last_timestamp_us_ += delta * kDeltaScaleFactor;
index += delta_size;
break;
}
case 3:
Clear();
RTC_LOG(LS_WARNING) << "Invalid delta_size for seq_no " << seq_no;
return false;
default:
RTC_NOTREACHED();
break;
}
++seq_no;
}
} else {
// The packet does not contain receive deltas.
include_timestamps_ = false;
for (size_t delta_size : delta_sizes) {
// Use delta sizes to detect if packet was received.
if (delta_size > 0) {
packets_.emplace_back(seq_no, 0);
}
++seq_no;
}
++seq_no;
}
size_bytes_ = RtcpPacket::kHeaderLength + index;
RTC_DCHECK_LE(index, end_index);
@ -495,7 +527,9 @@ bool TransportFeedback::IsConsistent() const {
timestamp_us += packet_it->delta_us();
++packet_it;
}
packet_size += delta_size;
if (include_timestamps_) {
packet_size += delta_size;
}
++seq_no;
}
if (packet_it != packets_.end()) {
@ -566,13 +600,15 @@ bool TransportFeedback::Create(uint8_t* packet,
*position += 2;
}
for (const auto& received_packet : packets_) {
int16_t delta = received_packet.delta_ticks();
if (delta >= 0 && delta <= 0xFF) {
packet[(*position)++] = delta;
} else {
ByteWriter<int16_t>::WriteBigEndian(&packet[*position], delta);
*position += 2;
if (include_timestamps_) {
for (const auto& received_packet : packets_) {
int16_t delta = received_packet.delta_ticks();
if (delta >= 0 && delta <= 0xFF) {
packet[(*position)++] = delta;
} else {
ByteWriter<int16_t>::WriteBigEndian(&packet[*position], delta);
*position += 2;
}
}
}

View File

@ -45,6 +45,10 @@ class TransportFeedback : public Rtpfb {
static constexpr size_t kMaxReportedPackets = 0xffff;
TransportFeedback();
explicit TransportFeedback(
bool include_timestamps); // If |include_timestamps| is set to false, the
// created packet will not contain the receive
// delta block.
TransportFeedback(const TransportFeedback&);
TransportFeedback(TransportFeedback&&);
@ -65,6 +69,9 @@ class TransportFeedback : public Rtpfb {
// Get the reference time in microseconds, including any precision loss.
int64_t GetBaseTimeUs() const;
// Does the feedback packet contain timestamp information?
bool IncludeTimestamps() const { return include_timestamps_; }
bool Parse(const CommonHeader& packet);
static std::unique_ptr<TransportFeedback> ParseFrom(const uint8_t* buffer,
size_t length);
@ -141,6 +148,7 @@ class TransportFeedback : public Rtpfb {
uint16_t num_seq_no_;
int32_t base_time_ticks_;
uint8_t feedback_seq_;
bool include_timestamps_;
int64_t last_timestamp_us_;
std::vector<ReceivedPacket> packets_;

View File

@ -33,9 +33,11 @@ static const int64_t kDeltaLimit = 0xFF * TransportFeedback::kDeltaScaleFactor;
class FeedbackTester {
public:
FeedbackTester()
FeedbackTester() : FeedbackTester(true) {}
explicit FeedbackTester(bool include_timestamps)
: expected_size_(kAnySize),
default_delta_(TransportFeedback::kDeltaScaleFactor * 4) {}
default_delta_(TransportFeedback::kDeltaScaleFactor * 4),
include_timestamps_(include_timestamps) {}
void WithExpectedSize(size_t expected_size) {
expected_size_ = expected_size;
@ -46,16 +48,16 @@ class FeedbackTester {
void WithInput(const uint16_t received_seq[],
const int64_t received_ts[],
uint16_t length) {
std::unique_ptr<int64_t[]> temp_deltas;
std::unique_ptr<int64_t[]> temp_timestamps;
if (received_ts == nullptr) {
temp_deltas.reset(new int64_t[length]);
GenerateDeltas(received_seq, length, temp_deltas.get());
received_ts = temp_deltas.get();
temp_timestamps.reset(new int64_t[length]);
GenerateReceiveTimestamps(received_seq, length, temp_timestamps.get());
received_ts = temp_timestamps.get();
}
expected_seq_.clear();
expected_deltas_.clear();
feedback_.reset(new TransportFeedback());
feedback_.reset(new TransportFeedback(include_timestamps_));
feedback_->SetBase(received_seq[0], received_ts[0]);
ASSERT_TRUE(feedback_->IsConsistent());
@ -81,8 +83,9 @@ class FeedbackTester {
VerifyInternal();
feedback_ =
TransportFeedback::ParseFrom(serialized_.data(), serialized_.size());
ASSERT_TRUE(feedback_->IsConsistent());
ASSERT_NE(nullptr, feedback_);
ASSERT_TRUE(feedback_->IsConsistent());
EXPECT_EQ(include_timestamps_, feedback_->IncludeTimestamps());
VerifyInternal();
}
@ -104,12 +107,14 @@ class FeedbackTester {
actual_deltas_us.push_back(packet.delta_us());
}
EXPECT_THAT(actual_seq_nos, ElementsAreArray(expected_seq_));
EXPECT_THAT(actual_deltas_us, ElementsAreArray(expected_deltas_));
if (include_timestamps_) {
EXPECT_THAT(actual_deltas_us, ElementsAreArray(expected_deltas_));
}
}
void GenerateDeltas(const uint16_t seq[],
const size_t length,
int64_t* deltas) {
void GenerateReceiveTimestamps(const uint16_t seq[],
const size_t length,
int64_t* timestamps) {
uint16_t last_seq = seq[0];
int64_t offset = 0;
@ -118,7 +123,7 @@ class FeedbackTester {
offset += 0x10000 * default_delta_;
last_seq = seq[i];
deltas[i] = offset + (last_seq * default_delta_);
timestamps[i] = offset + (last_seq * default_delta_);
}
}
@ -128,8 +133,17 @@ class FeedbackTester {
int64_t default_delta_;
std::unique_ptr<TransportFeedback> feedback_;
rtc::Buffer serialized_;
bool include_timestamps_;
};
// The following tests use FeedbackTester that simulates received packets as
// specified by the parameters |received_seq[]| and |received_ts[]| (optional).
// The following is verified in these tests:
// - Expected size of serialized packet.
// - Expected sequence numbers and receive deltas.
// - Sequence numbers and receive deltas are persistent after serialization
// followed by parsing.
// - The internal state of a feedback packet is consistent.
TEST(RtcpPacketTest, TransportFeedbackOneBitVector) {
const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
@ -142,6 +156,17 @@ TEST(RtcpPacketTest, TransportFeedbackOneBitVector) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedbackOneBitVectorNoRecvDelta) {
const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
const size_t kExpectedSizeBytes = kHeaderSize + kStatusChunkSize;
FeedbackTester test(/*include_timestamps=*/false);
test.WithExpectedSize(kExpectedSizeBytes);
test.WithInput(kReceived, nullptr, kLength);
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedbackFullOneBitVector) {
const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13, 14};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);