Don't recreate the speech encoder if we don't have to
If the specification for the speech encoder hasn't changed, we should
reuse it instead of recreating it. Otherwise, we lose its state. (This
problem was originally discovered because AudioEncoderOpus instances
would forget that they were supposed to be using DTX.)
BUG=webrtc:6020, chromium:622647
Review-Url: https://codereview.webrtc.org/2089183002
Cr-Commit-Position: refs/heads/master@{#13273}
diff --git a/webrtc/modules/audio_coding/acm2/codec_manager.cc b/webrtc/modules/audio_coding/acm2/codec_manager.cc
index 81adf81..f028c45 100644
--- a/webrtc/modules/audio_coding/acm2/codec_manager.cc
+++ b/webrtc/modules/audio_coding/acm2/codec_manager.cc
@@ -113,7 +113,7 @@
}
send_codec_inst_ = rtc::Optional<CodecInst>(send_codec);
- codec_stack_params_.speech_encoder.reset(); // Caller must recreate it.
+ recreate_encoder_ = true; // Caller must recreate it.
return true;
}
@@ -190,5 +190,67 @@
return true;
}
+bool CodecManager::MakeEncoder(RentACodec* rac, AudioCodingModule* acm) {
+ RTC_DCHECK(rac);
+ RTC_DCHECK(acm);
+
+ if (!recreate_encoder_) {
+ bool error = false;
+ // Try to re-use the speech encoder we've given to the ACM.
+ acm->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
+ if (!*encoder) {
+ // There is no existing encoder.
+ recreate_encoder_ = true;
+ return;
+ }
+
+ // Extract the speech encoder from the ACM.
+ std::unique_ptr<AudioEncoder> enc = std::move(*encoder);
+ while (true) {
+ auto sub_enc = enc->ReclaimContainedEncoders();
+ if (sub_enc.empty()) {
+ break;
+ }
+ RTC_CHECK_EQ(1u, sub_enc.size());
+
+ // Replace enc with its sub encoder. We need to put the sub encoder in
+ // a temporary first, since otherwise the old value of enc would be
+ // destroyed before the new value got assigned, which would be bad
+ // since the new value is a part of the old value.
+ auto tmp_enc = std::move(sub_enc[0]);
+ enc = std::move(tmp_enc);
+ }
+
+ // Wrap it in a new encoder stack and put it back.
+ codec_stack_params_.speech_encoder = std::move(enc);
+ *encoder = rac->RentEncoderStack(&codec_stack_params_);
+ if (!*encoder) {
+ error = true;
+ }
+ });
+ if (error) {
+ return false;
+ }
+ if (!recreate_encoder_) {
+ return true;
+ }
+ }
+
+ if (!send_codec_inst_) {
+ // We don't have the information we need to create a new speech encoder.
+ // (This is not an error.)
+ return true;
+ }
+
+ codec_stack_params_.speech_encoder = rac->RentEncoder(*send_codec_inst_);
+ auto stack = rac->RentEncoderStack(&codec_stack_params_);
+ if (!stack) {
+ return false;
+ }
+ acm->SetEncoder(std::move(stack));
+ recreate_encoder_ = false;
+ return true;
+}
+
} // namespace acm2
} // namespace webrtc
diff --git a/webrtc/modules/audio_coding/acm2/codec_manager.h b/webrtc/modules/audio_coding/acm2/codec_manager.h
index f6c6cd4..b60b7e7 100644
--- a/webrtc/modules/audio_coding/acm2/codec_manager.h
+++ b/webrtc/modules/audio_coding/acm2/codec_manager.h
@@ -59,31 +59,13 @@
// Uses the provided Rent-A-Codec to create a new encoder stack, if we have a
// complete specification; if so, it is then passed to set_encoder. On error,
// returns false.
- bool MakeEncoder(RentACodec* rac, AudioCodingModule* acm) {
- RTC_DCHECK(rac);
- RTC_DCHECK(acm);
- if (!codec_stack_params_.speech_encoder && send_codec_inst_) {
- // We have no speech encoder, but we have a specification for making one.
- auto enc = rac->RentEncoder(*send_codec_inst_);
- if (!enc)
- return false;
- codec_stack_params_.speech_encoder = std::move(enc);
- }
- auto stack = rac->RentEncoderStack(&codec_stack_params_);
- if (stack) {
- // Give new encoder stack to the ACM.
- acm->SetEncoder(std::move(stack));
- } else {
- // The specification was good but incomplete, so we have no encoder stack
- // to give to the ACM.
- }
- return true;
- }
+ bool MakeEncoder(RentACodec* rac, AudioCodingModule* acm);
private:
rtc::ThreadChecker thread_checker_;
rtc::Optional<CodecInst> send_codec_inst_;
RentACodec::StackParameters codec_stack_params_;
+ bool recreate_encoder_ = true; // Need to recreate encoder?
RTC_DISALLOW_COPY_AND_ASSIGN(CodecManager);
};