Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(78)

Unified Diff: webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h

Issue 2616343003: Refactor TransportFeedback ensuring it's consistency: (Closed)
Patch Set: -unused headers Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
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 03a1c4f728ac217d4cce1c0d03add9cb1bef7eb7..7f70dbc487962ef5849e2e9041b9371307983061 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h
@@ -11,12 +11,10 @@
#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 {
@@ -25,11 +23,12 @@ 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;
@@ -60,23 +59,6 @@ class TransportFeedback : public Rtpfb {
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,
@@ -86,30 +68,32 @@ class TransportFeedback : public Rtpfb {
size_t BlockLength() const override;
private:
- static PacketStatusChunk* ParseChunk(const uint8_t* buffer, size_t max_size);
-
- 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();
-
- int32_t base_seq_;
- int64_t base_time_;
+ using DeltaSize = uint8_t;
sprang_webrtc 2017/01/12 13:18:46 Add comment static that this represents the byte s
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
danilchap 2017/01/16 09:45:15 copied comment from definition of the class. Not s
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;
+
+ // Reset packet to consistent empty state.
+ void Clear();
+
+ 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 c
danilchap 2017/01/16 09:45:15 is rename enough? Added also a comment, is helpful
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);
};

Powered by Google App Engine
This is Rietveld 408576698