From 60fbd65482b58be2324901d7b8031a418ed22df6 Mon Sep 17 00:00:00 2001 From: "minyue@webrtc.org" Date: Thu, 25 Sep 2014 14:36:30 +0000 Subject: [PATCH] Removing error triggered for disabling FEC on non-opus A failure was triggered when one sets FEC status on a codec that does not support FEC. While it is definitely logical when one wants to enable it, it makes no good sense if one tries to disable it. BUG= R=tina.legrand@webrtc.org, xians@webrtc.org Review URL: https://webrtc-codereview.appspot.com/24729004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7298 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../audio_coding/main/acm2/acm_generic_codec.cc | 6 ++++++ .../audio_coding/main/acm2/acm_generic_codec.h | 4 ++-- webrtc/modules/audio_coding/main/test/TestRedFec.cc | 13 ++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc index 29dccd2f6e..84c7a9f91c 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc @@ -196,6 +196,12 @@ bool ACMGenericCodec::HasFrameToEncode() const { return true; } +int ACMGenericCodec::SetFEC(bool enable_fec) { + if (!HasInternalFEC() && enable_fec) + return -1; + return 0; +} + int16_t ACMGenericCodec::Encode(uint8_t* bitstream, int16_t* bitstream_len_byte, uint32_t* timestamp, diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h index ad6f412a5f..2ed62e9e79 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h +++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h @@ -580,10 +580,10 @@ class ACMGenericCodec { // disabled. // // Return value: - // -1 if failed, or the codec does not support FEC + // -1 if failed, // 0 if succeeded. // - virtual int SetFEC(bool /* enable_fec */) { return -1; } + virtual int SetFEC(bool enable_fec); /////////////////////////////////////////////////////////////////////////// // int SetPacketLossRate() diff --git a/webrtc/modules/audio_coding/main/test/TestRedFec.cc b/webrtc/modules/audio_coding/main/test/TestRedFec.cc index 10498d8c57..b69a1ed026 100644 --- a/webrtc/modules/audio_coding/main/test/TestRedFec.cc +++ b/webrtc/modules/audio_coding/main/test/TestRedFec.cc @@ -245,7 +245,7 @@ void TestRedFec::Perform() { EXPECT_EQ(0, _acmA->SetCodecFEC(true)); _outFileB.Close(); - // Codecs does not support internal FEC. + // Codecs does not support internal FEC, cannot enable FEC. RegisterSendCodec('A', nameG722, 16000); EXPECT_FALSE(_acmA->REDStatus()); EXPECT_EQ(-1, _acmA->SetCodecFEC(true)); @@ -255,6 +255,17 @@ void TestRedFec::Perform() { EXPECT_FALSE(_acmA->REDStatus()); EXPECT_EQ(-1, _acmA->SetCodecFEC(true)); EXPECT_FALSE(_acmA->CodecFEC()); + + // Codecs does not support internal FEC, disable FEC does not trigger failure. + RegisterSendCodec('A', nameG722, 16000); + EXPECT_FALSE(_acmA->REDStatus()); + EXPECT_EQ(0, _acmA->SetCodecFEC(false)); + EXPECT_FALSE(_acmA->CodecFEC()); + + RegisterSendCodec('A', nameISAC, 16000); + EXPECT_FALSE(_acmA->REDStatus()); + EXPECT_EQ(0, _acmA->SetCodecFEC(false)); + EXPECT_FALSE(_acmA->CodecFEC()); } int32_t TestRedFec::SetVAD(bool enableDTX, bool enableVAD, ACMVADMode vadMode) {