Revert "Make RTCP cumulative_lost be a signed value"

This reverts commit 4c34f435db2b921b82b8be19ee5c1746f46cb188.

Reason for revert: Broke internal projects. Type mismatch.

Original change's description:
> Make RTCP cumulative_lost be a signed value
> 
> This is formally defined as a signed 24-bit value in RFC 3550 section 6.4.1.
> See RFC 3550 Appendix A.3 for the reason why it may turn negative.
> 
> Noticed on discuss-webrtc mailing list.
> 
> BUG=webrtc:8626
> 
> Change-Id: I7317f73e9490a876e8445bd3d6b66095ce53ca0a
> Reviewed-on: https://webrtc-review.googlesource.com/30901
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21142}

TBR=stefan@webrtc.org,hta@webrtc.org

Change-Id: I544f7979d584cfb72a2d0d526f4fef84aebeecb3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8626
Reviewed-on: https://webrtc-review.googlesource.com/31040
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Commit-Queue: Zhi Huang <zhihuang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21144}
This commit is contained in:
Zhi Huang
2017-12-07 18:12:35 +00:00
committed by Commit Bot
parent 292a73eeea
commit 062a8ead3b
5 changed files with 15 additions and 22 deletions

View File

@ -27,7 +27,7 @@ namespace {
const uint32_t kSenderSsrc = 0x12345678; const uint32_t kSenderSsrc = 0x12345678;
const uint32_t kRemoteSsrc = 0x23456789; const uint32_t kRemoteSsrc = 0x23456789;
const uint8_t kFractionLost = 55; const uint8_t kFractionLost = 55;
const int32_t kCumulativeLost = 0x111213; const uint32_t kCumulativeLost = 0x111213;
const uint32_t kExtHighestSeqNum = 0x22232425; const uint32_t kExtHighestSeqNum = 0x22232425;
const uint32_t kJitter = 0x33343536; const uint32_t kJitter = 0x33343536;
const uint32_t kLastSr = 0x44454647; const uint32_t kLastSr = 0x44454647;

View File

@ -76,9 +76,8 @@ void ReportBlock::Create(uint8_t* buffer) const {
ByteWriter<uint32_t>::WriteBigEndian(&buffer[20], delay_since_last_sr()); ByteWriter<uint32_t>::WriteBigEndian(&buffer[20], delay_since_last_sr());
} }
bool ReportBlock::SetCumulativeLost(int32_t cumulative_lost) { bool ReportBlock::SetCumulativeLost(uint32_t cumulative_lost) {
// We have only 3 bytes to store it, and it's a signed value. if (cumulative_lost >= (1u << 24)) { // Have only 3 bytes to store it.
if (cumulative_lost >= (1 << 23) || cumulative_lost < -(1 << 23)) {
RTC_LOG(LS_WARNING) RTC_LOG(LS_WARNING)
<< "Cumulative lost is too big to fit into Report Block"; << "Cumulative lost is too big to fit into Report Block";
return false; return false;

View File

@ -17,8 +17,6 @@
namespace webrtc { namespace webrtc {
namespace rtcp { namespace rtcp {
// A ReportBlock represents the Sender Report packet from
// RFC 3550 section 6.4.1.
class ReportBlock { class ReportBlock {
public: public:
static const size_t kLength = 24; static const size_t kLength = 24;
@ -36,7 +34,7 @@ class ReportBlock {
void SetFractionLost(uint8_t fraction_lost) { void SetFractionLost(uint8_t fraction_lost) {
fraction_lost_ = fraction_lost; fraction_lost_ = fraction_lost;
} }
bool SetCumulativeLost(int32_t cumulative_lost); bool SetCumulativeLost(uint32_t cumulative_lost);
void SetExtHighestSeqNum(uint32_t ext_highest_seq_num) { void SetExtHighestSeqNum(uint32_t ext_highest_seq_num) {
extended_high_seq_num_ = ext_highest_seq_num; extended_high_seq_num_ = ext_highest_seq_num;
} }
@ -48,20 +46,20 @@ class ReportBlock {
uint32_t source_ssrc() const { return source_ssrc_; } uint32_t source_ssrc() const { return source_ssrc_; }
uint8_t fraction_lost() const { return fraction_lost_; } uint8_t fraction_lost() const { return fraction_lost_; }
int32_t cumulative_lost() const { return cumulative_lost_; } uint32_t cumulative_lost() const { return cumulative_lost_; }
uint32_t extended_high_seq_num() const { return extended_high_seq_num_; } uint32_t extended_high_seq_num() const { return extended_high_seq_num_; }
uint32_t jitter() const { return jitter_; } uint32_t jitter() const { return jitter_; }
uint32_t last_sr() const { return last_sr_; } uint32_t last_sr() const { return last_sr_; }
uint32_t delay_since_last_sr() const { return delay_since_last_sr_; } uint32_t delay_since_last_sr() const { return delay_since_last_sr_; }
private: private:
uint32_t source_ssrc_; // 32 bits uint32_t source_ssrc_;
uint8_t fraction_lost_; // 8 bits representing a fixed point value 0..1 uint8_t fraction_lost_;
int32_t cumulative_lost_; // Signed 24-bit value uint32_t cumulative_lost_;
uint32_t extended_high_seq_num_; // 32 bits uint32_t extended_high_seq_num_;
uint32_t jitter_; // 32 bits uint32_t jitter_;
uint32_t last_sr_; // 32 bits uint32_t last_sr_;
uint32_t delay_since_last_sr_; // 32 bits, units of 1/65536 seconds uint32_t delay_since_last_sr_;
}; };
} // namespace rtcp } // namespace rtcp

View File

@ -23,7 +23,7 @@ namespace {
const uint32_t kRemoteSsrc = 0x23456789; const uint32_t kRemoteSsrc = 0x23456789;
const uint8_t kFractionLost = 55; const uint8_t kFractionLost = 55;
// Use values that are streamed differently LE and BE. // Use values that are streamed differently LE and BE.
const int32_t kCumulativeLost = 0x111213; const uint32_t kCumulativeLost = 0x111213;
const uint32_t kExtHighestSeqNum = 0x22232425; const uint32_t kExtHighestSeqNum = 0x22232425;
const uint32_t kJitter = 0x33343536; const uint32_t kJitter = 0x33343536;
const uint32_t kLastSr = 0x44454647; const uint32_t kLastSr = 0x44454647;
@ -76,14 +76,10 @@ TEST(RtcpPacketReportBlockTest, ParseMatchCreate) {
} }
TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) { TEST(RtcpPacketReportBlockTest, ValidateCumulativeLost) {
// CumulativeLost is a signed 24-bit integer. const uint32_t kMaxCumulativeLost = 0xffffff;
const int32_t kMaxCumulativeLost = 0x7fffff;
const int32_t kMinCumulativeLost = -0x800000;
ReportBlock rb; ReportBlock rb;
EXPECT_FALSE(rb.SetCumulativeLost(kMaxCumulativeLost + 1)); EXPECT_FALSE(rb.SetCumulativeLost(kMaxCumulativeLost + 1));
EXPECT_TRUE(rb.SetCumulativeLost(kMaxCumulativeLost)); EXPECT_TRUE(rb.SetCumulativeLost(kMaxCumulativeLost));
EXPECT_FALSE(rb.SetCumulativeLost(kMinCumulativeLost - 1));
EXPECT_TRUE(rb.SetCumulativeLost(kMinCumulativeLost));
} }
} // namespace } // namespace

View File

@ -358,7 +358,7 @@ TEST_F(RtcpSenderTest, SendRrWithOneReportBlock) {
const rtcp::ReportBlock& rb = parser()->receiver_report()->report_blocks()[0]; const rtcp::ReportBlock& rb = parser()->receiver_report()->report_blocks()[0];
EXPECT_EQ(kRemoteSsrc, rb.source_ssrc()); EXPECT_EQ(kRemoteSsrc, rb.source_ssrc());
EXPECT_EQ(0U, rb.fraction_lost()); EXPECT_EQ(0U, rb.fraction_lost());
EXPECT_EQ(0, rb.cumulative_lost()); EXPECT_EQ(0U, rb.cumulative_lost());
EXPECT_EQ(kSeqNum, rb.extended_high_seq_num()); EXPECT_EQ(kSeqNum, rb.extended_high_seq_num());
} }