Reland "Call OnReceivedOverhead after audio network adaptor is created."

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}
This commit is contained in:
Jakob Ivarsson 2020-08-20 16:48:49 +02:00 committed by Commit Bot
parent 6a9a910fd2
commit fde2b24281
3 changed files with 55 additions and 0 deletions

View File

@ -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;

View File

@ -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) {

View File

@ -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,