From 2a476e9c95b7ec402d44fc7023f268f5b0a0bfdf Mon Sep 17 00:00:00 2001 From: "mikhal@webrtc.org" Date: Fri, 28 Sep 2012 19:47:23 +0000 Subject: [PATCH] Switching scale functions to use VideoFrame. Review URL: https://webrtc-codereview.appspot.com/852004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2849 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/common_video/libyuv/include/scaler.h | 7 +- src/common_video/libyuv/scaler.cc | 40 +++++------- src/common_video/libyuv/scaler_unittest.cc | 64 +++++++++---------- src/modules/utility/source/frame_scaler.cc | 8 +-- .../codecs/test/videoprocessor.cc | 11 +--- .../main/source/spatial_resampler.cc | 10 +-- 6 files changed, 53 insertions(+), 87 deletions(-) diff --git a/src/common_video/libyuv/include/scaler.h b/src/common_video/libyuv/include/scaler.h index 40fa660380..8838844b65 100644 --- a/src/common_video/libyuv/include/scaler.h +++ b/src/common_video/libyuv/include/scaler.h @@ -27,8 +27,6 @@ enum ScaleMethod { kScaleBox }; -// TODO (mikhal): Have set return the expected value of the dst_frame, such -// that the user can allocate memory for Scale(). class Scaler { public: Scaler(); @@ -49,9 +47,8 @@ class Scaler { // Return value: 0 - OK, // -1 - parameter error // -2 - scaler not set - int Scale(const uint8_t* src_frame, - uint8_t*& dst_frame, - int& dst_size); + int Scale(const VideoFrame& src_frame, + VideoFrame* dst_frame); private: // Determine if the VideoTypes are currently supported. diff --git a/src/common_video/libyuv/scaler.cc b/src/common_video/libyuv/scaler.cc index b27c75cf32..1304e0dfa4 100644 --- a/src/common_video/libyuv/scaler.cc +++ b/src/common_video/libyuv/scaler.cc @@ -44,39 +44,33 @@ int Scaler::Set(int src_width, int src_height, return 0; } -int Scaler::Scale(const uint8_t* src_frame, - uint8_t*& dst_frame, - int& dst_size) { - if (src_frame == NULL) +int Scaler::Scale(const VideoFrame& src_frame, + VideoFrame* dst_frame) { + assert(dst_frame); + if (src_frame.Buffer() == NULL || src_frame.Length() == 0) return -1; if (!set_) return -2; - // Making sure that destination frame is of sufficient size - int dst_half_width = (dst_width_ + 1) >> 1; - int dst_half_height = (dst_height_ + 1) >> 1; - int required_dst_size = dst_width_ * dst_height_ + 2 * (dst_half_width * - dst_half_height); - if (dst_frame && required_dst_size > dst_size) { - // allocated buffer is too small - delete [] dst_frame; - dst_frame = NULL; - } - if (dst_frame == NULL) { - // TODO(mikhal): align this buffer to 16 bytes. - dst_frame = new uint8_t[required_dst_size]; - dst_size = required_dst_size; - } + // Making sure that destination frame is of sufficient size. + int required_dst_size = CalcBufferSize(kI420, dst_width_, dst_height_); + dst_frame->VerifyAndAllocate(required_dst_size); + // Set destination length and dimensions. + dst_frame->SetLength(required_dst_size); + dst_frame->SetWidth(dst_width_); + dst_frame->SetHeight(dst_height_); int src_half_width = (src_width_ + 1) >> 1; int src_half_height = (src_height_ + 1) >> 1; + int dst_half_width = (dst_width_ + 1) >> 1; + int dst_half_height = (dst_height_ + 1) >> 1; // Converting to planes: - const uint8_t* src_yplane = src_frame; - const uint8_t* src_uplane = src_frame + src_width_ * src_height_; + const uint8_t* src_yplane = src_frame.Buffer(); + const uint8_t* src_uplane = src_yplane + src_width_ * src_height_; const uint8_t* src_vplane = src_uplane + src_half_width * src_half_height; - uint8_t* dst_yplane = dst_frame; - uint8_t* dst_uplane = dst_frame + dst_width_ * dst_height_; + uint8_t* dst_yplane = dst_frame->Buffer(); + uint8_t* dst_uplane = dst_yplane + dst_width_ * dst_height_; uint8_t* dst_vplane = dst_uplane + dst_half_width * dst_half_height; return libyuv::I420Scale(src_yplane, src_width_, diff --git a/src/common_video/libyuv/scaler_unittest.cc b/src/common_video/libyuv/scaler_unittest.cc index f6ee15d375..b8e7a305b0 100644 --- a/src/common_video/libyuv/scaler_unittest.cc +++ b/src/common_video/libyuv/scaler_unittest.cc @@ -39,7 +39,7 @@ class TestScaler : public ::testing::Test { Scaler test_scaler_; FILE* source_file_; - uint8_t* test_buffer_; + VideoFrame test_frame_; const int width_; const int height_; const int frame_length_; @@ -60,7 +60,8 @@ void TestScaler::SetUp() { source_file_ = fopen(input_file_name.c_str(), "rb"); ASSERT_TRUE(source_file_ != NULL) << "Cannot read file: "<< input_file_name << "\n"; - test_buffer_ = new uint8_t[frame_length_]; + test_frame_.VerifyAndAllocate(frame_length_); + test_frame_.SetLength(frame_length_); } void TestScaler::TearDown() { @@ -68,12 +69,11 @@ void TestScaler::TearDown() { ASSERT_EQ(0, fclose(source_file_)); } source_file_ = NULL; - delete [] test_buffer_; + test_frame_.Free(); } TEST_F(TestScaler, ScaleWithoutSettingValues) { - int size = 100; - EXPECT_EQ(-2, test_scaler_.Scale(test_buffer_, test_buffer_, size)); + EXPECT_EQ(-2, test_scaler_.Scale(test_frame_, &test_frame_)); } TEST_F(TestScaler, ScaleBadInitialValues) { @@ -85,19 +85,22 @@ TEST_F(TestScaler, ScaleBadInitialValues) { } TEST_F(TestScaler, ScaleSendingNullSourcePointer) { - int size = 0; - EXPECT_EQ(-1, test_scaler_.Scale(NULL, test_buffer_, size)); + VideoFrame null_src_frame; + EXPECT_EQ(-1, test_scaler_.Scale(null_src_frame, &test_frame_)); } TEST_F(TestScaler, ScaleSendingBufferTooSmall) { // Sending a buffer which is too small (should reallocate and update size) EXPECT_EQ(0, test_scaler_.Set(352, 288, 144, 288, kI420, kI420, kScalePoint)); - uint8_t* test_buffer2 = NULL; - int size = 0; - EXPECT_GT(fread(test_buffer_, 1, frame_length_, source_file_), 0U); - EXPECT_EQ(0, test_scaler_.Scale(test_buffer_, test_buffer2, size)); - EXPECT_EQ(144 * 288 * 3 / 2, size); - delete [] test_buffer2; + VideoFrame test_frame2; + EXPECT_GT(fread(test_frame_.Buffer(), 1, frame_length_, source_file_), 0U); + EXPECT_EQ(0, test_scaler_.Scale(test_frame_, &test_frame2)); + EXPECT_EQ(CalcBufferSize(kI420, 144, 288), + static_cast(test_frame2.Size())); + EXPECT_EQ(144u, test_frame2.Width()); + EXPECT_EQ(288u, test_frame2.Height()); + EXPECT_EQ(CalcBufferSize(kI420, 144, 288), + static_cast(test_frame2.Length())); } //TODO (mikhal): Converge the test into one function that accepts the method. @@ -378,9 +381,7 @@ double TestScaler::ComputeAvgSequencePSNR(FILE* input_file, rewind(input_file); rewind(output_file); - int half_width = (width + 1) >> 1; - int half_height = (height + 1) >> 1; - int required_size = height * width + 2 * (half_width * half_height); + int required_size = CalcBufferSize(kI420, width, height); uint8_t* input_buffer = new uint8_t[required_size]; uint8_t* output_buffer = new uint8_t[required_size]; @@ -421,35 +422,30 @@ void TestScaler::ScaleSequence(ScaleMethod method, rewind(source_file); - int src_half_width = (src_width + 1) >> 1; - int src_half_height = (src_height + 1) >> 1; - int dst_half_width = (dst_width + 1) >> 1; - int dst_half_height = (dst_height + 1) >> 1; + int out_required_size = CalcBufferSize(kI420, dst_width, dst_height); + int in_required_size = CalcBufferSize(kI420, src_width, src_height); - int out_required_size = dst_width * dst_height + 2 * (dst_half_width * - dst_half_height); - int in_required_size = src_height * src_width + 2 * (src_half_width * - src_half_height); - - uint8_t* input_buffer = new uint8_t[in_required_size]; - uint8_t* output_buffer = new uint8_t[out_required_size]; + VideoFrame input_frame, output_frame; + input_frame.VerifyAndAllocate(in_required_size); + input_frame.SetLength(in_required_size); + output_frame.VerifyAndAllocate(out_required_size); + output_frame.SetLength(out_required_size); int64_t start_clock, total_clock; total_clock = 0; int frame_count = 0; - // Running through entire sequence + // Running through entire sequence. while (feof(source_file) == 0) { if ((size_t)in_required_size != - fread(input_buffer, 1, in_required_size, source_file)) + fread(input_frame.Buffer(), 1, in_required_size, source_file)) break; start_clock = TickTime::MillisecondTimestamp(); - EXPECT_EQ(0, test_scaler_.Scale(input_buffer, output_buffer, - out_required_size)); + EXPECT_EQ(0, test_scaler_.Scale(input_frame, &output_frame)); total_clock += TickTime::MillisecondTimestamp() - start_clock; - if (fwrite(output_buffer, 1, out_required_size, - output_file) != static_cast(out_required_size)) { + if (fwrite(output_frame.Buffer(), 1, output_frame.Size(), + output_file) != static_cast(output_frame.Size())) { return; } frame_count++; @@ -462,8 +458,6 @@ void TestScaler::ScaleSequence(ScaleMethod method, (static_cast(total_clock) / frame_count)); } ASSERT_EQ(0, fclose(output_file)); - delete [] input_buffer; - delete [] output_buffer; } } // namespace webrtc diff --git a/src/modules/utility/source/frame_scaler.cc b/src/modules/utility/source/frame_scaler.cc index c012e892ea..048792b3ca 100644 --- a/src/modules/utility/source/frame_scaler.cc +++ b/src/modules/utility/source/frame_scaler.cc @@ -35,17 +35,11 @@ int FrameScaler::ResizeFrameIfNeeded(VideoFrame* video_frame, // Set correct scale settings and scale |video_frame| into |scaled_frame_|. scaler_->Set(video_frame->Width(), video_frame->Height(), out_width, out_height, kI420, kI420, kScaleBox); - int out_length = CalcBufferSize(kI420, out_width, out_height); - scaled_frame_.VerifyAndAllocate(out_length); - int ret = scaler_->Scale(video_frame->Buffer(), scaled_frame_.Buffer(), - out_length); + int ret = scaler_->Scale(*video_frame, &scaled_frame_); if (ret < 0) { return ret; } - scaled_frame_.SetWidth(out_width); - scaled_frame_.SetHeight(out_height); - scaled_frame_.SetLength(out_length); scaled_frame_.SetRenderTime(video_frame->RenderTimeMs()); scaled_frame_.SetTimeStamp(video_frame->TimeStamp()); video_frame->SwapFrame(scaled_frame_); diff --git a/src/modules/video_coding/codecs/test/videoprocessor.cc b/src/modules/video_coding/codecs/test/videoprocessor.cc index 750d990096..fadc1e8980 100644 --- a/src/modules/video_coding/codecs/test/videoprocessor.cc +++ b/src/modules/video_coding/codecs/test/videoprocessor.cc @@ -293,15 +293,7 @@ void VideoProcessorImpl::FrameDecoded(const VideoFrame& image) { // upsample back to original size: needed for PSNR and SSIM computations. if (image.Width() != config_.codec_settings->width || image.Height() != config_.codec_settings->height) { - int required_size = CalcBufferSize(kI420, - config_.codec_settings->width, - config_.codec_settings->height); VideoFrame up_image; - up_image.VerifyAndAllocate(required_size); - up_image.SetLength(required_size); - up_image.SetWidth(config_.codec_settings->width); - up_image.SetHeight(config_.codec_settings->height); - int ret_val = scaler_.Set(image.Width(), image.Height(), config_.codec_settings->width, config_.codec_settings->height, @@ -311,8 +303,7 @@ void VideoProcessorImpl::FrameDecoded(const VideoFrame& image) { fprintf(stderr, "Failed to set scalar for frame: %d, return code: %d\n", frame_number, ret_val); } - ret_val = scaler_.Scale(image.Buffer(), up_image.Buffer(), - required_size); + ret_val = scaler_.Scale(image, &up_image); assert(ret_val >= 0); if (ret_val < 0) { fprintf(stderr, "Failed to scale frame: %d, return code: %d\n", diff --git a/src/modules/video_processing/main/source/spatial_resampler.cc b/src/modules/video_processing/main/source/spatial_resampler.cc index f4c4415505..0d7c2e592b 100644 --- a/src/modules/video_processing/main/source/spatial_resampler.cc +++ b/src/modules/video_processing/main/source/spatial_resampler.cc @@ -82,16 +82,12 @@ VPMSimpleSpatialResampler::ResampleFrame(const VideoFrame& inFrame, if (retVal < 0) return retVal; - // Disabling cut/pad for now - only scaling. - int requiredSize = CalcBufferSize(kI420, _targetWidth, _targetHeight); - outFrame.VerifyAndAllocate(requiredSize); + // Setting time parameters to the output frame - all the rest will be + // set by the scaler. outFrame.SetTimeStamp(inFrame.TimeStamp()); outFrame.SetRenderTime(inFrame.RenderTimeMs()); - outFrame.SetWidth(_targetWidth); - outFrame.SetHeight(_targetHeight); - retVal = _scaler.Scale(inFrame.Buffer(), outFrame.Buffer(), requiredSize); - outFrame.SetLength(requiredSize); + retVal = _scaler.Scale(inFrame, &outFrame); if (retVal == 0) return VPM_OK; else