Always call RemoteBitrateEstimator::IncomingPacket from Call.

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 92 matching lines @@
 
   // Implements PacketReceiver.
   DeliveryStatus DeliverPacket(MediaType media_type,
                                const uint8_t* packet,
                                size_t length,
                                const PacketTime& packet_time) override;
 
   // Implements RecoveredPacketReceiver.
   bool OnRecoveredPacket(const uint8_t* packet, size_t length) override;
 
-  void NotifyBweOfReceivedPacket(const RtpPacketReceived& packet);
-
   void SetBitrateConfig(
       const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
 
   void SignalChannelNetworkState(MediaType media, NetworkState state) override;
 
   void OnTransportOverheadChanged(MediaType media,
                                   int transport_overhead_per_packet) override;
 
   void OnNetworkRouteChanged(const std::string& transport_name,
                              const rtc::NetworkRoute& network_route) override;
@@ skipping 23 matching lines @@
 
   VoiceEngine* voice_engine() {
     internal::AudioState* audio_state =
         static_cast<internal::AudioState*>(config_.audio_state.get());
     if (audio_state)
       return audio_state->voice_engine();
     else
       return nullptr;
   }
 
+  void NotifyBweOfReceivedPacket(const RtpPacketReceived& packet)
+      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_);
 
   void UpdateSendHistograms() EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_);
   void UpdateReceiveHistograms();
   void UpdateHistograms();
   void UpdateAggregateNetworkState();
 
@@ skipping 23 matching lines @@
   // streams.
   std::multimap<uint32_t, FlexfecReceiveStreamImpl*>
       flexfec_receive_ssrcs_media_ GUARDED_BY(receive_crit_);
   std::map<uint32_t, FlexfecReceiveStreamImpl*>
       flexfec_receive_ssrcs_protection_ GUARDED_BY(receive_crit_);
   std::set<FlexfecReceiveStreamImpl*> flexfec_receive_streams_
       GUARDED_BY(receive_crit_);
   std::map<std::string, AudioReceiveStream*> sync_stream_mapping_
       GUARDED_BY(receive_crit_);
 
-  // Registered RTP header extensions for each stream.
-  // Note that RTP header extensions are negotiated per track ("m= line") in the
-  // SDP, but we have no notion of tracks at the Call level. We therefore store
-  // the RTP header extensions per SSRC instead, which leads to some storage
-  // overhead.
-  std::map<uint32_t, RtpHeaderExtensionMap> received_rtp_header_extensions_
+  // This extra map is used for receive processing which is
+  // independent of media type.
+
+  // TODO(nisse): In the RTP transport refactoring, we should have a
+  // single mapping from ssrc to a more abstract receive stream, with
+  // accessor methods for all configuration we need at this level.
+  struct ReceiveRtpConfig {
+    ReceiveRtpConfig() = default;  // Needed by std::map
+    ReceiveRtpConfig(const std::vector<RtpExtension>& extensions,
+                     bool transport_cc)
+        : extensions(extensions), transport_cc(transport_cc) {}
+
+    // Registered RTP header extensions for each stream. Note that RTP header
+    // extensions are negotiated per track ("m= line") in the SDP, but we have
+    // no notion of tracks at the Call level. We therefore store the RTP header
+    // extensions per SSRC instead, which leads to some storage overhead.
+    RtpHeaderExtensionMap extensions;
+    // True if send side bandwith estimation was negotiated.
+    bool transport_cc;
+  };
+  std::map<uint32_t, ReceiveRtpConfig> receive_rtp_config_
       GUARDED_BY(receive_crit_);
 
   std::unique_ptr<RWLockWrapper> send_crit_;
   // Audio and Video send streams are owned by the client that creates them.
   std::map<uint32_t, AudioSendStream*> audio_send_ssrcs_ GUARDED_BY(send_crit_);
   std::map<uint32_t, VideoSendStream*> video_send_ssrcs_ GUARDED_BY(send_crit_);
   std::set<VideoSendStream*> video_send_streams_ GUARDED_BY(send_crit_);
 
   VideoSendStream::RtpStateMap suspended_video_send_ssrcs_;
   webrtc::RtcEventLog* event_log_;
@@ skipping 143 matching lines @@
 }
 
 rtc::Optional<RtpPacketReceived> Call::ParseRtpPacket(
     const uint8_t* packet,
     size_t length,
     const PacketTime& packet_time) {
   RtpPacketReceived parsed_packet;
   if (!parsed_packet.Parse(packet, length))
     return rtc::Optional<RtpPacketReceived>();
 
-  auto it = received_rtp_header_extensions_.find(parsed_packet.Ssrc());
-  if (it != received_rtp_header_extensions_.end())
-    parsed_packet.IdentifyExtensions(it->second);
+  auto it = receive_rtp_config_.find(parsed_packet.Ssrc());
+  if (it != receive_rtp_config_.end())
+    parsed_packet.IdentifyExtensions(it->second.extensions);
 
   int64_t arrival_time_ms;
   if (packet_time.timestamp != -1) {
     arrival_time_ms = (packet_time.timestamp + 500) / 1000;
   } else {
     arrival_time_ms = clock_->TimeInMilliseconds();
   }
   parsed_packet.set_arrival_time_ms(arrival_time_ms);
 
   return rtc::Optional<RtpPacketReceived>(std::move(parsed_packet));
@@ skipping 137 matching lines @@
   AudioReceiveStream* receive_stream = new AudioReceiveStream(
       &packet_router_,
       // TODO(nisse): Used only when UseSendSideBwe(config) is true.
       congestion_controller_->GetRemoteBitrateEstimator(true), config,
       config_.audio_state, event_log_);
   {
     WriteLockScoped write_lock(*receive_crit_);
     RTC_DCHECK(audio_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
                audio_receive_ssrcs_.end());
     audio_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
+    receive_rtp_config_[config.rtp.remote_ssrc] =
+        ReceiveRtpConfig(config.rtp.extensions, config.rtp.transport_cc);
+
     ConfigureSync(config.sync_group);
   }
   {
     ReadLockScoped read_lock(*send_crit_);
     auto it = audio_send_ssrcs_.find(config.rtp.local_ssrc);
     if (it != audio_send_ssrcs_.end()) {
       receive_stream->AssociateSendStream(it->second);
     }
   }
   receive_stream->SignalNetworkState(audio_network_state_);
   UpdateAggregateNetworkState();
   return receive_stream;
 }
 
 void Call::DestroyAudioReceiveStream(
     webrtc::AudioReceiveStream* receive_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyAudioReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(receive_stream != nullptr);
   webrtc::internal::AudioReceiveStream* audio_receive_stream =
       static_cast<webrtc::internal::AudioReceiveStream*>(receive_stream);
   {
     WriteLockScoped write_lock(*receive_crit_);
-    size_t num_deleted = audio_receive_ssrcs_.erase(
-        audio_receive_stream->config().rtp.remote_ssrc);
+    uint32_t ssrc = audio_receive_stream->config().rtp.remote_ssrc;
+
+    size_t num_deleted = audio_receive_ssrcs_.erase(ssrc);
     RTC_DCHECK(num_deleted == 1);
     const std::string& sync_group = audio_receive_stream->config().sync_group;
     const auto it = sync_stream_mapping_.find(sync_group);
     if (it != sync_stream_mapping_.end() &&
         it->second == audio_receive_stream) {
       sync_stream_mapping_.erase(it);
       ConfigureSync(sync_group);
     }
+    receive_rtp_config_.erase(ssrc);
   }
   UpdateAggregateNetworkState();
   delete audio_receive_stream;
 }
 
 webrtc::VideoSendStream* Call::CreateVideoSendStream(
     webrtc::VideoSendStream::Config config,
     VideoEncoderConfig encoder_config) {
   TRACE_EVENT0("webrtc", "Call::CreateVideoSendStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
@@ skipping 64 matching lines @@
 webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream(
     webrtc::VideoReceiveStream::Config configuration) {
   TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   VideoReceiveStream* receive_stream = new VideoReceiveStream(
       num_cpu_cores_, congestion_controller_.get(), &packet_router_,
       std::move(configuration), voice_engine(), module_process_thread_.get(),
       call_stats_.get(), &remb_);
 
   const webrtc::VideoReceiveStream::Config& config = receive_stream->config();
+  ReceiveRtpConfig receive_config(config.rtp.extensions,
+                                  config.rtp.transport_cc);
   {
     WriteLockScoped write_lock(*receive_crit_);
     RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
                video_receive_ssrcs_.end());
     video_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
+    receive_rtp_config_[config.rtp.remote_ssrc] = receive_config;
     // TODO(pbos): Configure different RTX payloads per receive payload.
     VideoReceiveStream::Config::Rtp::RtxMap::const_iterator it =
         config.rtp.rtx.begin();
-    if (it != config.rtp.rtx.end())
+    if (it != config.rtp.rtx.end()) {
       video_receive_ssrcs_[it->second.ssrc] = receive_stream;
+      receive_rtp_config_[it->second.ssrc] = receive_config;
+    }
     video_receive_streams_.insert(receive_stream);
+
     ConfigureSync(config.sync_group);
   }
   receive_stream->SignalNetworkState(video_network_state_);
   UpdateAggregateNetworkState();
   event_log_->LogVideoReceiveStreamConfig(config);
   return receive_stream;
 }
 
 void Call::DestroyVideoReceiveStream(
     webrtc::VideoReceiveStream* receive_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyVideoReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(receive_stream != nullptr);
   VideoReceiveStream* receive_stream_impl = nullptr;
   {
     WriteLockScoped write_lock(*receive_crit_);
     // Remove all ssrcs pointing to a receive stream. As RTX retransmits on a
     // separate SSRC there can be either one or two.
     auto it = video_receive_ssrcs_.begin();
     while (it != video_receive_ssrcs_.end()) {
       if (it->second == static_cast<VideoReceiveStream*>(receive_stream)) {
         if (receive_stream_impl != nullptr)
           RTC_DCHECK(receive_stream_impl == it->second);
         receive_stream_impl = it->second;
-        video_receive_ssrcs_.erase(it++);
+        receive_rtp_config_.erase(it->first);
+        it = video_receive_ssrcs_.erase(it);
       } else {
         ++it;
       }
     }
     video_receive_streams_.erase(receive_stream_impl);
     RTC_CHECK(receive_stream_impl != nullptr);
     ConfigureSync(receive_stream_impl->config().sync_group);
   }
   UpdateAggregateNetworkState();
   delete receive_stream_impl;
@@ skipping 16 matching lines @@
                flexfec_receive_streams_.end());
     flexfec_receive_streams_.insert(receive_stream);
 
     for (auto ssrc : config.protected_media_ssrcs)
       flexfec_receive_ssrcs_media_.insert(std::make_pair(ssrc, receive_stream));
 
     RTC_DCHECK(flexfec_receive_ssrcs_protection_.find(config.remote_ssrc) ==
                flexfec_receive_ssrcs_protection_.end());
     flexfec_receive_ssrcs_protection_[config.remote_ssrc] = receive_stream;
 
-    RTC_DCHECK(received_rtp_header_extensions_.find(config.remote_ssrc) ==
-               received_rtp_header_extensions_.end());
-    RtpHeaderExtensionMap rtp_header_extensions(config.rtp_header_extensions);
-    received_rtp_header_extensions_[config.remote_ssrc] = rtp_header_extensions;
+    RTC_DCHECK(receive_rtp_config_.find(config.remote_ssrc) ==
+               receive_rtp_config_.end());
+    receive_rtp_config_[config.remote_ssrc] =
+        ReceiveRtpConfig(config.rtp_header_extensions, config.transport_cc);
   }
 
   // TODO(brandtr): Store config in RtcEventLog here.
 
   return receive_stream;
 }
 
 void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) {
   TRACE_EVENT0("webrtc", "Call::DestroyFlexfecReceiveStream");
   RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
 
   RTC_DCHECK(receive_stream != nullptr);
   // There exist no other derived classes of FlexfecReceiveStream,
   // so this downcast is safe.
   FlexfecReceiveStreamImpl* receive_stream_impl =
       static_cast<FlexfecReceiveStreamImpl*>(receive_stream);
   {
     WriteLockScoped write_lock(*receive_crit_);
 
     uint32_t ssrc = receive_stream_impl->GetConfig().remote_ssrc;
-    received_rtp_header_extensions_.erase(ssrc);
+    receive_rtp_config_.erase(ssrc);
 
     // Remove all SSRCs pointing to the FlexfecReceiveStreamImpl to be
     // destroyed.
     auto prot_it = flexfec_receive_ssrcs_protection_.begin();
     while (prot_it != flexfec_receive_ssrcs_protection_.end()) {
       if (prot_it->second == receive_stream_impl)
         prot_it = flexfec_receive_ssrcs_protection_.erase(prot_it);
       else
         ++prot_it;
     }
@@ skipping 350 matching lines @@
     event_log_->LogRtcpPacket(kIncomingPacket, media_type, packet, length);
 
   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");
-  // Minimum RTP header size.
-  if (length < 12)
+
+  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);
+
+  if (!parsed_packet)
     return DELIVERY_PACKET_ERROR;
 
-  uint32_t ssrc = ByteReader<uint32_t>::ReadBigEndian(&packet[8]);
-  ReadLockScoped read_lock(*receive_crit_);
+  NotifyBweOfReceivedPacket(*parsed_packet);
+
+  uint32_t ssrc = parsed_packet->Ssrc();
+
   if (media_type == MediaType::ANY || media_type == MediaType::AUDIO) {
     auto it = audio_receive_ssrcs_.find(ssrc);
     if (it != audio_receive_ssrcs_.end()) {
       received_bytes_per_second_counter_.Add(static_cast<int>(length));
       received_audio_bytes_per_second_counter_.Add(static_cast<int>(length));
       auto status = it->second->DeliverRtp(packet, length, packet_time)
                         ? DELIVERY_OK
                         : DELIVERY_PACKET_ERROR;
       if (status == DELIVERY_OK)
         event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
       return status;
     }
   }
   if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) {
     auto it = video_receive_ssrcs_.find(ssrc);
     if (it != video_receive_ssrcs_.end()) {
       received_bytes_per_second_counter_.Add(static_cast<int>(length));
       received_video_bytes_per_second_counter_.Add(static_cast<int>(length));
       // TODO(brandtr): Notify the BWE of received media packets here.
       auto status = it->second->DeliverRtp(packet, length, packet_time)
                         ? DELIVERY_OK
                         : DELIVERY_PACKET_ERROR;
       // Deliver media packets to FlexFEC subsystem. RTP header extensions need
       // not be parsed, as FlexFEC is oblivious to the semantic meaning of the
       // packet contents beyond the 12 byte RTP base header. The BWE is fed
       // information about these media packets from the regular media pipeline.
-      rtc::Optional<RtpPacketReceived> parsed_packet =
-          ParseRtpPacket(packet, length, packet_time);
       if (parsed_packet) {
         auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc);
         for (auto it = it_bounds.first; it != it_bounds.second; ++it)
           it->second->AddAndProcessReceivedPacket(*parsed_packet);
       }
       if (status == DELIVERY_OK)
         event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
       return status;
     }
   }
   if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) {
     auto it = flexfec_receive_ssrcs_protection_.find(ssrc);
     if (it != flexfec_receive_ssrcs_protection_.end()) {
-      rtc::Optional<RtpPacketReceived> parsed_packet =
-          ParseRtpPacket(packet, length, packet_time);
       if (parsed_packet) {
-        NotifyBweOfReceivedPacket(*parsed_packet);
         auto status = it->second->AddAndProcessReceivedPacket(*parsed_packet)
                           ? DELIVERY_OK
                           : DELIVERY_PACKET_ERROR;
         if (status == DELIVERY_OK)
           event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
         return status;
       }
     }
   }
   return DELIVERY_UNKNOWN_SSRC;
@@ skipping 19 matching lines @@
 bool Call::OnRecoveredPacket(const uint8_t* packet, size_t length) {
   uint32_t ssrc = ByteReader<uint32_t>::ReadBigEndian(&packet[8]);
   ReadLockScoped read_lock(*receive_crit_);
   auto it = video_receive_ssrcs_.find(ssrc);
   if (it == video_receive_ssrcs_.end())
     return false;
   return it->second->OnRecoveredPacket(packet, length);
 }
 
 void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet) {
+  auto it = receive_rtp_config_.find(packet.Ssrc());
+  bool transport_cc =
+      (it != receive_rtp_config_.end()) && it->second.transport_cc;
+
   RTPHeader header;
   packet.GetHeader(&header);
+
+  if (transport_cc != header.extension.hasTransportSequenceNumber) {

[nisse-webrtc, 2017/01/27 13:26:14]

I intended to use the check
 
  if (transport_cc && !header.extension.hasTransportSequenceNumber)

but then tests fail. Additional logging shows that we get

  transport_cc = 0, hasTransportSequenceNumber = 1

for the
TestWithNewVideoJitterBuffer/EndToEndTest.*TransportFeedbackNotConfigured/*
tests.

Can that reasonably happen? I though that transport_cc being false meant the RTP
extension wasn't negotiated, hence shouldn't be included in
RtpHeaderExtensionMap, and then the RTP parser should never set
hasTransportSequenceNumber true, no matter what bits arrive on the wire. Am I
missing something, or is there a bug somewhere?

+    // Send side BWE was negotiated, but required header extension not
+    // present on this packet. Then don't use it for bandwidth
+    // estimation at all.
+    LOG(LS_ERROR) << "Not calling OnReceivedPacket";
+    return;
+  }
   congestion_controller_->OnReceivedPacket(packet.arrival_time_ms(),
                                            packet.payload_size(), header);
 }
 
 }  // namespace internal
 }  // namespace webrtc