Make Call::OnRecoveredPacket parse RTP header and call OnRtpPacket.

webrtc/video/rtp_stream_receiver.cc

Index: webrtc/video/rtp_stream_receiver.cc
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index 88d86ef986714df85332333ef5cff9d6849bd1e0..1f96abc7642850eb49666357af0338183c1e1cce 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -275,6 +275,9 @@ int32_t RtpStreamReceiver::OnReceivedPayloadData(
   return 0;
 }
 
+// TODO(nisse): Try to delete this method. Obstacles: It is used by
+// ParseAndHandleEncapsulatingHeader, for handling Rtx packets. And
+// it's part of the RtpData interface which we implement.
 bool RtpStreamReceiver::OnRecoveredPacket(const uint8_t* rtp_packet,
                                           size_t rtp_packet_length) {
   RTPHeader header;
@@ -302,6 +305,8 @@ void RtpStreamReceiver::OnIncomingSSRCChanged(const uint32_t ssrc) {
   rtp_rtcp_->SetRemoteSSRC(ssrc);
 }
 
+// This method handles both regular RTP packets and packets recovered
+// via FlexFEC.
 void RtpStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
   {
     rtc::CritScope lock(&receive_cs_);
@@ -309,29 +314,30 @@ void RtpStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
       return;
     }
   }
-
-  int64_t now_ms = clock_->TimeInMilliseconds();
-
-  {
-    // Periodically log the RTP header of incoming packets.
-    rtc::CritScope lock(&receive_cs_);
-    if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) {
-      std::stringstream ss;
-      ss << "Packet received on SSRC: " << packet.Ssrc()
-         << " with payload type: " << static_cast<int>(packet.PayloadType())
-         << ", timestamp: " << packet.Timestamp()
-         << ", sequence number: " << packet.SequenceNumber()
-         << ", arrival time: " << packet.arrival_time_ms();
-      int32_t time_offset;
-      if (packet.GetExtension<TransmissionOffset>(&time_offset)) {
-        ss << ", toffset: " << time_offset;
+  if (!packet.recovered()) {
+    int64_t now_ms = clock_->TimeInMilliseconds();
+
+    {
+      // Periodically log the RTP header of incoming packets.
+      rtc::CritScope lock(&receive_cs_);
+      if (now_ms - last_packet_log_ms_ > kPacketLogIntervalMs) {
+        std::stringstream ss;
+        ss << "Packet received on SSRC: " << packet.Ssrc()
+           << " with payload type: " << static_cast<int>(packet.PayloadType())
+           << ", timestamp: " << packet.Timestamp()
+           << ", sequence number: " << packet.SequenceNumber()
+           << ", arrival time: " << packet.arrival_time_ms();
+        int32_t time_offset;
+        if (packet.GetExtension<TransmissionOffset>(&time_offset)) {
+          ss << ", toffset: " << time_offset;
+        }
+        uint32_t send_time;
+        if (packet.GetExtension<AbsoluteSendTime>(&send_time)) {
+          ss << ", abs send time: " << send_time;
+        }
+        LOG(LS_INFO) << ss.str();
+        last_packet_log_ms_ = now_ms;
       }
-      uint32_t send_time;
-      if (packet.GetExtension<AbsoluteSendTime>(&send_time)) {
-        ss << ", abs send time: " << send_time;
-      }
-      LOG(LS_INFO) << ss.str();
-      last_packet_log_ms_ = now_ms;
     }

[sprang_webrtc, 2017/05/11 08:48:58]

nit: can we clean up the extra scopes for locking here? I don't think it's
justified to have one for checking if we're receiving above, then release it
only to (potentially) grab it again after checking the time....

[nisse-webrtc, 2017/05/11 09:04:01]

Done.

   }
 
@@ -343,13 +349,20 @@ void RtpStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
   header.payload_type_frequency = kVideoPayloadTypeFrequency;
 
   bool in_order = IsPacketInOrder(header);
-  rtp_payload_registry_.SetIncomingPayloadType(header);
+  if (!packet.recovered()) {
+    // TODO(nisse): Why isn't this done for recovered packets?

[sprang_webrtc, 2017/05/11 08:48:58]

Will the header here have the actual payload type, or will it have ulpfec or red
or rtx some other payload type we don't want to register for this ssrc?
Anyway, that logic might be better put in
rtp_payload_registry_.SetIncomingPayloadType() since that's where rtx is
filtered out...

[nisse-webrtc, 2017/05/11 09:04:01]

No idea. At the moment I don't think rtx or ulpfec enter this code path.

+    rtp_payload_registry_.SetIncomingPayloadType(header);
+  }
   ReceivePacket(packet.data(), packet.size(), header, in_order);
   // Update receive statistics after ReceivePacket.
   // Receive statistics will be reset if the payload type changes (make sure
   // that the first packet is included in the stats).
-  rtp_receive_statistics_->IncomingPacket(
-      header, packet.size(), IsPacketRetransmitted(header, in_order));
+  if (!packet.recovered()) {
+    // TODO(nisse): We should pass a recovered flag to stats, to aid
+    // fixing bug bugs.webrtc.org/6339.
+    rtp_receive_statistics_->IncomingPacket(
+        header, packet.size(), IsPacketRetransmitted(header, in_order));
+  }
 }
 
 int32_t RtpStreamReceiver::RequestKeyFrame() {