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-Commit-Position: refs/heads/master@{#19572}
This commit is contained in:
magjed 2017-08-29 00:58:52 -07:00 committed by Commit Bot
parent d4495312dc
commit f52d34d682
5 changed files with 50 additions and 51 deletions

View File

@ -69,7 +69,7 @@ void GetForcedFallbackParamsFromFieldTrialGroup(uint32_t* param_low_kbps,
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 @@ VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper(
packet_loss_(0),
rtt_(0),
codec_(codec),
encoder_(encoder),
encoder_(std::move(encoder)),
callback_(nullptr),
forced_fallback_possible_(EnableForcedFallback(codec)) {
if (forced_fallback_possible_) {

View File

@ -25,8 +25,9 @@ namespace webrtc {
// 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 @@ class VideoEncoderSoftwareFallbackWrapper : public VideoEncoder {
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_;

View File

@ -39,7 +39,9 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test {
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 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test {
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::EncodeFrame(int expected_ret) {
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 @@ void VideoEncoderSoftwareFallbackWrapperTest::UtilizeFallbackEncoder() {
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 @@ void VideoEncoderSoftwareFallbackWrapperTest::UtilizeFallbackEncoder() {
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 @@ void VideoEncoderSoftwareFallbackWrapperTest::FallbackFromEncodeRequest() {
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, CanUtilizeFallbackEncoder) {
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 @@ TEST_F(VideoEncoderSoftwareFallbackWrapperTest,
// 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,
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 @@ class ForcedFallbackTest : public VideoEncoderSoftwareFallbackWrapperTest {
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, MultipleStartEndFallback) {
}
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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsKeptWhenInitEncodeIsCalled) {
// 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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsEndedWhenResolutionIsTooLarge) {
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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsEndedForNonValidSettings) {
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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsEndedForNonValidSettings) {
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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsKeptIfResolutionIsTooSmall) {
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 @@ TEST_F(ForcedFallbackTestEnabled, FallbackIsKeptIfResolutionIsTooSmall) {
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.

View File

@ -1415,16 +1415,15 @@ WebRtcVideoChannel::WebRtcVideoSendStream::VideoSendStreamParameters::
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 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoder(
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 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoder(
internal_encoder_factory_->CreateVideoEncoder(codec));
}
return AllocatedEncoder(std::move(internal_encoder),
nullptr /* external_encoder */, codec,
false /* is_external */, codec,
false /* has_internal_source */);
}

View File

@ -317,7 +317,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport {
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 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport {
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 @@ class WebRtcVideoChannel : public VideoMediaChannel, public webrtc::Transport {
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_;
};