zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

CodecOwner::SetEncoders: Return error code when given bad arguments

Browse Source 
Instead of FATAL on a bad codec specification, log and return an error
code. This is a band-aid until callers are taught to only give it good
specifications.

BUG=webrtc:5033, chromium:526478

Review URL: https://codereview.webrtc.org/1364193002

Cr-Commit-Position: refs/heads/master@{#10066}
This commit is contained in:
kwiberg
2015-09-24 22:53:55 -07:00
committed by Commit bot
parent 6979b024d7
commit 574d5daa6d
4 changed files with 33 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
webrtc/modules/audio_coding/main/acm2/codec_manager.cc
View File

@ -281,9 +281,10 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
// VAD/DTX not supported.
dtx_enabled_ = false;
}
codec_owner_.SetEncoders(
send_codec, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1);
if (!codec_owner_.SetEncoders(
send_codec, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1))
return -1;
RTC_DCHECK(codec_owner_.Encoder());
codec_fec_enabled_ = codec_fec_enabled_ &&
@ -297,9 +298,10 @@ int CodecManager::RegisterEncoder(const CodecInst& send_codec) {
if (send_codec_inst_.plfreq != send_codec.plfreq ||
send_codec_inst_.pacsize != send_codec.pacsize ||
send_codec_inst_.channels != send_codec.channels) {
codec_owner_.SetEncoders(
send_codec, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1);
if (!codec_owner_.SetEncoders(
send_codec, dtx_enabled_ ? CngPayloadType(send_codec.plfreq) : -1,
vad_mode_, red_enabled_ ? RedPayloadType(send_codec.plfreq) : -1))
return -1;
RTC_DCHECK(codec_owner_.Encoder());
}
send_codec_inst_.plfreq = send_codec.plfreq;

11
webrtc/modules/audio_coding/main/acm2/codec_owner.cc
View File

@ -11,6 +11,7 @@
#include "webrtc/modules/audio_coding/main/acm2/codec_owner.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
#include "webrtc/engine_configurations.h"
#include "webrtc/modules/audio_coding/codecs/cng/include/audio_encoder_cng.h"
#include "webrtc/modules/audio_coding/codecs/g711/include/audio_encoder_pcm.h"
@ -117,6 +118,8 @@ rtc::scoped_ptr<AudioEncoder> CreateIsacEncoder(
#endif
}
// Returns a new speech encoder, or null on error.
// TODO(kwiberg): Don't handle errors here (bug 5033)
rtc::scoped_ptr<AudioEncoder> CreateSpeechEncoder(
const CodecInst& speech_inst,
LockedIsacBandwidthInfo* bwinfo) {
@ -135,7 +138,8 @@ rtc::scoped_ptr<AudioEncoder> CreateSpeechEncoder(
} else if (IsG722(speech_inst)) {
return rtc_make_scoped_ptr(new AudioEncoderG722(speech_inst));
} else {
FATAL() << "Could not create encoder of type " << speech_inst.plname;
LOG_F(LS_ERROR) << "Could not create encoder of type "
<< speech_inst.plname;
return rtc::scoped_ptr<AudioEncoder>();
}
}
@ -189,13 +193,16 @@ void CreateCngEncoder(int cng_payload_type,
}
} // namespace
void CodecOwner::SetEncoders(const CodecInst& speech_inst,
bool CodecOwner::SetEncoders(const CodecInst& speech_inst,
int cng_payload_type,
ACMVADMode vad_mode,
int red_payload_type) {
speech_encoder_ = CreateSpeechEncoder(speech_inst, &isac_bandwidth_info_);
if (!speech_encoder_)
return false;
external_speech_encoder_ = nullptr;
ChangeCngAndRed(cng_payload_type, vad_mode, red_payload_type);
return true;
}
void CodecOwner::SetEncoders(AudioEncoder* external_speech_encoder,

6
webrtc/modules/audio_coding/main/acm2/codec_owner.h
View File

@ -29,10 +29,12 @@ class CodecOwner {
CodecOwner();
~CodecOwner();
void SetEncoders(const CodecInst& speech_inst,
// Start using the specified encoder. Returns false on error.
// TODO(kwiberg): Don't handle errors here (bug 5033)
bool SetEncoders(const CodecInst& speech_inst,
int cng_payload_type,
ACMVADMode vad_mode,
int red_payload_type);
int red_payload_type) WARN_UNUSED_RESULT;
void SetEncoders(AudioEncoder* external_speech_encoder,
int cng_payload_type,

14
webrtc/modules/audio_coding/main/acm2/codec_owner_unittest.cc
View File

@ -8,6 +8,8 @@
* be found in the AUTHORS file in the root of the source tree.
*/
#include <cstring>
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/base/arraysize.h"
#include "webrtc/base/safe_conversions.h"
@ -34,7 +36,8 @@ class CodecOwnerTest : public ::testing::Test {
CodecOwnerTest() : timestamp_(0) {}
void CreateCodec() {
codec_owner_.SetEncoders(kDefaultCodecInst, kCngPt, VADNormal, -1);
ASSERT_TRUE(
codec_owner_.SetEncoders(kDefaultCodecInst, kCngPt, VADNormal, -1));
}
void EncodeAndVerify(size_t expected_out_length,
@ -167,7 +170,7 @@ TEST_F(CodecOwnerTest, ExternalEncoder) {
// Change to internal encoder.
CodecInst codec_inst = kDefaultCodecInst;
codec_inst.pacsize = kPacketSizeSamples;
codec_owner_.SetEncoders(codec_inst, -1, VADNormal, -1);
ASSERT_TRUE(codec_owner_.SetEncoders(codec_inst, -1, VADNormal, -1));
// Don't expect any more calls to the external encoder.
info = codec_owner_.Encoder()->Encode(1, audio, arraysize(audio),
arraysize(encoded), encoded);
@ -196,5 +199,12 @@ TEST_F(CodecOwnerTest, NoCngAndRedNoSpeechEncoderReset) {
TestCngAndRedResetSpeechEncoder(false, false);
}
TEST_F(CodecOwnerTest, SetEncodersError) {
CodecInst codec_inst = kDefaultCodecInst;
static const char bad_name[] = "Robert'); DROP TABLE Students;";
std::memcpy(codec_inst.plname, bad_name, sizeof bad_name);
EXPECT_FALSE(codec_owner_.SetEncoders(codec_inst, -1, VADNormal, -1));
}
} // namespace acm2
} // namespace webrtc