From caea68f31e33a1bdf8adcdacaa53bc2f3c29ab88 Mon Sep 17 00:00:00 2001
From: brandtr <brandtr@webrtc.org>
Date: Wed, 23 Aug 2017 00:55:17 -0700
Subject: [PATCH] Let Call::OnRecoveredPacket parse RTP header extensions.

Packets recovered by ULPFEC enter through
RtpVideoStreamReceiver::OnRecoveredPacket, which does RTP
header extension parsing. Packets recovered by FlexFEC, however,
enter through Call::OnRecoveredPacket, which prior to this
CL did not do RTP header extension parsing.

The lack of RTP header extension parsing for FlexFEC packets is a
regression since https://codereview.webrtc.org/2886993005/.

TESTED=Using Android app with FlexFEC field trial enabled.
BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/3002023002
Cr-Commit-Position: refs/heads/master@{#19460}
---
 webrtc/call/call.cc              | 19 ++++++++++++++++---
 webrtc/video/end_to_end_tests.cc | 13 +++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index 16ee1cde78..f2bf27efa7 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -1382,10 +1382,7 @@ PacketReceiver::DeliveryStatus Call::DeliverPacket(
   return DeliverRtp(media_type, packet, length, packet_time);
 }
 
-// TODO(brandtr): Update this member function when we support protecting
-// audio packets with FlexFEC.
 void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) {
-  ReadLockScoped read_lock(*receive_crit_);
   rtc::Optional<RtpPacketReceived> parsed_packet =
       ParseRtpPacket(packet, length, nullptr);
   if (!parsed_packet)
@@ -1393,6 +1390,22 @@ void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) {
 
   parsed_packet->set_recovered(true);
 
+  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.
+    return;
+  }
+  parsed_packet->IdentifyExtensions(it->second.extensions);
+
+  // TODO(brandtr): Update here when we support protecting audio packets too.
   video_receiver_controller_.OnRtpPacket(*parsed_packet);
 }
 
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index 91a544c506..a4d7033603 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -831,6 +831,8 @@ class FlexfecRenderObserver : public test::EndToEndTest,
   }
 
   void OnFrame(const VideoFrame& video_frame) override {
+    EXPECT_EQ(kVideoRotation_90, video_frame.rotation());
+
     rtc::CritScope lock(&crit_);
     // Rendering frame with timestamp of packet that was dropped -> FEC
     // protection worked.
@@ -868,6 +870,11 @@ class FlexfecRenderObserver : public test::EndToEndTest,
     }
   }
 
+  void OnFrameGeneratorCapturerCreated(
+      test::FrameGeneratorCapturer* frame_generator_capturer) override {
+    frame_generator_capturer->SetFakeRotation(kVideoRotation_90);
+  }
+
   void ModifyFlexfecConfigs(
       std::vector<FlexfecReceiveStream::Config>* receive_configs) override {
     (*receive_configs)[0].local_ssrc = kFlexfecLocalSsrc;
@@ -1133,6 +1140,7 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) {
     }
 
     void OnFrame(const VideoFrame& frame) override {
+      EXPECT_EQ(kVideoRotation_90, frame.rotation());
       {
         rtc::CritScope lock(&crit_);
         if (frame.timestamp() == retransmitted_timestamp_)
@@ -1185,6 +1193,11 @@ void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) {
       (*receive_configs)[0].decoders[0].payload_name = "VP8";
     }
 
+    void OnFrameGeneratorCapturerCreated(
+        test::FrameGeneratorCapturer* frame_generator_capturer) override {
+      frame_generator_capturer->SetFakeRotation(kVideoRotation_90);
+    }
+
     void PerformTest() override {
       EXPECT_TRUE(Wait())
           << "Timed out while waiting for retransmission to render.";