Let VideoEncoderSoftwareFallbackWrapper own the wrapped encoder
Currently, ownership of the wrapped hardware encoder is handled outside
VideoEncoderSoftwareFallbackWrapper. It's easier if
VideoEncoderSoftwareFallbackWrapper owns and relases it instead.
BUG=webrtc:7925
Review-Url: https://codereview.webrtc.org/3007683002
Cr-Original-Commit-Position: refs/heads/master@{#19572}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: f52d34d682a049028ef5f2d126721dce03a50d8b
diff --git a/media/engine/videoencodersoftwarefallbackwrapper.cc b/media/engine/videoencodersoftwarefallbackwrapper.cc
index 946b91e..7e2a678 100644
--- a/media/engine/videoencodersoftwarefallbackwrapper.cc
+++ b/media/engine/videoencodersoftwarefallbackwrapper.cc
@@ -69,7 +69,7 @@
VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper(
const cricket::VideoCodec& codec,
- webrtc::VideoEncoder* encoder)
+ std::unique_ptr<webrtc::VideoEncoder> encoder)
: number_of_cores_(0),
max_payload_size_(0),
rates_set_(false),
@@ -78,7 +78,7 @@
packet_loss_(0),
rtt_(0),
codec_(codec),
- encoder_(encoder),
+ encoder_(std::move(encoder)),
callback_(nullptr),
forced_fallback_possible_(EnableForcedFallback(codec)) {
if (forced_fallback_possible_) {
diff --git a/media/engine/videoencodersoftwarefallbackwrapper.h b/media/engine/videoencodersoftwarefallbackwrapper.h
index 8061c00..8dec2f5 100644
--- a/media/engine/videoencodersoftwarefallbackwrapper.h
+++ b/media/engine/videoencodersoftwarefallbackwrapper.h
@@ -25,8 +25,9 @@
// hardware restrictions, such as max resolution.
class VideoEncoderSoftwareFallbackWrapper : public VideoEncoder {
public:
- VideoEncoderSoftwareFallbackWrapper(const cricket::VideoCodec& codec,
- webrtc::VideoEncoder* encoder);
+ VideoEncoderSoftwareFallbackWrapper(
+ const cricket::VideoCodec& codec,
+ std::unique_ptr<webrtc::VideoEncoder> encoder);
int32_t InitEncode(const VideoCodec* codec_settings,
int32_t number_of_cores,
@@ -96,7 +97,7 @@
int64_t rtt_;
cricket::VideoCodec codec_;
- webrtc::VideoEncoder* const encoder_;
+ std::unique_ptr<webrtc::VideoEncoder> encoder_;
std::unique_ptr<webrtc::VideoEncoder> fallback_encoder_;
std::string fallback_implementation_name_;
diff --git a/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc b/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc
index ecf7199..f8521ab 100644
--- a/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc
+++ b/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc
@@ -39,7 +39,9 @@
explicit VideoEncoderSoftwareFallbackWrapperTest(
const std::string& field_trials)
: override_field_trials_(field_trials),
- fallback_wrapper_(cricket::VideoCodec("VP8"), &fake_encoder_) {}
+ fake_encoder_(new CountingFakeEncoder()),
+ fallback_wrapper_(cricket::VideoCodec("VP8"),
+ std::unique_ptr<VideoEncoder>(fake_encoder_)) {}
class CountingFakeEncoder : public VideoEncoder {
public:
@@ -134,7 +136,8 @@
test::ScopedFieldTrials override_field_trials_;
FakeEncodedImageCallback callback_;
- CountingFakeEncoder fake_encoder_;
+ // |fake_encoder_| is owned and released by |fallback_wrapper_|.
+ CountingFakeEncoder* fake_encoder_;
VideoEncoderSoftwareFallbackWrapper fallback_wrapper_;
VideoCodec codec_ = {};
std::unique_ptr<VideoFrame> frame_;
@@ -157,7 +160,7 @@
void VideoEncoderSoftwareFallbackWrapperTest::UtilizeFallbackEncoder() {
fallback_wrapper_.RegisterEncodeCompleteCallback(&callback_);
- EXPECT_EQ(&callback_, fake_encoder_.encode_complete_callback_);
+ EXPECT_EQ(&callback_, fake_encoder_->encode_complete_callback_);
// Register with failing fake encoder. Should succeed with VP8 fallback.
codec_.codecType = kVideoCodecVP8;
@@ -171,7 +174,7 @@
rate_allocator_.reset(
new SimulcastRateAllocator(codec_, std::move(tl_factory)));
- fake_encoder_.init_encode_return_code_ = WEBRTC_VIDEO_CODEC_ERROR;
+ fake_encoder_->init_encode_return_code_ = WEBRTC_VIDEO_CODEC_ERROR;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
EXPECT_EQ(
@@ -180,9 +183,9 @@
rate_allocator_->GetAllocation(300000, kFramerate), kFramerate));
int callback_count = callback_.callback_count_;
- int encode_count = fake_encoder_.encode_count_;
+ int encode_count = fake_encoder_->encode_count_;
EncodeFrame();
- EXPECT_EQ(encode_count, fake_encoder_.encode_count_);
+ EXPECT_EQ(encode_count, fake_encoder_->encode_count_);
EXPECT_EQ(callback_count + 1, callback_.callback_count_);
}
@@ -203,30 +206,30 @@
WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.SetRateAllocation(
rate_allocator_->GetAllocation(300000, kFramerate), kFramerate));
- EXPECT_EQ(1, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_);
// Have the non-fallback encoder request a software fallback.
- fake_encoder_.encode_return_code_ = WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE;
+ fake_encoder_->encode_return_code_ = WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE;
int callback_count = callback_.callback_count_;
- int encode_count = fake_encoder_.encode_count_;
+ int encode_count = fake_encoder_->encode_count_;
EncodeFrame();
// Single encode request, which returned failure.
- EXPECT_EQ(encode_count + 1, fake_encoder_.encode_count_);
+ EXPECT_EQ(encode_count + 1, fake_encoder_->encode_count_);
EXPECT_EQ(callback_count + 1, callback_.callback_count_);
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest, InitializesEncoder) {
VideoCodec codec = {};
fallback_wrapper_.InitEncode(&codec, 2, kMaxPayloadSize);
- EXPECT_EQ(1, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_);
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest, EncodeRequestsFallback) {
FallbackFromEncodeRequest();
// After fallback, further encodes shouldn't hit the fake encoder.
- int encode_count = fake_encoder_.encode_count_;
+ int encode_count = fake_encoder_->encode_count_;
EncodeFrame();
- EXPECT_EQ(encode_count, fake_encoder_.encode_count_);
+ EXPECT_EQ(encode_count, fake_encoder_->encode_count_);
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest, CanUtilizeFallbackEncoder) {
@@ -236,20 +239,20 @@
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
InternalEncoderReleasedDuringFallback) {
- EXPECT_EQ(0, fake_encoder_.release_count_);
+ EXPECT_EQ(0, fake_encoder_->release_count_);
UtilizeFallbackEncoder();
- EXPECT_EQ(1, fake_encoder_.release_count_);
+ EXPECT_EQ(1, fake_encoder_->release_count_);
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release());
// No extra release when the fallback is released.
- EXPECT_EQ(1, fake_encoder_.release_count_);
+ EXPECT_EQ(1, fake_encoder_->release_count_);
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
InternalEncoderNotEncodingDuringFallback) {
UtilizeFallbackEncoder();
- int encode_count = fake_encoder_.encode_count_;
+ int encode_count = fake_encoder_->encode_count_;
EncodeFrame();
- EXPECT_EQ(encode_count, fake_encoder_.encode_count_);
+ EXPECT_EQ(encode_count, fake_encoder_->encode_count_);
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release());
}
@@ -261,7 +264,7 @@
// encoder is being used.
FakeEncodedImageCallback callback2;
fallback_wrapper_.RegisterEncodeCompleteCallback(&callback2);
- EXPECT_EQ(&callback2, fake_encoder_.encode_complete_callback_);
+ EXPECT_EQ(&callback2, fake_encoder_->encode_complete_callback_);
// Encoding a frame using the fallback should arrive at the new callback.
std::vector<FrameType> types(1, kVideoFrameKey);
@@ -275,32 +278,32 @@
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
SetChannelParametersForwardedDuringFallback) {
UtilizeFallbackEncoder();
- EXPECT_EQ(0, fake_encoder_.set_channel_parameters_count_);
+ EXPECT_EQ(0, fake_encoder_->set_channel_parameters_count_);
fallback_wrapper_.SetChannelParameters(1, 1);
- EXPECT_EQ(1, fake_encoder_.set_channel_parameters_count_);
+ EXPECT_EQ(1, fake_encoder_->set_channel_parameters_count_);
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release());
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
SetRatesForwardedDuringFallback) {
UtilizeFallbackEncoder();
- EXPECT_EQ(1, fake_encoder_.set_rates_count_);
+ EXPECT_EQ(1, fake_encoder_->set_rates_count_);
fallback_wrapper_.SetRateAllocation(BitrateAllocation(), 1);
- EXPECT_EQ(2, fake_encoder_.set_rates_count_);
+ EXPECT_EQ(2, fake_encoder_->set_rates_count_);
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release());
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
SupportsNativeHandleForwardedWithoutFallback) {
fallback_wrapper_.SupportsNativeHandle();
- EXPECT_EQ(1, fake_encoder_.supports_native_handle_count_);
+ EXPECT_EQ(1, fake_encoder_->supports_native_handle_count_);
}
TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
SupportsNativeHandleNotForwardedDuringFallback) {
UtilizeFallbackEncoder();
fallback_wrapper_.SupportsNativeHandle();
- EXPECT_EQ(0, fake_encoder_.supports_native_handle_count_);
+ EXPECT_EQ(0, fake_encoder_->supports_native_handle_count_);
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.Release());
}
@@ -343,7 +346,7 @@
ConfigureVp8Codec();
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_.InitEncode(
&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(1, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_);
}
void TearDown() override {
@@ -509,7 +512,7 @@
}
TEST_F(ForcedFallbackTestEnabled, DropsFirstNonNativeFrameAfterFallbackEnds) {
- fake_encoder_.supports_native_handle_ = true;
+ fake_encoder_->supports_native_handle_ = true;
// Bitrate at low threshold.
SetRateAllocation(kLowKbps);
@@ -536,7 +539,7 @@
// Re-initialize encoder, still expect fallback.
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(1, fake_encoder_.init_encode_count_); // No change.
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_); // No change.
SetRateAllocation(kLowKbps);
EncodeFrameAndVerifyLastName("libvpx");
}
@@ -553,7 +556,7 @@
codec_.width += 1;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(2, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(2, fake_encoder_->init_encode_count_);
SetRateAllocation(kLowKbps);
EncodeFrameAndVerifyLastName("fake-encoder");
}
@@ -570,7 +573,7 @@
codec_.VP8()->numberOfTemporalLayers = 2;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(2, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(2, fake_encoder_->init_encode_count_);
SetRateAllocation(kLowKbps);
EncodeFrameAndVerifyLastName("fake-encoder");
@@ -578,7 +581,7 @@
codec_.VP8()->numberOfTemporalLayers = 1;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(3, fake_encoder_.init_encode_count_);
+ EXPECT_EQ(3, fake_encoder_->init_encode_count_);
// Bitrate at low threshold.
SetRateAllocation(kLowKbps);
EncodeFrameAndVerifyLastName("fake-encoder");
@@ -622,7 +625,7 @@
codec_.height = kMinPixelsStop / codec_.width - 1;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(1, fake_encoder_.init_encode_count_); // No change
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_); // No change
SetRateAllocation(kHighKbps - 1);
EncodeFrameAndVerifyLastName("libvpx");
// Bitrate at high threshold but resolution too small for fallback to end.
@@ -633,7 +636,7 @@
codec_.height++;
EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
fallback_wrapper_.InitEncode(&codec_, kNumCores, kMaxPayloadSize));
- EXPECT_EQ(1, fake_encoder_.init_encode_count_); // No change
+ EXPECT_EQ(1, fake_encoder_->init_encode_count_); // No change
SetRateAllocation(kHighKbps - 1);
EncodeFrameAndVerifyLastName("libvpx");
// Bitrate at high threshold and resolution large enough for fallback to end.
diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index 9c11bb9..274b801 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1415,16 +1415,15 @@
WebRtcVideoChannel::WebRtcVideoSendStream::AllocatedEncoder::AllocatedEncoder(
std::unique_ptr<webrtc::VideoEncoder> encoder,
- std::unique_ptr<webrtc::VideoEncoder> external_encoder,
+ bool is_external,
const cricket::VideoCodec& codec,
bool has_internal_source)
: encoder_(std::move(encoder)),
- external_encoder_(std::move(external_encoder)),
+ is_external_(is_external),
codec_(codec),
has_internal_source_(has_internal_source) {}
void WebRtcVideoChannel::WebRtcVideoSendStream::AllocatedEncoder::Reset() {
- external_encoder_.reset();
encoder_.reset();
codec_ = cricket::VideoCodec();
}
@@ -1602,13 +1601,13 @@
if (external_encoder) {
std::unique_ptr<webrtc::VideoEncoder> internal_encoder(
new webrtc::VideoEncoderSoftwareFallbackWrapper(
- codec, external_encoder.get()));
+ codec, std::move(external_encoder)));
const webrtc::VideoCodecType codec_type =
webrtc::PayloadStringToCodecType(codec.name);
const bool has_internal_source =
external_encoder_factory_->EncoderTypeHasInternalSource(codec_type);
return AllocatedEncoder(std::move(internal_encoder),
- std::move(external_encoder), codec,
+ true /* is_external */, codec,
has_internal_source);
}
}
@@ -1629,7 +1628,7 @@
internal_encoder_factory_->CreateVideoEncoder(codec));
}
return AllocatedEncoder(std::move(internal_encoder),
- nullptr /* external_encoder */, codec,
+ false /* is_external */, codec,
false /* has_internal_source */);
}
diff --git a/media/engine/webrtcvideoengine.h b/media/engine/webrtcvideoengine.h
index caca11c..deb13b1 100644
--- a/media/engine/webrtcvideoengine.h
+++ b/media/engine/webrtcvideoengine.h
@@ -317,7 +317,7 @@
AllocatedEncoder& operator=(AllocatedEncoder&&) = default;
AllocatedEncoder(std::unique_ptr<webrtc::VideoEncoder> encoder,
- std::unique_ptr<webrtc::VideoEncoder> external_encoder,
+ bool is_external,
const cricket::VideoCodec& codec,
bool has_internal_source);
@@ -326,7 +326,7 @@
webrtc::VideoEncoder* encoder() { return encoder_.get(); }
// Returns true if the encoder is external.
- bool IsExternal() { return static_cast<bool>(external_encoder_); }
+ bool IsExternal() { return is_external_; }
cricket::VideoCodec codec() { return codec_; }
@@ -337,11 +337,7 @@
private:
std::unique_ptr<webrtc::VideoEncoder> encoder_;
- // TODO(magjed): |external_encoder_| is not used except for managing
- // the lifetime when we use VideoEncoderSoftwareFallbackWrapper. Let
- // VideoEncoderSoftwareFallbackWrapper own the external encoder instead
- // and remove this member variable.
- std::unique_ptr<webrtc::VideoEncoder> external_encoder_;
+ bool is_external_;
cricket::VideoCodec codec_;
bool has_internal_source_;
};
