From 4dc941128f89d8997264d1b0d22262dc283de468 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Wed, 11 Nov 2015 08:34:21 -0800 Subject: [PATCH] CodecManager::RegisterEncoder: Call SetFec on new encoder, not old BUG=webrtc:5028 Review URL: https://codereview.webrtc.org/1416633011 Cr-Commit-Position: refs/heads/master@{#10604} --- .../audio_coding_module_unittest_oldapi.cc | 3 + .../audio_coding/main/acm2/codec_manager.cc | 11 +++- .../main/acm2/codec_manager_unittest.cc | 66 +++++++++++++++++++ webrtc/modules/modules.gyp | 1 + 4 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 webrtc/modules/audio_coding/main/acm2/codec_manager_unittest.cc diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc index cfceb0df83..3fac6583f8 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc @@ -1647,6 +1647,9 @@ TEST_F(AcmSenderBitExactnessOldApi, External_Pcmu_20ms) { EXPECT_CALL(mock_encoder, EncodeInternal(_, _, _, _)) .Times(AtLeast(1)) .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::EncodeInternal)); + EXPECT_CALL(mock_encoder, SetFec(_)) + .Times(AtLeast(1)) + .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::SetFec)); ASSERT_NO_FATAL_FAILURE( SetUpTestExternalEncoder(&mock_encoder, codec_inst.pltype)); Run("81a9d4c0bb72e9becc43aef124c981e9", "8f9b8750bd80fe26b6cbf6659b89f0f9", diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc index e4070c76ae..ac21020dc2 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc @@ -319,8 +319,15 @@ void CodecManager::RegisterEncoder(AudioEncoder* external_speech_encoder) { if (send_codec_inst_.channels != 1) dtx_enabled_ = false; - codec_fec_enabled_ = - codec_fec_enabled_ && codec_owner_.Encoder()->SetFec(codec_fec_enabled_); + if (codec_fec_enabled_) { + // Switch FEC on. On failure, remember that FEC is off. + if (!external_speech_encoder->SetFec(true)) + codec_fec_enabled_ = false; + } else { + // Switch FEC off. This shouldn't fail. + const bool success = external_speech_encoder->SetFec(false); + RTC_DCHECK(success); + } int cng_pt = dtx_enabled_ ? CngPayloadType(external_speech_encoder->SampleRateHz()) : -1; diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager_unittest.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager_unittest.cc new file mode 100644 index 0000000000..e930ca1ea5 --- /dev/null +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager_unittest.cc @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2015 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. + */ + +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_encoder.h" +#include "webrtc/modules/audio_coding/main/acm2/codec_manager.h" + +namespace webrtc { +namespace acm2 { + +using ::testing::Return; + +namespace { + +// Create a MockAudioEncoder with some reasonable default behavior. +rtc::scoped_ptr CreateMockEncoder() { + auto enc = rtc_make_scoped_ptr(new MockAudioEncoder); + EXPECT_CALL(*enc, SampleRateHz()).WillRepeatedly(Return(8000)); + EXPECT_CALL(*enc, NumChannels()).WillRepeatedly(Return(1)); + EXPECT_CALL(*enc, Max10MsFramesInAPacket()).WillRepeatedly(Return(1)); + EXPECT_CALL(*enc, Die()); + return enc; +} + +} // namespace + +TEST(CodecManagerTest, ExternalEncoderFec) { + auto enc0 = CreateMockEncoder(); + auto enc1 = CreateMockEncoder(); + { + ::testing::InSequence s; + EXPECT_CALL(*enc0, SetFec(false)).WillOnce(Return(true)); + EXPECT_CALL(*enc0, Mark("A")); + EXPECT_CALL(*enc0, SetFec(true)).WillOnce(Return(true)); + EXPECT_CALL(*enc1, SetFec(true)).WillOnce(Return(true)); + EXPECT_CALL(*enc1, SetFec(false)).WillOnce(Return(true)); + EXPECT_CALL(*enc0, Mark("B")); + EXPECT_CALL(*enc0, SetFec(false)).WillOnce(Return(true)); + } + + CodecManager cm; + EXPECT_FALSE(cm.codec_fec_enabled()); + cm.RegisterEncoder(enc0.get()); + EXPECT_FALSE(cm.codec_fec_enabled()); + enc0->Mark("A"); + EXPECT_EQ(0, cm.SetCodecFEC(true)); + EXPECT_TRUE(cm.codec_fec_enabled()); + cm.RegisterEncoder(enc1.get()); + EXPECT_TRUE(cm.codec_fec_enabled()); + + EXPECT_EQ(0, cm.SetCodecFEC(false)); + enc0->Mark("B"); + EXPECT_FALSE(cm.codec_fec_enabled()); + cm.RegisterEncoder(enc0.get()); + EXPECT_FALSE(cm.codec_fec_enabled()); +} + +} // namespace acm2 +} // namespace webrtc diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index a32271afb2..9795f44f48 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -101,6 +101,7 @@ 'audio_coding/main/acm2/acm_receiver_unittest_oldapi.cc', 'audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc', 'audio_coding/main/acm2/call_statistics_unittest.cc', + 'audio_coding/main/acm2/codec_manager_unittest.cc', 'audio_coding/main/acm2/codec_owner_unittest.cc', 'audio_coding/main/acm2/initial_delay_manager_unittest.cc', 'audio_coding/main/acm2/rent_a_codec_unittest.cc',