Removed virtual from several methods in DecoderDatabase to minimize
the number of points that need to be mocked for testing.
For the now non-virtual methods, DecoderDatabase now does a lookup
through GetDecoderInfo and then delegates to the appropriate method in
the DecoderInfo object, if one is found.
A few other methods were also changed to look up through GetDecoderInfo.
Also moved the audio decoder factory into DecoderInfo, so that
DecoderInfo::GetDecoder can be used directly.
Review-Url: https://codereview.webrtc.org/2276913002
Cr-Commit-Position: refs/heads/master@{#13933}
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc
index 165522f..b189e4b 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.cc
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc
@@ -10,7 +10,6 @@
#include "webrtc/modules/audio_coding/neteq/decoder_database.h"
-#include <assert.h>
#include <utility> // pair
#include "webrtc/base/checks.h"
@@ -27,11 +26,14 @@
DecoderDatabase::~DecoderDatabase() = default;
-DecoderDatabase::DecoderInfo::DecoderInfo(NetEqDecoder ct,
- const std::string& nm)
+DecoderDatabase::DecoderInfo::DecoderInfo(
+ NetEqDecoder ct,
+ const std::string& nm,
+ AudioDecoderFactory* factory)
: codec_type(ct),
name(nm),
audio_format_(acm2::RentACodec::NetEqDecoderToSdpAudioFormat(ct)),
+ factory_(factory),
external_decoder_(nullptr),
cng_decoder_(CngDecoder::Create(ct)) {}
@@ -48,21 +50,39 @@
DecoderDatabase::DecoderInfo::DecoderInfo(DecoderInfo&&) = default;
DecoderDatabase::DecoderInfo::~DecoderInfo() = default;
-AudioDecoder* DecoderDatabase::DecoderInfo::GetDecoder(
- AudioDecoderFactory* factory) {
+AudioDecoder* DecoderDatabase::DecoderInfo::GetDecoder() const {
if (external_decoder_) {
RTC_DCHECK(!decoder_);
RTC_DCHECK(!cng_decoder_);
return external_decoder_;
}
+ if (IsRed() || IsComfortNoise() || IsDtmf())
+ return nullptr;
RTC_DCHECK(audio_format_);
if (!decoder_) {
- decoder_ = factory->MakeAudioDecoder(*audio_format_);
+ RTC_DCHECK(factory_);
+ decoder_ = factory_->MakeAudioDecoder(*audio_format_);
}
RTC_DCHECK(decoder_) << "Failed to create: " << *audio_format_;
return decoder_.get();
}
+
+bool DecoderDatabase::DecoderInfo::IsComfortNoise() const {
+ return codec_type == NetEqDecoder::kDecoderCNGnb
+ || codec_type == NetEqDecoder::kDecoderCNGwb
+ || codec_type == NetEqDecoder::kDecoderCNGswb32kHz
+ || codec_type == NetEqDecoder::kDecoderCNGswb48kHz;
+}
+
+bool DecoderDatabase::DecoderInfo::IsDtmf() const {
+ return codec_type == NetEqDecoder::kDecoderAVT;
+}
+
+bool DecoderDatabase::DecoderInfo::IsRed() const {
+ return codec_type == NetEqDecoder::kDecoderRED;
+}
+
rtc::Optional<DecoderDatabase::DecoderInfo::CngDecoder>
DecoderDatabase::DecoderInfo::CngDecoder::Create(NetEqDecoder ct) {
const auto cng = [](int sample_rate_hz) {
@@ -102,7 +122,7 @@
if (!CodecSupported(codec_type)) {
return kCodecNotSupported;
}
- DecoderInfo info(codec_type, name);
+ DecoderInfo info(codec_type, name, decoder_factory_.get());
auto ret =
decoders_.insert(std::make_pair(rtp_payload_type, std::move(info)));
if (ret.second == false) {
@@ -172,82 +192,32 @@
return kRtpPayloadTypeError;
}
-AudioDecoder* DecoderDatabase::GetDecoder(uint8_t rtp_payload_type) {
- if (IsDtmf(rtp_payload_type) || IsRed(rtp_payload_type) ||
- IsComfortNoise(rtp_payload_type)) {
- // These are not real decoders.
- return NULL;
- }
- DecoderMap::iterator it = decoders_.find(rtp_payload_type);
- if (it == decoders_.end()) {
- // Decoder not found.
- return NULL;
- }
- DecoderInfo* info = &(*it).second;
- return info->GetDecoder(decoder_factory_.get());
-}
-
-bool DecoderDatabase::IsType(uint8_t rtp_payload_type,
- NetEqDecoder codec_type) const {
- DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
- if (it == decoders_.end()) {
- // Decoder not found.
- return false;
- }
- return ((*it).second.codec_type == codec_type);
-}
-
-bool DecoderDatabase::IsComfortNoise(uint8_t rtp_payload_type) const {
- DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
- if (it == decoders_.end()) {
- // Decoder not found.
- return false;
- }
- const auto& type = it->second.codec_type;
- return type == NetEqDecoder::kDecoderCNGnb
- || type == NetEqDecoder::kDecoderCNGwb
- || type == NetEqDecoder::kDecoderCNGswb32kHz
- || type == NetEqDecoder::kDecoderCNGswb48kHz;
-}
-
-bool DecoderDatabase::IsDtmf(uint8_t rtp_payload_type) const {
- return IsType(rtp_payload_type, NetEqDecoder::kDecoderAVT);
-}
-
-bool DecoderDatabase::IsRed(uint8_t rtp_payload_type) const {
- return IsType(rtp_payload_type, NetEqDecoder::kDecoderRED);
-}
-
int DecoderDatabase::SetActiveDecoder(uint8_t rtp_payload_type,
bool* new_decoder) {
// Check that |rtp_payload_type| exists in the database.
- DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
- if (it == decoders_.end()) {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ if (!info) {
// Decoder not found.
return kDecoderNotFound;
}
- RTC_CHECK(!IsComfortNoise(rtp_payload_type));
- assert(new_decoder);
+ RTC_CHECK(!info->IsComfortNoise());
+ RTC_DCHECK(new_decoder);
*new_decoder = false;
if (active_decoder_type_ < 0) {
// This is the first active decoder.
*new_decoder = true;
} else if (active_decoder_type_ != rtp_payload_type) {
// Moving from one active decoder to another. Delete the first one.
- DecoderMap::iterator it = decoders_.find(active_decoder_type_);
- if (it == decoders_.end()) {
- // Decoder not found. This should not be possible.
- assert(false);
- return kDecoderNotFound;
- }
- it->second.DropDecoder();
+ const DecoderInfo *old_info = GetDecoderInfo(active_decoder_type_);
+ RTC_DCHECK(old_info);
+ old_info->DropDecoder();
*new_decoder = true;
}
active_decoder_type_ = rtp_payload_type;
return kOK;
}
-AudioDecoder* DecoderDatabase::GetActiveDecoder() {
+AudioDecoder* DecoderDatabase::GetActiveDecoder() const {
if (active_decoder_type_ < 0) {
// No active decoder.
return NULL;
@@ -257,27 +227,22 @@
int DecoderDatabase::SetActiveCngDecoder(uint8_t rtp_payload_type) {
// Check that |rtp_payload_type| exists in the database.
- DecoderMap::const_iterator it = decoders_.find(rtp_payload_type);
- if (it == decoders_.end()) {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ if (!info) {
// Decoder not found.
return kDecoderNotFound;
}
if (active_cng_decoder_type_ >= 0 &&
active_cng_decoder_type_ != rtp_payload_type) {
// Moving from one active CNG decoder to another. Delete the first one.
- DecoderMap::iterator it = decoders_.find(active_cng_decoder_type_);
- if (it == decoders_.end()) {
- // Decoder not found. This should not be possible.
- assert(false);
- return kDecoderNotFound;
- }
+ RTC_DCHECK(active_cng_decoder_);
active_cng_decoder_.reset();
}
active_cng_decoder_type_ = rtp_payload_type;
return kOK;
}
-ComfortNoiseDecoder* DecoderDatabase::GetActiveCngDecoder() {
+ComfortNoiseDecoder* DecoderDatabase::GetActiveCngDecoder() const {
if (active_cng_decoder_type_ < 0) {
// No active CNG decoder.
return NULL;
@@ -288,10 +253,36 @@
return active_cng_decoder_.get();
}
+AudioDecoder* DecoderDatabase::GetDecoder(uint8_t rtp_payload_type) const {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ return info ? info->GetDecoder() : nullptr;
+}
+
+bool DecoderDatabase::IsType(uint8_t rtp_payload_type,
+ NetEqDecoder codec_type) const {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ return info && info->codec_type == codec_type;
+}
+
+bool DecoderDatabase::IsComfortNoise(uint8_t rtp_payload_type) const {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ return info && info->IsComfortNoise();
+}
+
+bool DecoderDatabase::IsDtmf(uint8_t rtp_payload_type) const {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ return info && info->IsDtmf();
+}
+
+bool DecoderDatabase::IsRed(uint8_t rtp_payload_type) const {
+ const DecoderInfo *info = GetDecoderInfo(rtp_payload_type);
+ return info && info->IsRed();
+}
+
int DecoderDatabase::CheckPayloadTypes(const PacketList& packet_list) const {
PacketList::const_iterator it;
for (it = packet_list.begin(); it != packet_list.end(); ++it) {
- if (decoders_.find((*it)->header.payloadType) == decoders_.end()) {
+ if (!GetDecoderInfo((*it)->header.payloadType)) {
// Payload type is not found.
LOG(LS_WARNING) << "CheckPayloadTypes: unknown RTP payload type "
<< static_cast<int>((*it)->header.payloadType);
diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.h b/webrtc/modules/audio_coding/neteq/decoder_database.h
index 404f257..1454874 100644
--- a/webrtc/modules/audio_coding/neteq/decoder_database.h
+++ b/webrtc/modules/audio_coding/neteq/decoder_database.h
@@ -41,7 +41,10 @@
// Class that stores decoder info in the database.
class DecoderInfo {
public:
- DecoderInfo(NetEqDecoder ct, const std::string& nm);
+ DecoderInfo(
+ NetEqDecoder ct,
+ const std::string& nm,
+ AudioDecoderFactory* factory = nullptr);
DecoderInfo(NetEqDecoder ct,
const std::string& nm,
AudioDecoder* ext_dec);
@@ -49,11 +52,11 @@
~DecoderInfo();
// Get the AudioDecoder object, creating it first if necessary.
- AudioDecoder* GetDecoder(AudioDecoderFactory* factory);
+ AudioDecoder* GetDecoder() const;
// Delete the AudioDecoder object, unless it's external. (This means we can
// always recreate it later if we need it.)
- void DropDecoder() { decoder_.reset(); }
+ void DropDecoder() const { decoder_.reset(); }
int SampleRateHz() const {
RTC_DCHECK_EQ(1, !!decoder_ + !!external_decoder_ + !!cng_decoder_);
@@ -62,12 +65,22 @@
: cng_decoder_->sample_rate_hz;
}
+ // Returns true if |codec_type| is comfort noise.
+ bool IsComfortNoise() const;
+
+ // Returns true if |codec_type| is DTMF.
+ bool IsDtmf() const;
+
+ // Returns true if |codec_type| is RED.
+ bool IsRed() const;
+
const NetEqDecoder codec_type;
const std::string name;
private:
const rtc::Optional<SdpAudioFormat> audio_format_;
- std::unique_ptr<AudioDecoder> decoder_;
+ AudioDecoderFactory* factory_;
+ mutable std::unique_ptr<AudioDecoder> decoder_;
// Set iff this is an external decoder.
AudioDecoder* const external_decoder_;
@@ -129,31 +142,13 @@
// method may return any of them.
virtual uint8_t GetRtpPayloadType(NetEqDecoder codec_type) const;
- // Returns a pointer to the AudioDecoder object associated with
- // |rtp_payload_type|, or NULL if none is registered. If the AudioDecoder
- // object does not exist for that decoder, the object is created.
- virtual AudioDecoder* GetDecoder(uint8_t rtp_payload_type);
-
- // Returns true if |rtp_payload_type| is registered as a |codec_type|.
- virtual bool IsType(uint8_t rtp_payload_type,
- NetEqDecoder codec_type) const;
-
- // Returns true if |rtp_payload_type| is registered as comfort noise.
- virtual bool IsComfortNoise(uint8_t rtp_payload_type) const;
-
- // Returns true if |rtp_payload_type| is registered as DTMF.
- virtual bool IsDtmf(uint8_t rtp_payload_type) const;
-
- // Returns true if |rtp_payload_type| is registered as RED.
- virtual bool IsRed(uint8_t rtp_payload_type) const;
-
// Sets the active decoder to be |rtp_payload_type|. If this call results in a
// change of active decoder, |new_decoder| is set to true. The previous active
// decoder's AudioDecoder object is deleted.
virtual int SetActiveDecoder(uint8_t rtp_payload_type, bool* new_decoder);
// Returns the current active decoder, or NULL if no active decoder exists.
- virtual AudioDecoder* GetActiveDecoder();
+ virtual AudioDecoder* GetActiveDecoder() const;
// Sets the active comfort noise decoder to be |rtp_payload_type|. If this
// call results in a change of active comfort noise decoder, the previous
@@ -162,11 +157,32 @@
// Returns the current active comfort noise decoder, or NULL if no active
// comfort noise decoder exists.
- virtual ComfortNoiseDecoder* GetActiveCngDecoder();
+ virtual ComfortNoiseDecoder* GetActiveCngDecoder() const;
+
+ // The following are utility methods: they will look up DecoderInfo through
+ // GetDecoderInfo and call the respective method on that info object, if it
+ // exists.
+
+ // Returns a pointer to the AudioDecoder object associated with
+ // |rtp_payload_type|, or NULL if none is registered. If the AudioDecoder
+ // object does not exist for that decoder, the object is created.
+ AudioDecoder* GetDecoder(uint8_t rtp_payload_type) const;
+
+ // Returns true if |rtp_payload_type| is registered as a |codec_type|.
+ bool IsType(uint8_t rtp_payload_type, NetEqDecoder codec_type) const;
+
+ // Returns true if |rtp_payload_type| is registered as comfort noise.
+ bool IsComfortNoise(uint8_t rtp_payload_type) const;
+
+ // Returns true if |rtp_payload_type| is registered as DTMF.
+ bool IsDtmf(uint8_t rtp_payload_type) const;
+
+ // Returns true if |rtp_payload_type| is registered as RED.
+ bool IsRed(uint8_t rtp_payload_type) const;
// Returns kOK if all packets in |packet_list| carry payload types that are
// registered in the database. Otherwise, returns kDecoderNotFound.
- virtual int CheckPayloadTypes(const PacketList& packet_list) const;
+ int CheckPayloadTypes(const PacketList& packet_list) const;
private:
typedef std::map<uint8_t, DecoderInfo> DecoderMap;
@@ -174,7 +190,7 @@
DecoderMap decoders_;
int active_decoder_type_;
int active_cng_decoder_type_;
- std::unique_ptr<ComfortNoiseDecoder> active_cng_decoder_;
+ mutable std::unique_ptr<ComfortNoiseDecoder> active_cng_decoder_;
rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_;
RTC_DISALLOW_COPY_AND_ASSIGN(DecoderDatabase);
diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h
index 3865db2..1f6d3bc 100644
--- a/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h
+++ b/webrtc/modules/audio_coding/neteq/mock/mock_decoder_database.h
@@ -21,7 +21,9 @@
class MockDecoderDatabase : public DecoderDatabase {
public:
- MockDecoderDatabase() : DecoderDatabase(nullptr) {}
+ explicit MockDecoderDatabase(
+ rtc::scoped_refptr<AudioDecoderFactory> factory = nullptr)
+ : DecoderDatabase(factory) {}
virtual ~MockDecoderDatabase() { Die(); }
MOCK_METHOD0(Die, void());
MOCK_CONST_METHOD0(Empty,
@@ -44,26 +46,14 @@
const DecoderInfo*(uint8_t rtp_payload_type));
MOCK_CONST_METHOD1(GetRtpPayloadType,
uint8_t(NetEqDecoder codec_type));
- MOCK_METHOD1(GetDecoder,
- AudioDecoder*(uint8_t rtp_payload_type));
- MOCK_CONST_METHOD2(IsType,
- bool(uint8_t rtp_payload_type, NetEqDecoder codec_type));
- MOCK_CONST_METHOD1(IsComfortNoise,
- bool(uint8_t rtp_payload_type));
- MOCK_CONST_METHOD1(IsDtmf,
- bool(uint8_t rtp_payload_type));
- MOCK_CONST_METHOD1(IsRed,
- bool(uint8_t rtp_payload_type));
MOCK_METHOD2(SetActiveDecoder,
int(uint8_t rtp_payload_type, bool* new_decoder));
- MOCK_METHOD0(GetActiveDecoder,
+ MOCK_CONST_METHOD0(GetActiveDecoder,
AudioDecoder*());
MOCK_METHOD1(SetActiveCngDecoder,
int(uint8_t rtp_payload_type));
- MOCK_METHOD0(GetActiveCngDecoder,
+ MOCK_CONST_METHOD0(GetActiveCngDecoder,
ComfortNoiseDecoder*());
- MOCK_CONST_METHOD1(CheckPayloadTypes,
- int(const PacketList& packet_list));
};
} // namespace webrtc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
index f9b9e7b..f6caca3 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -269,24 +269,10 @@
*dec = std::move(mock_decoder);
}));
- DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "");
+ DecoderDatabase::DecoderInfo info(NetEqDecoder::kDecoderPCMu, "",
+ mock_decoder_factory);
// Expectations for decoder database.
- EXPECT_CALL(*mock_decoder_database_, IsRed(kPayloadType))
- .WillRepeatedly(Return(false)); // This is not RED.
- EXPECT_CALL(*mock_decoder_database_, CheckPayloadTypes(_))
- .Times(2)
- .WillRepeatedly(Return(DecoderDatabase::kOK)); // Payload type is valid.
- EXPECT_CALL(*mock_decoder_database_, IsDtmf(kPayloadType))
- .WillRepeatedly(Return(false)); // This is not DTMF.
- EXPECT_CALL(*mock_decoder_database_, GetDecoder(kPayloadType))
- .Times(3)
- .WillRepeatedly(
- Invoke([&info, mock_decoder_factory](uint8_t payload_type) {
- return info.GetDecoder(mock_decoder_factory);
- }));
- EXPECT_CALL(*mock_decoder_database_, IsComfortNoise(kPayloadType))
- .WillRepeatedly(Return(false)); // This is not CNG.
EXPECT_CALL(*mock_decoder_database_, GetDecoderInfo(kPayloadType))
.WillRepeatedly(Return(&info));
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
index da35301..73dbd62 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc
@@ -174,10 +174,6 @@
}
MockDecoderDatabase decoder_database;
- EXPECT_CALL(decoder_database, IsComfortNoise(0))
- .WillRepeatedly(Return(false));
- EXPECT_CALL(decoder_database, IsDtmf(0))
- .WillRepeatedly(Return(false));
uint8_t current_pt = 0xFF;
uint8_t current_cng_pt = 0xFF;
EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacketList(&list,
@@ -216,10 +212,6 @@
MockDecoderDatabase decoder_database;
- EXPECT_CALL(decoder_database, IsComfortNoise(_))
- .WillRepeatedly(Return(false));
- EXPECT_CALL(decoder_database, IsDtmf(_))
- .WillRepeatedly(Return(false));
uint8_t current_pt = 0xFF;
uint8_t current_cng_pt = 0xFF;
EXPECT_EQ(PacketBuffer::kFlushed, buffer.InsertPacketList(&list,
@@ -349,10 +341,6 @@
}
MockDecoderDatabase decoder_database;
- EXPECT_CALL(decoder_database, IsComfortNoise(0))
- .WillRepeatedly(Return(false));
- EXPECT_CALL(decoder_database, IsDtmf(0))
- .WillRepeatedly(Return(false));
uint8_t current_pt = 0xFF;
uint8_t current_cng_pt = 0xFF;
@@ -424,10 +412,6 @@
list.push_back(packet);
list.push_back(gen.NextPacket(payload_len)); // Valid packet.
MockDecoderDatabase decoder_database;
- EXPECT_CALL(decoder_database, IsComfortNoise(0))
- .WillRepeatedly(Return(false));
- EXPECT_CALL(decoder_database, IsDtmf(0))
- .WillRepeatedly(Return(false));
uint8_t current_pt = 0xFF;
uint8_t current_cng_pt = 0xFF;
EXPECT_EQ(PacketBuffer::kInvalidPacket,
