Call should allow pass through of keep-alive packets.

webrtc/call/call.cc

 /*
  *  Copyright (c) 2013 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.
  */
 
@@ skipping 218 matching lines @@
                             const uint8_t* packet,
                             size_t length,
                             const PacketTime& packet_time);
   void ConfigureSync(const std::string& sync_group)
       EXCLUSIVE_LOCKS_REQUIRED(receive_crit_);
 
   void NotifyBweOfReceivedPacket(const RtpPacketReceived& packet,
                                  MediaType media_type)
       SHARED_LOCKS_REQUIRED(receive_crit_);
 
-  rtc::Optional<RtpPacketReceived> ParseRtpPacket(const uint8_t* packet,
-                                                  size_t length,
-                                                  const PacketTime* packet_time)
-      SHARED_LOCKS_REQUIRED(receive_crit_);
+  rtc::Optional<RtpPacketReceived> ParseRtpPacket(
+      const uint8_t* packet,
+      size_t length,
+      const PacketTime* packet_time) const;
 
   void UpdateSendHistograms(int64_t first_sent_packet_ms)
       EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_);
   void UpdateReceiveHistograms();
   void UpdateHistograms();
   void UpdateAggregateNetworkState();
 
   // Applies update to the BitrateConfig cached in |config_|, restarting
   // bandwidth estimation from |new_start| if set.
   void UpdateCurrentBitrateConfig(const rtc::Optional<int>& new_start);
@@ skipping 225 matching lines @@
   }
   UpdateReceiveHistograms();
   UpdateHistograms();
 
   Trace::ReturnTrace();
 }
 
 rtc::Optional<RtpPacketReceived> Call::ParseRtpPacket(
     const uint8_t* packet,
     size_t length,
-    const PacketTime* packet_time) {
+    const PacketTime* packet_time) const {
   RtpPacketReceived parsed_packet;
   if (!parsed_packet.Parse(packet, length))
     return rtc::Optional<RtpPacketReceived>();
 
   int64_t arrival_time_ms;
   if (packet_time && packet_time->timestamp != -1) {
     arrival_time_ms = (packet_time->timestamp + 500) / 1000;
   } else {
     arrival_time_ms = clock_->TimeInMilliseconds();
   }
@@ skipping 793 matching lines @@
 
   return rtcp_delivered ? DELIVERY_OK : DELIVERY_PACKET_ERROR;
 }
 
 PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
                                                 const uint8_t* packet,
                                                 size_t length,
                                                 const PacketTime& packet_time) {
   TRACE_EVENT0("webrtc", "Call::DeliverRtp");
 
-  RTC_DCHECK(media_type == MediaType::AUDIO || media_type == MediaType::VIDEO);
-
-  ReadLockScoped read_lock(*receive_crit_);
   // TODO(nisse): We should parse the RTP header only here, and pass
   // on parsed_packet to the receive streams.
   rtc::Optional<RtpPacketReceived> parsed_packet =
       ParseRtpPacket(packet, length, &packet_time);
+  const bool is_keep_alive_packet =

[pbos-webrtc, 2017/07/10 22:36:31]

Can you put some reference to keepalive packets to motivate this check? Should
this also be verifying a payload type?

[pbos-webrtc, 2017/07/10 22:39:16]

Reading the other CL, please check config_.rtp.keepalive.payload_type here too
(instead? does payload_size() matter here?), if this is signalled from the other
side and not just a send-payload-type attribute.

[sprang_webrtc, 2017/07/11 08:33:06]

This is based on https://tools.ietf.org/html/rfc6263#section-4.6
The intent is that the packets should have an explicitly unknown payload type.
RFC3550 states "a receiver MUST ignore packets with payload types that it does
not understand".
The packets should also have zero payload length, since we have a special case
for such keep-alive packets in the receiver; if the payload length is non-zero,
we will log a warning.

So we can't check the payload type on the receiver. (This method is receive side
only, right?)
I'll add a comment.

+      parsed_packet && parsed_packet->payload_size() == 0;
+
+  RTC_DCHECK(media_type == MediaType::AUDIO || media_type == MediaType::VIDEO ||
+             is_keep_alive_packet);
 
   if (!parsed_packet)
     return DELIVERY_PACKET_ERROR;
 
+  ReadLockScoped read_lock(*receive_crit_);
   auto it = receive_rtp_config_.find(parsed_packet->Ssrc());
   if (it == receive_rtp_config_.end()) {
     LOG(LS_ERROR) << "receive_rtp_config_ lookup failed for ssrc "
                   << parsed_packet->Ssrc();
     // Destruction of the receive stream, including deregistering from the
     // RtpDemuxer, is not protected by the |receive_crit_| lock. But
     // deregistering in the |receive_rtp_config_| map is protected by that lock.
     // So by not passing the packet on to demuxing in this case, we prevent
     // incoming packets to be passed on via the demuxer to a receive stream
     // which is being torned down.
@@ skipping 84 matching lines @@
       (use_send_side_bwe && header.extension.hasTransportSequenceNumber)) {
     receive_side_cc_.OnReceivedPacket(
         packet.arrival_time_ms(), packet.payload_size() + packet.padding_size(),
         header);
   }
 }
 
 }  // namespace internal
 
 }  // namespace webrtc