From 70e39e159eb350cb1acbf81dbe0d84d51c77b664 Mon Sep 17 00:00:00 2001 From: brandtr Date: Wed, 3 May 2017 03:40:58 -0700 Subject: [PATCH] Don't reinit encoder when rotation changes. TESTED=By rotating phone in AppRTCMobile. BUG=webrtc:7535 Review-Url: https://codereview.webrtc.org/2853463004 Cr-Commit-Position: refs/heads/master@{#17985} --- .../codecs/vp8/test/vp8_impl_unittest.cc | 21 +++++++++++++++++++ .../codecs/vp9/test/vp9_impl_unittest.cc | 19 +++++++++++++++++ webrtc/video/vie_encoder.cc | 13 +++++------- webrtc/video/vie_encoder.h | 3 --- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index ece5b63f05..d566ea82b0 100644 --- a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -83,6 +83,7 @@ Vp8UnitTestEncodeCompleteCallback::OnEncodedImage( encoded_frame_->_timeStamp = encoded_frame._timeStamp; encoded_frame_->_frameType = encoded_frame._frameType; encoded_frame_->_completeFrame = encoded_frame._completeFrame; + encoded_frame_->rotation_ = encoded_frame.rotation_; encoded_frame_->qp_ = encoded_frame.qp_; codec_specific_info_->codecType = codec_specific_info->codecType; // Skip |codec_name|, to avoid allocating. @@ -273,6 +274,26 @@ TEST_F(TestVp8Impl, EncoderParameterTest) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, decoder_->InitDecode(&codec_settings_, 1)); } +// We only test the encoder here, since the decoded frame rotation is set based +// on the CVO RTP header extension in VCMDecodedFrameCallback::Decoded. +// TODO(brandtr): Consider passing through the rotation flag through the decoder +// in the same way as done in the encoder. +TEST_F(TestVp8Impl, EncodedRotationEqualsInputRotation) { + SetUpEncodeDecode(); + + input_frame_->set_rotation(kVideoRotation_0); + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + encoder_->Encode(*input_frame_, nullptr, nullptr)); + WaitForEncodedFrame(); + EXPECT_EQ(kVideoRotation_0, encoded_frame_.rotation_); + + input_frame_->set_rotation(kVideoRotation_90); + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + encoder_->Encode(*input_frame_, nullptr, nullptr)); + WaitForEncodedFrame(); + EXPECT_EQ(kVideoRotation_90, encoded_frame_.rotation_); +} + TEST_F(TestVp8Impl, DecodedQpEqualsEncodedQp) { SetUpEncodeDecode(); encoder_->Encode(*input_frame_, nullptr, nullptr); diff --git a/webrtc/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc b/webrtc/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc index e4cb7eb814..2ed5e852b6 100644 --- a/webrtc/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc @@ -66,6 +66,25 @@ TEST_F(TestVp9Impl, EncodeDecode) { EXPECT_GT(I420PSNR(input_frame_.get(), decoded_frame.get()), 36); } +// We only test the encoder here, since the decoded frame rotation is set based +// on the CVO RTP header extension in VCMDecodedFrameCallback::Decoded. +// TODO(brandtr): Consider passing through the rotation flag through the decoder +// in the same way as done in the encoder. +TEST_F(TestVp9Impl, EncodedRotationEqualsInputRotation) { + input_frame_->set_rotation(kVideoRotation_0); + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + encoder_->Encode(*input_frame_, nullptr, nullptr)); + EncodedImage encoded_frame; + ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame)); + EXPECT_EQ(kVideoRotation_0, encoded_frame.rotation_); + + input_frame_->set_rotation(kVideoRotation_90); + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + encoder_->Encode(*input_frame_, nullptr, nullptr)); + ASSERT_TRUE(WaitForEncodedFrame(&encoded_frame)); + EXPECT_EQ(kVideoRotation_90, encoded_frame.rotation_); +} + TEST_F(TestVp9Impl, DecodedQpEqualsEncodedQp) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Encode(*input_frame_, nullptr, nullptr)); diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 2287a7e3bb..ed6d07cdac 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -483,8 +483,8 @@ void ViEEncoder::ConfigureEncoderOnTaskQueue(VideoEncoderConfig config, if (last_frame_info_) { ReconfigureEncoder(); } else if (settings_.internal_source) { - last_frame_info_ = rtc::Optional( - VideoFrameInfo(176, 144, kVideoRotation_0, false)); + last_frame_info_ = + rtc::Optional(VideoFrameInfo(176, 144, false)); ReconfigureEncoder(); } } @@ -653,16 +653,13 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame, if (!last_frame_info_ || video_frame.width() != last_frame_info_->width || video_frame.height() != last_frame_info_->height || - video_frame.rotation() != last_frame_info_->rotation || video_frame.is_texture() != last_frame_info_->is_texture) { pending_encoder_reconfiguration_ = true; - last_frame_info_ = rtc::Optional( - VideoFrameInfo(video_frame.width(), video_frame.height(), - video_frame.rotation(), video_frame.is_texture())); + last_frame_info_ = rtc::Optional(VideoFrameInfo( + video_frame.width(), video_frame.height(), video_frame.is_texture())); LOG(LS_INFO) << "Video frame parameters changed: dimensions=" << last_frame_info_->width << "x" << last_frame_info_->height - << ", rotation=" << last_frame_info_->rotation - << ", texture=" << last_frame_info_->is_texture; + << ", texture=" << last_frame_info_->is_texture << "."; } if (initial_rampup_ < kMaxInitialFramedrop && diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 213ff6d0a2..8481117ef8 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -133,15 +133,12 @@ class ViEEncoder : public rtc::VideoSinkInterface, public: VideoFrameInfo(int width, int height, - VideoRotation rotation, bool is_texture) : width(width), height(height), - rotation(rotation), is_texture(is_texture) {} int width; int height; - VideoRotation rotation; bool is_texture; int pixel_count() const { return width * height; } };