Refactor scaling.

webrtc/modules/video_coding/codecs/test/videoprocessor.cc

Index: webrtc/modules/video_coding/codecs/test/videoprocessor.cc
diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor.cc b/webrtc/modules/video_coding/codecs/test/videoprocessor.cc
index 9a9a0ddf16576e9232bdd9998c1cfe53bb5d1a6d..57bfcff7386ce8f9bda204d5bcfb568ae520f96d 100644
--- a/webrtc/modules/video_coding/codecs/test/videoprocessor.cc
+++ b/webrtc/modules/video_coding/codecs/test/videoprocessor.cc
@@ -66,8 +66,7 @@ VideoProcessorImpl::VideoProcessorImpl(webrtc::VideoEncoder* encoder,
       num_dropped_frames_(0),
       num_spatial_resizes_(0),
       last_encoder_frame_width_(0),
-      last_encoder_frame_height_(0),
-      scaler_() {
+      last_encoder_frame_height_(0) {
   assert(encoder);
   assert(decoder);
   assert(frame_reader);
@@ -335,23 +334,19 @@ 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) {
-    VideoFrame up_image;
-    int ret_val = scaler_.Set(
-        image.width(), image.height(), config_.codec_settings->width,
-        config_.codec_settings->height, kI420, kI420, kScaleBilinear);
-    assert(ret_val >= 0);
-    if (ret_val < 0) {
-      fprintf(stderr, "Failed to set scalar for frame: %d, return code: %d\n",
-              frame_number, ret_val);
-    }
-    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",
-              frame_number, ret_val);
-    }
+    rtc::scoped_refptr<I420Buffer> up_image(
+        new rtc::RefCountedObject<I420Buffer>(config_.codec_settings->width,
+                                              config_.codec_settings->height));
+
+    // Should be the same aspect ation, no cropping needed.

[pbos-webrtc, 2016/05/31 14:14:02]

ratio

[nisse-webrtc, 2016/06/01 09:15:18]

Done.

+    RTC_DCHECK_EQ(image.width() * config_.codec_settings->height,
+                  image.height() * config_.codec_settings->width);
+
+    up_image->CropAndScale(image.video_frame_buffer(), 0, 0,

[pbos-webrtc, 2016/05/31 14:14:02]

Should this be the center crop?

[nisse-webrtc, 2016/06/01 09:15:18]

This code used to use the Scaler class, which always does a center crop if
needed to change the aspect ratio. 

But as far as I understand, this piece of code should always have the same
aspect ratio? (Possibly with rounding error, which would trigger the DCHECK
above).

From my reading of the comments here, I guess the frame was scaled down before
encoding to get better quality for available bitrate, and here it is scaled back
to the original size after decoding. Right?

[pbos-webrtc, 2016/06/01 15:55:41]

I just meant that the center crop method has easier-to-read arguments, does that
make sense?

[nisse-webrtc, 2016/06/02 09:00:06]

I see you point, but it's a bit strange to call a function which tries to do
more work, just because it has fewer arguments...

Would it make more sense with an additional convenience wrapper
I420Buffer::Scale? It would be used here and in androidvideocapturer_jni.cc.

And do you agree that this call site doesn't want any cropping?

+                           image.width(), image.height());
     // TODO(mikhal): Extracting the buffer for now - need to update test.
-    size_t length = CalcBufferSize(kI420, up_image.width(), up_image.height());
+    size_t length =
+        CalcBufferSize(kI420, up_image->width(), up_image->height());
     std::unique_ptr<uint8_t[]> image_buffer(new uint8_t[length]);
     int extracted_length = ExtractBuffer(up_image, length, image_buffer.get());
     assert(extracted_length > 0);