Refactor TransportFeedback ensuring it's consistency:

webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_TRANSPORT_FEEDBACK_H_
 #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_TRANSPORT_FEEDBACK_H_
 
-#include <deque>
 #include <memory>
 #include <vector>
 
 #include "webrtc/base/constructormagic.h"
-#include "webrtc/base/deprecation.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rtpfb.h"
 
 namespace webrtc {
 namespace rtcp {
 class CommonHeader;
 
 class TransportFeedback : public Rtpfb {
  public:
-  class PacketStatusChunk;
   // TODO(sprang): IANA reg?
   static constexpr uint8_t kFeedbackMessageType = 15;
   // Convert to multiples of 0.25ms.
   static constexpr int kDeltaScaleFactor = 250;
+  // Maximum number of packets (including missing) TransportFeedback can report.
+  static constexpr size_t kMaxReportedPackets = 0xffff;
 
   TransportFeedback();
   ~TransportFeedback() override;
 
   void SetBase(uint16_t base_sequence,     // Seq# of first packet in this msg.
                int64_t ref_timestamp_us);  // Reference timestamp for this msg.
   void SetFeedbackSequenceNumber(uint8_t feedback_sequence);
   // NOTE: This method requires increasing sequence numbers (excepting wraps).
   bool AddReceivedPacket(uint16_t sequence_number, int64_t timestamp_us);
 
@@ skipping 10 matching lines @@
   // Get the reference time in microseconds, including any precision loss.
   int64_t GetBaseTimeUs() const;
   // Convenience method for getting all deltas as microseconds. The first delta
   // is relative the base time.
   std::vector<int64_t> GetReceiveDeltasUs() const;
 
   bool Parse(const CommonHeader& packet);
   static std::unique_ptr<TransportFeedback> ParseFrom(const uint8_t* buffer,
                                                       size_t length);
 
-  RTC_DEPRECATED
-  void WithPacketSenderSsrc(uint32_t ssrc) { SetSenderSsrc(ssrc); }
-  RTC_DEPRECATED
-  void WithMediaSourceSsrc(uint32_t ssrc) { SetMediaSsrc(ssrc); }
-  RTC_DEPRECATED
-  void WithBase(uint16_t base_sequence, int64_t ref_timestamp_us) {
-    SetBase(base_sequence, ref_timestamp_us);
-  }
-  RTC_DEPRECATED
-  void WithFeedbackSequenceNumber(uint8_t feedback_sequence) {
-    SetFeedbackSequenceNumber(feedback_sequence);
-  }
-  RTC_DEPRECATED
-  bool WithReceivedPacket(uint16_t sequence_number, int64_t timestamp_us) {
-    return AddReceivedPacket(sequence_number, timestamp_us);
-  }
-
  protected:
   bool Create(uint8_t* packet,
               size_t* position,
               size_t max_length,
               PacketReadyCallback* callback) const override;
 
   size_t BlockLength() const override;
 
  private:
-  static PacketStatusChunk* ParseChunk(const uint8_t* buffer, size_t max_size);
+  using DeltaSize = uint8_t;

[sprang_webrtc, 2017/01/12 13:18:46]

Add comment static that this represents the byte size of the encoded time delta,
so it's not confused with the size of the delta itself.

[danilchap, 2017/01/16 09:45:15]

Done.

+  class LastChunk;

[sprang_webrtc, 2017/01/12 13:18:46]

I'm not sure about this name. Could you update it or add a comment which makes
it clear that this is a buffer where the last symbols are stored until the
suitable chunk type to encode to is determined.

[danilchap, 2017/01/16 09:45:15]

copied comment from definition of the class.
Not sure about better name: it is used to keep DeltaSizes for the last chunk in
case no more ReceivedPackets will be reported.

[sprang_webrtc, 2017/01/16 10:41:24]

Let's keep the name, the comment is good.

+  struct ReceivedPacket {
+    ReceivedPacket(uint16_t sequence_number, int16_t delta_ticks)
+        : sequence_number(sequence_number), delta_ticks(delta_ticks) {}
+    uint16_t sequence_number;
+    int16_t delta_ticks;
+  };
+  // Pre and postcondition for all public methods.
+  bool IsConsistent() const;
 
-  int64_t Unwrap(uint16_t sequence_number);
-  bool AddSymbol(StatusSymbol symbol, int64_t seq);
-  bool Encode(StatusSymbol symbol);
-  bool HandleRleCandidate(StatusSymbol symbol,
-                          int current_capacity,
-                          int delta_size);
-  void EmitRemaining();
-  void EmitVectorChunk();
-  void EmitRunLengthChunk();
+  // Reset packet to consistent empty state.
+  void Clear();
 
-  int32_t base_seq_;
-  int64_t base_time_;
+  bool AddDeltaSize(DeltaSize delta_size);
+
+  uint16_t base_seq_no_;
+  uint16_t num_seq_no_;
+  int32_t base_time_ticks_;
   uint8_t feedback_seq_;
-  std::vector<PacketStatusChunk*> status_chunks_;
-  std::vector<int16_t> receive_deltas_;
 
-  int64_t last_seq_;
-  int64_t last_timestamp_;
-  std::deque<StatusSymbol> symbol_vec_;
-  uint16_t first_symbol_cardinality_;
-  bool vec_needs_two_bit_symbols_;
-  uint32_t size_bytes_;
+  int64_t last_timestamp_us_;
+  std::vector<ReceivedPacket> packets_;
+  std::vector<uint16_t> chunks_;

[sprang_webrtc, 2017/01/12 13:18:46]

Can you comment or possibly typedef this so it's clear that this is the encoded
2 byte status chunkgs?

[danilchap, 2017/01/16 09:45:15]

is rename enough?
Added also a comment, is helpful or redundant?

[sprang_webrtc, 2017/01/16 10:41:25]

This is fine with me, thanks!

+  const std::unique_ptr<LastChunk> last_chunk_;
+  size_t size_bytes_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(TransportFeedback);
 };
 
 }  // namespace rtcp
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_TRANSPORT_FEEDBACK_H_