Use RtcpPacket to send FIR in RtcpSender

webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

Index: webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
index 6967754fe62f737e73f450314da31bf1ebd9dcdd..e08c20bfd5a6894596fb73168a8f1f319a8ca867 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -564,36 +564,20 @@ RTCPSender::BuildResult RTCPSender::BuildPLI(RtcpContext* ctx) {
 }
 
 RTCPSender::BuildResult RTCPSender::BuildFIR(RtcpContext* ctx) {
-  // sanity
-  if (ctx->position + 20 >= IP_PACKET_SIZE)
-    return BuildResult::kTruncated;
-
+  uint8_t seq = sequence_number_fir_;
   if (!ctx->repeat)
-    sequence_number_fir_++;  // do not increase if repetition
+    ++seq;  // Do not increase if repetition.
 
-  // add full intra request indicator
-  uint8_t FMT = 4;
-  *ctx->AllocateData(1) = 0x80 + FMT;
-  *ctx->AllocateData(1) = 206;
-
-  //Length of 4
-  *ctx->AllocateData(1) = 0;
-  *ctx->AllocateData(1) = 4;
-
-  // Add our own SSRC
-  ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4), ssrc_);
+  rtcp::Fir fir;
+  fir.From(ssrc_);
+  fir.To(remote_ssrc_);
+  fir.WithCommandSeqNum(seq);
 
-  // RFC 5104     4.3.1.2.  Semantics
-  // SSRC of media source
-  ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4), 0);
-
-  // Additional Feedback Control Information (FCI)
-  ByteWriter<uint32_t>::WriteBigEndian(ctx->AllocateData(4), remote_ssrc_);
+  PacketBuiltCallback callback(ctx);
+  if (!callback.BuildPacket(fir))
+    return BuildResult::kTruncated;
 
-  *ctx->AllocateData(1) = sequence_number_fir_;
-  *ctx->AllocateData(1) = 0;
-  *ctx->AllocateData(1) = 0;
-  *ctx->AllocateData(1) = 0;
+  sequence_number_fir_ = seq;  // Only update if we actually sent packet.

[åsapersson, 2015/08/04 12:07:27]

Will the caller know if the FIR request is not sent due to truncation? If not,
then a repeat for this request will correspond to the previous request.

[sprang_webrtc, 2015/08/04 12:59:38]

I updated the comment to the more clear "Update state only if we built the
packet."
That is, we don't want to update the state of the sequence number if we don't
even have space enough to build the packet. The behavior is the same as before,
we just need this temporary since the old code would return early in the
truncation case.

 
   TRACE_EVENT_INSTANT0(TRACE_DISABLED_BY_DEFAULT("webrtc_rtp"),
                        "RTCPSender::FIR");