Rent-A-Codec: Reference count the shared iSAC bandwidth estimation state
Now that the Rent-A-Codec no longer owns the encoders and decoders it
produces, they may outlive it. It's thus no longer correct for the
Rent-A-Codec to own the bandwidth estimation state; all of the
involved objects need to share ownership.
BUG=webrtc:5028
Review URL: https://codereview.webrtc.org/1821513003
Cr-Commit-Position: refs/heads/master@{#12159}
diff --git a/webrtc/modules/audio_coding/acm2/rent_a_codec.cc b/webrtc/modules/audio_coding/acm2/rent_a_codec.cc
index 71d34d1..3bf959e 100644
--- a/webrtc/modules/audio_coding/acm2/rent_a_codec.cc
+++ b/webrtc/modules/audio_coding/acm2/rent_a_codec.cc
@@ -40,6 +40,10 @@
#include "webrtc/modules/audio_coding/acm2/acm_codec_database.h"
#include "webrtc/modules/audio_coding/acm2/acm_common_defs.h"
+#if defined(WEBRTC_CODEC_ISACFX) || defined(WEBRTC_CODEC_ISAC)
+#include "webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h"
+#endif
+
namespace webrtc {
namespace acm2 {
@@ -145,8 +149,9 @@
// Returns a new speech encoder, or null on error.
// TODO(kwiberg): Don't handle errors here (bug 5033)
-std::unique_ptr<AudioEncoder> CreateEncoder(const CodecInst& speech_inst,
- LockedIsacBandwidthInfo* bwinfo) {
+std::unique_ptr<AudioEncoder> CreateEncoder(
+ const CodecInst& speech_inst,
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo) {
#if defined(WEBRTC_CODEC_ISACFX)
if (STR_CASE_CMP(speech_inst.plname, "isac") == 0)
return std::unique_ptr<AudioEncoder>(
@@ -221,7 +226,7 @@
}
std::unique_ptr<AudioDecoder> CreateIsacDecoder(
- LockedIsacBandwidthInfo* bwinfo) {
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo) {
#if defined(WEBRTC_CODEC_ISACFX)
return std::unique_ptr<AudioDecoder>(new AudioDecoderIsacFix(bwinfo));
#elif defined(WEBRTC_CODEC_ISAC)
@@ -234,12 +239,16 @@
} // namespace
-RentACodec::RentACodec() = default;
+RentACodec::RentACodec() {
+#if defined(WEBRTC_CODEC_ISACFX) || defined(WEBRTC_CODEC_ISAC)
+ isac_bandwidth_info_ = new LockedIsacBandwidthInfo;
+#endif
+}
RentACodec::~RentACodec() = default;
std::unique_ptr<AudioEncoder> RentACodec::RentEncoder(
const CodecInst& codec_inst) {
- return CreateEncoder(codec_inst, &isac_bandwidth_info_);
+ return CreateEncoder(codec_inst, isac_bandwidth_info_);
}
RentACodec::StackParameters::StackParameters() {
@@ -295,7 +304,7 @@
}
std::unique_ptr<AudioDecoder> RentACodec::RentIsacDecoder() {
- return CreateIsacDecoder(&isac_bandwidth_info_);
+ return CreateIsacDecoder(isac_bandwidth_info_);
}
} // namespace acm2
diff --git a/webrtc/modules/audio_coding/acm2/rent_a_codec.h b/webrtc/modules/audio_coding/acm2/rent_a_codec.h
index e41f77d..2c5873d 100644
--- a/webrtc/modules/audio_coding/acm2/rent_a_codec.h
+++ b/webrtc/modules/audio_coding/acm2/rent_a_codec.h
@@ -18,23 +18,16 @@
#include "webrtc/base/array_view.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/optional.h"
+#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/modules/audio_coding/codecs/audio_decoder.h"
#include "webrtc/modules/audio_coding/codecs/audio_encoder.h"
#include "webrtc/modules/audio_coding/include/audio_coding_module_typedefs.h"
#include "webrtc/typedefs.h"
-#if defined(WEBRTC_CODEC_ISAC) || defined(WEBRTC_CODEC_ISACFX)
-#include "webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h"
-#else
-// Dummy implementation, for when we don't have iSAC.
-namespace webrtc {
-class LockedIsacBandwidthInfo {};
-}
-#endif
-
namespace webrtc {
struct CodecInst;
+class LockedIsacBandwidthInfo;
namespace acm2 {
@@ -229,7 +222,7 @@
std::unique_ptr<AudioEncoder> speech_encoder_;
std::unique_ptr<AudioEncoder> cng_encoder_;
std::unique_ptr<AudioEncoder> red_encoder_;
- LockedIsacBandwidthInfo isac_bandwidth_info_;
+ rtc::scoped_refptr<LockedIsacBandwidthInfo> isac_bandwidth_info_;
RTC_DISALLOW_COPY_AND_ASSIGN(RentACodec);
};
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t.h b/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t.h
index 845af42..d9d20ec 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t.h
@@ -13,6 +13,7 @@
#include <vector>
+#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/modules/audio_coding/codecs/audio_decoder.h"
#include "webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h"
@@ -22,7 +23,8 @@
class AudioDecoderIsacT final : public AudioDecoder {
public:
AudioDecoderIsacT();
- explicit AudioDecoderIsacT(LockedIsacBandwidthInfo* bwinfo);
+ explicit AudioDecoderIsacT(
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo);
~AudioDecoderIsacT() override;
bool HasDecodePlc() const override;
@@ -43,7 +45,7 @@
private:
typename T::instance_type* isac_state_;
- LockedIsacBandwidthInfo* bwinfo_;
+ rtc::scoped_refptr<LockedIsacBandwidthInfo> bwinfo_;
int decoder_sample_rate_hz_;
RTC_DISALLOW_COPY_AND_ASSIGN(AudioDecoderIsacT);
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t_impl.h b/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t_impl.h
index a986bc4..4998fea 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t_impl.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_decoder_isac_t_impl.h
@@ -22,7 +22,8 @@
: AudioDecoderIsacT(nullptr) {}
template <typename T>
-AudioDecoderIsacT<T>::AudioDecoderIsacT(LockedIsacBandwidthInfo* bwinfo)
+AudioDecoderIsacT<T>::AudioDecoderIsacT(
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo)
: bwinfo_(bwinfo), decoder_sample_rate_hz_(-1) {
RTC_CHECK_EQ(0, T::Create(&isac_state_));
T::DecoderInit(isac_state_);
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
index e840f3b..0da8ed7 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
@@ -13,6 +13,7 @@
#include <vector>
+#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/modules/audio_coding/codecs/audio_encoder.h"
#include "webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h"
@@ -30,7 +31,7 @@
struct Config {
bool IsOk() const;
- LockedIsacBandwidthInfo* bwinfo = nullptr;
+ rtc::scoped_refptr<LockedIsacBandwidthInfo> bwinfo;
int payload_type = 103;
int sample_rate_hz = 16000;
@@ -50,8 +51,9 @@
};
explicit AudioEncoderIsacT(const Config& config);
- explicit AudioEncoderIsacT(const CodecInst& codec_inst,
- LockedIsacBandwidthInfo* bwinfo);
+ explicit AudioEncoderIsacT(
+ const CodecInst& codec_inst,
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo);
~AudioEncoderIsacT() override;
size_t MaxEncodedBytes() const override;
@@ -77,7 +79,7 @@
Config config_;
typename T::instance_type* isac_state_ = nullptr;
- LockedIsacBandwidthInfo* bwinfo_ = nullptr;
+ rtc::scoped_refptr<LockedIsacBandwidthInfo> bwinfo_;
// Have we accepted input but not yet emitted it in a packet?
bool packet_in_progress_ = false;
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
index 3ad0f27..1debbeb 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
@@ -21,7 +21,7 @@
template <typename T>
typename AudioEncoderIsacT<T>::Config CreateIsacConfig(
const CodecInst& codec_inst,
- LockedIsacBandwidthInfo* bwinfo) {
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo) {
typename AudioEncoderIsacT<T>::Config config;
config.bwinfo = bwinfo;
config.payload_type = codec_inst.pltype;
@@ -69,8 +69,9 @@
}
template <typename T>
-AudioEncoderIsacT<T>::AudioEncoderIsacT(const CodecInst& codec_inst,
- LockedIsacBandwidthInfo* bwinfo)
+AudioEncoderIsacT<T>::AudioEncoderIsacT(
+ const CodecInst& codec_inst,
+ const rtc::scoped_refptr<LockedIsacBandwidthInfo>& bwinfo)
: AudioEncoderIsacT(CreateIsacConfig<T>(codec_inst, bwinfo)) {}
template <typename T>
diff --git a/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.cc b/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.cc
index 86f6055..5b9439f 100644
--- a/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.cc
+++ b/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.cc
@@ -12,7 +12,7 @@
namespace webrtc {
-LockedIsacBandwidthInfo::LockedIsacBandwidthInfo() {
+LockedIsacBandwidthInfo::LockedIsacBandwidthInfo() : ref_count_(0) {
bwinfo_.in_use = 0;
}
diff --git a/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h b/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h
index 6bb32c9..1ab5ae9 100644
--- a/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h
+++ b/webrtc/modules/audio_coding/codecs/isac/locked_bandwidth_info.h
@@ -11,6 +11,7 @@
#ifndef WEBRTC_MODULES_AUDIO_CODING_CODECS_ISAC_LOCKED_BANDWIDTH_INFO_H_
#define WEBRTC_MODULES_AUDIO_CODING_CODECS_ISAC_LOCKED_BANDWIDTH_INFO_H_
+#include "webrtc/base/atomicops.h"
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/modules/audio_coding/codecs/isac/bandwidth_info.h"
@@ -34,7 +35,18 @@
bwinfo_ = bwinfo;
}
+ int AddRef() const { return rtc::AtomicOps::Increment(&ref_count_); }
+
+ int Release() const {
+ const int count = rtc::AtomicOps::Decrement(&ref_count_);
+ if (count == 0) {
+ delete this;
+ }
+ return count;
+ }
+
private:
+ mutable volatile int ref_count_;
rtc::CriticalSection lock_;
IsacBandwidthInfo bwinfo_ GUARDED_BY(lock_);
};
