From fde2b2428108cfeec0dae1dac5645e7101c5d0d4 Mon Sep 17 00:00:00 2001
From: Jakob Ivarsson <jakobi@webrtc.org>
Date: Thu, 20 Aug 2020 16:48:49 +0200
Subject: [PATCH] Reland "Call OnReceivedOverhead after audio network adaptor
 is created."
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Potential deadlock fixed by acquiring lock before calling encoder.

This is a reland of a135557b3c7ffa4fb1710d2d907c3cb86c5d5913

Original change's description:
> Call OnReceivedOverhead after audio network adaptor is created.
>
> This prevents ending up in a state where audio network adaptor never
> receives the current packet overhead and therefore doesn't work.
>
> Bug: chromium:1086942
> Change-Id: I8ee2ffbb7741b342b3ec93fc89f2859a146f4ba7
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181583
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Per Åhgren <peah@webrtc.org>
> Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31951}

Bug: chromium:1086942
Change-Id: I514e523c6607cee0099b87919f0f77ebec966ddd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181888
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Minyue Li <minyue@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31971}
---
 audio/audio_send_stream.cc          |  8 ++++++
 audio/audio_send_stream_unittest.cc | 43 +++++++++++++++++++++++++++++
 test/mock_audio_encoder.h           |  4 +++
 3 files changed, 55 insertions(+)

diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 24f8ce908e..3fcdbf432e 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -734,11 +734,19 @@ void AudioSendStream::ReconfigureANA(const Config& new_config) {
     return;
   }
   if (new_config.audio_network_adaptor_config) {
+    // This lock needs to be acquired before CallEncoder, since it aquires
+    // another lock and we need to maintain the same order at all call sites to
+    // avoid deadlock.
+    MutexLock lock(&overhead_per_packet_lock_);
+    size_t overhead = GetPerPacketOverheadBytes();
     channel_send_->CallEncoder([&](AudioEncoder* encoder) {
       if (encoder->EnableAudioNetworkAdaptor(
               *new_config.audio_network_adaptor_config, event_log_)) {
         RTC_LOG(LS_INFO) << "Audio network adaptor enabled on SSRC "
                          << new_config.rtp.ssrc;
+        if (overhead > 0) {
+          encoder->OnReceivedOverhead(overhead);
+        }
       } else {
         RTC_LOG(LS_INFO) << "Failed to enable Audio network adaptor on SSRC "
                          << new_config.rtp.ssrc;
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index 481405c297..bfec59bf92 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -45,6 +45,7 @@ using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::Eq;
 using ::testing::Field;
+using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::Ne;
 using ::testing::Return;
@@ -556,6 +557,48 @@ TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) {
   }
 }
 
+TEST(AudioSendStreamTest, AudioNetworkAdaptorReceivesOverhead) {
+  for (bool use_null_audio_processing : {false, true}) {
+    ConfigHelper helper(false, true, use_null_audio_processing);
+    helper.config().send_codec_spec =
+        AudioSendStream::Config::SendCodecSpec(0, kOpusFormat);
+    const std::string kAnaConfigString = "abcde";
+    helper.config().rtp.extensions.push_back(RtpExtension(
+        RtpExtension::kTransportSequenceNumberUri, kTransportSequenceNumberId));
+
+    EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _))
+        .WillOnce(Invoke(
+            [&kAnaConfigString](int payload_type, const SdpAudioFormat& format,
+                                absl::optional<AudioCodecPairId> codec_pair_id,
+                                std::unique_ptr<AudioEncoder>* return_value) {
+              auto mock_encoder = SetupAudioEncoderMock(payload_type, format);
+              InSequence s;
+              EXPECT_CALL(
+                  *mock_encoder,
+                  OnReceivedOverhead(Eq(kOverheadPerPacket.bytes<size_t>())))
+                  .Times(2);
+              EXPECT_CALL(*mock_encoder,
+                          EnableAudioNetworkAdaptor(StrEq(kAnaConfigString), _))
+                  .WillOnce(Return(true));
+              // Note: Overhead is received AFTER ANA has been enabled.
+              EXPECT_CALL(
+                  *mock_encoder,
+                  OnReceivedOverhead(Eq(kOverheadPerPacket.bytes<size_t>())))
+                  .WillOnce(Return());
+              *return_value = std::move(mock_encoder);
+            }));
+    EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead)
+        .WillRepeatedly(Return(kOverheadPerPacket.bytes<size_t>()));
+
+    auto send_stream = helper.CreateAudioSendStream();
+
+    auto stream_config = helper.config();
+    stream_config.audio_network_adaptor_config = kAnaConfigString;
+
+    send_stream->Reconfigure(stream_config);
+  }
+}
+
 // VAD is applied when codec is mono and the CNG frequency matches the codec
 // clock rate.
 TEST(AudioSendStreamTest, SendCodecCanApplyVad) {
diff --git a/test/mock_audio_encoder.h b/test/mock_audio_encoder.h
index eeb63f1062..87b8cc8c8e 100644
--- a/test/mock_audio_encoder.h
+++ b/test/mock_audio_encoder.h
@@ -48,6 +48,10 @@ class MockAudioEncoder : public AudioEncoder {
               OnReceivedUplinkPacketLossFraction,
               (float uplink_packet_loss_fraction),
               (override));
+  MOCK_METHOD(void,
+              OnReceivedOverhead,
+              (size_t overhead_bytes_per_packet),
+              (override));
 
   MOCK_METHOD(bool,
               EnableAudioNetworkAdaptor,