From c8a1cccd0a80b35a5b6846f6efe6082f96c29083 Mon Sep 17 00:00:00 2001
From: sprang <sprang@webrtc.org>
Date: Fri, 4 Sep 2015 04:38:13 -0700
Subject: [PATCH] Fixed base time in TransportFeedback message writing.

Value was incorrectly truncated to 16 bits when serializing the message.
Fixed, with added regression tests.

BUG=

Review URL: https://codereview.webrtc.org/1294393002

Cr-Commit-Position: refs/heads/master@{#9858}
---
 .../source/rtcp_packet/transport_feedback.cc  |  8 ++-----
 .../source/rtcp_packet/transport_feedback.h   |  1 -
 .../transport_feedback_unittest.cc            | 24 +++++++++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc
index b426887d06..fee634b383 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc
@@ -526,12 +526,8 @@ uint16_t TransportFeedback::GetBaseSequence() const {
   return base_seq_;
 }
 
-int32_t TransportFeedback::GetBaseTime() const {
-  return static_cast<int32_t>(base_time_ & 0x00FFFFFF);
-}
-
 int64_t TransportFeedback::GetBaseTimeUs() const {
-  return GetBaseTime() * kBaseScaleFactor;
+  return base_time_ * kBaseScaleFactor;
 }
 
 std::vector<TransportFeedback::StatusSymbol>
@@ -591,7 +587,7 @@ bool TransportFeedback::Create(uint8_t* packet,
   *position += 2;
 
   ByteWriter<int32_t, 3>::WriteBigEndian(&packet[*position],
-                                         static_cast<int16_t>(base_time_));
+                                         static_cast<int32_t>(base_time_));
   *position += 3;
 
   packet[(*position)++] = feedback_seq_;
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
index 6e51fb88b6..357355d281 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
@@ -43,7 +43,6 @@ class TransportFeedback : public RtcpPacket {
   };
 
   uint16_t GetBaseSequence() const;
-  int32_t GetBaseTime() const;
   std::vector<TransportFeedback::StatusSymbol> GetStatusVector() const;
   std::vector<int16_t> GetReceiveDeltas() const;
 
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc
index 6a26d1cfad..76b51df0ef 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc
@@ -389,6 +389,30 @@ TEST(RtcpPacketTest, TransportFeedback_Limits) {
       1, kMaxNegativeTimeDelta - TransportFeedback::kDeltaScaleFactor));
   EXPECT_TRUE(packet->WithReceivedPacket(1, kMaxNegativeTimeDelta));
 
+  // Base time at maximum value.
+  int64_t kMaxBaseTime =
+      static_cast<int64_t>(TransportFeedback::kDeltaScaleFactor) * (1L << 8) *
+      ((1L << 23) - 1);
+  packet.reset(new TransportFeedback());
+  packet->WithBase(0, kMaxBaseTime);
+  packet->WithReceivedPacket(0, kMaxBaseTime);
+  // Serialize and de-serialize (verify 24bit parsing).
+  rtc::scoped_ptr<rtcp::RawPacket> raw_packet = packet->Build();
+  packet =
+      TransportFeedback::ParseFrom(raw_packet->Buffer(), raw_packet->Length());
+  EXPECT_EQ(kMaxBaseTime, packet->GetBaseTimeUs());
+
+  // Base time above maximum value.
+  int64_t kTooLargeBaseTime =
+      kMaxBaseTime + (TransportFeedback::kDeltaScaleFactor * (1L << 8));
+  packet.reset(new TransportFeedback());
+  packet->WithBase(0, kTooLargeBaseTime);
+  packet->WithReceivedPacket(0, kTooLargeBaseTime);
+  raw_packet = packet->Build();
+  packet =
+      TransportFeedback::ParseFrom(raw_packet->Buffer(), raw_packet->Length());
+  EXPECT_NE(kTooLargeBaseTime, packet->GetBaseTimeUs());
+
   // TODO(sprang): Once we support max length lower than RTCP length limit,
   // add back test for max size in bytes.
 }