Refactor scaling.

Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
R=pbos@webrtc.org, perkj@webrtc.org, stefan@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/718a763d5924d5a9948a4e5ecbf0fde1da11129f

[nisse-webrtc, 2016-05-27 08:34:55]

nisse@webrtc.org changed reviewers:
+ magjed@webrtc.org, pbos@webrtc.org

[nisse-webrtc, 2016-05-27 08:34:55]

This cl consolidates the scaling code abit. Get rid of the Stretch* methods (one
of the blockers for removal of cricket::VideoFrame), and the Scaler class.

Who else should review this, it touches quite a lot of code?

We should remove MutableData from VideoFrameBuffer. Ideally, it should be
non-public also in I420Buffer, but there's always going to be obscure mutable
operations that doesn't deserve to be there own methods, so we probably so we
can't do that.

For the new methods, my reasoning was that I420Buffer::CropAndScale is a
sufficiently generally useful operation to deserve to be a regular method, and
it needs to access MutableData directly.

I420CenterCropAndScale is a convenience helper, and doesn't need to call
MutablaData directly, so it's defined as a static method. 

Actually, it doesn't need to be a "method" at all, but I think it is reasonable
to put it in the I420Buffer namespace. If possible in C++, I'd like to get it
into the I420Buffer:: name space, but only allow it to access public fields and
methods.

Feel free to suggest other rules, I don't have a very strong opinion on static
vs non-static here.

[nisse-webrtc, 2016-05-27 08:35:23]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 08:35:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2020593002/1

[commit-bot: I haz the power, 2016-05-27 08:40:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-27 08:40:04]

Dry run: Try jobs failed on following builders:
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/10150)
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/15365)

[nisse-webrtc, 2016-05-27 11:33:51]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-27 11:33:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2020593002/80001

[commit-bot: I haz the power, 2016-05-27 12:32:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-27 12:32:06]

Dry run: This issue passed the CQ dry run.

[magjed_webrtc, 2016-05-30 11:58:27]

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/api/java/jni/andro...
File webrtc/api/java/jni/androidvideocapturer_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidvideocapturer_jni.cc:221:
scaled->CropAndScale(buffer, 0, 0, rotated_width, rotated_height);
You should use crop_x/crop_y instead of 0?

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/inclu...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/inclu...
webrtc/common_video/include/video_frame_buffer.h:208:
rtc::scoped_refptr<VideoFrameBuffer> ShallowCenterCrop(
Is this unused now?

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/video...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/video...
webrtc/common_video/video_frame_buffer.cc:240: RTC_DCHECK(res == 0);
Use RTC_DCHECK_EQ instead

[nisse-webrtc, 2016-05-30 12:58:22]

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/api/java/jni/andro...
File webrtc/api/java/jni/androidvideocapturer_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidvideocapturer_jni.cc:221:
scaled->CropAndScale(buffer, 0, 0, rotated_width, rotated_height);
On 2016/05/30 11:58:26, magjed_webrtc wrote:
> You should use crop_x/crop_y instead of 0?

I don't think so. Cropping was done in the above call to NV12ToI420Rotate, so
here we should scale the full image.

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/inclu...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/inclu...
webrtc/common_video/include/video_frame_buffer.h:208:
rtc::scoped_refptr<VideoFrameBuffer> ShallowCenterCrop(
On 2016/05/30 11:58:26, magjed_webrtc wrote:
> Is this unused now?

It is! It became unused in the previous cl,
https://codereview.webrtc.org/1973873003/

I'll delete it.

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/video...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2020593002/diff/80001/webrtc/common_video/video...
webrtc/common_video/video_frame_buffer.cc:240: RTC_DCHECK(res == 0);
On 2016/05/30 11:58:26, magjed_webrtc wrote:
> Use RTC_DCHECK_EQ instead

Done.

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

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidmediaencoder_jni.cc:692:
quality_scaler_.GetScaledFrame(frame.video_frame_buffer()));
GetScaledVideoFrameBuffer?

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:117: void CropAndScale(const
rtc::scoped_refptr<VideoFrameBuffer>& buffer,
const VideoFrameBuffer& src?

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:126: const
rtc::scoped_refptr<I420Buffer>& dst,
Should this be a I420Buffer* dst and const VideoFrameBuffer& src? No need to
have the refptrs in the interface, is there?

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
File webrtc/common_video/libyuv/include/webrtc_libyuv.h (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
webrtc/common_video/libyuv/include/webrtc_libyuv.h:96: int ExtractBuffer(const
rtc::scoped_refptr<VideoFrameBuffer>& input_frame,
const VideoFrameBuffer& src

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
webrtc/common_video/libyuv/webrtc_libyuv.cc:149: if (!input_frame)
Seems to me like all of these could be DCHECKs, I don't see us needing a return
code for this function

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:341: // Should be the
same aspect ation, no cropping needed.
ratio

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:345:
up_image->CropAndScale(image.video_frame_buffer(), 0, 0,
Should this be the center crop?

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:11: // TODO(nisse): Switch
to RTC_CHECK (or RTC_DCHECK) ?
Do that now, please. Pref DCHECK. :)

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_pro...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_pro...
webrtc/modules/video_processing/spatial_resampler.cc:57: new
rtc::RefCountedObject<I420Buffer>(target_width_, target_height_));
This doesn't reuse buffers anymore. Might apply to the other video_processing
code, but this should use a pool.

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

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidmediaencoder_jni.cc:692:
quality_scaler_.GetScaledFrame(frame.video_frame_buffer()));
On 2016/05/31 14:14:01, pbos-webrtc wrote:
> GetScaledVideoFrameBuffer?

If you really want, I can rename the quality scaler method. To me, the exact
type used to represent a frame doesn't seem central to what this class and
method does.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:117: void CropAndScale(const
rtc::scoped_refptr<VideoFrameBuffer>& buffer,
On 2016/05/31 14:14:01, pbos-webrtc wrote:
> const VideoFrameBuffer& src?

I had a brief discussion with Magnus and Tommi, and Tommi favored using refptr
everywhere.

The case where we really need refptr is where we might return the input object,
e.g., QualityScaler::GetScaledFrame.
 
But I'd really like if input parameters could be properly const, i.e.,

const rtc::scoped_refptr<const VideoFrameBuffer>& buffer
                         ^^^^^

Does that work, or need some silly conversion operator in the scoped_reftpr
template machinery?

Maybe we should discuss this face-to-face, I'm happy to change to raw pointers
anywhere people agree it is appropriate.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
webrtc/common_video/libyuv/webrtc_libyuv.cc:149: if (!input_frame)
On 2016/05/31 14:14:01, pbos-webrtc wrote:
> Seems to me like all of these could be DCHECKs, I don't see us needing a
return
> code for this function

Agree. Not fixed yet.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:341: // Should be the
same aspect ation, no cropping needed.
On 2016/05/31 14:14:02, pbos-webrtc wrote:
> ratio

Done.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:345:
up_image->CropAndScale(image.video_frame_buffer(), 0, 0,
On 2016/05/31 14:14:02, pbos-webrtc wrote:
> Should this be the center crop?

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?

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc
(right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc:528:
if (remove(config_.output_filename.c_str()) < 0) {
This is a very unrelated bugfix, I was annoyed by the message.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:11: // TODO(nisse): Switch
to RTC_CHECK (or RTC_DCHECK) ?
On 2016/05/31 14:14:02, pbos-webrtc wrote:
> Do that now, please. Pref DCHECK. :)

Done.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_pro...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_pro...
webrtc/modules/video_processing/spatial_resampler.cc:57: new
rtc::RefCountedObject<I420Buffer>(target_width_, target_height_));
On 2016/05/31 14:14:02, pbos-webrtc wrote:
> This doesn't reuse buffers anymore. Might apply to the other video_processing
> code, but this should use a pool.

Good catch. Added a pool.

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

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidmediaencoder_jni.cc:692:
quality_scaler_.GetScaledFrame(frame.video_frame_buffer()));
On 2016/06/01 09:15:17, nisse-webrtc wrote:
> On 2016/05/31 14:14:01, pbos-webrtc wrote:
> > GetScaledVideoFrameBuffer?
> 
> If you really want, I can rename the quality scaler method. To me, the exact
> type used to represent a frame doesn't seem central to what this class and
> method does.

No but I think you should know what you get out of it. GetScaledBuffer would
also work for me, but you don't get a VideoFrame, so GetScaledFrame sounds
weird.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:345:
up_image->CropAndScale(image.video_frame_buffer(), 0, 0,
On 2016/06/01 09:15:18, nisse-webrtc wrote:
> On 2016/05/31 14:14:02, pbos-webrtc wrote:
> > Should this be the center crop?
> 
> 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?

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

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:100:
RTC_DCHECK(low_qp_threshold_ >= 0);
GE

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:101:
RTC_DCHECK(num_samples_upscale_ > 0);
GT

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:102:
RTC_DCHECK(num_samples_downscale_ > 0);
GT

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:141:
RTC_DCHECK(downscale_shift_ >= 0);
GE

[nisse-webrtc, 2016-06-02 08:13:34]

Description was changed from

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
==========

to

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
==========

[nisse-webrtc, 2016-06-02 08:13:34]

nisse@webrtc.org changed reviewers:
+ perkj@webrtc.org

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

Renamed GetScaledFrame --> GetScaledBuffer, and changed RTC_DCHECK -->
RTC_DCHECK_G* as suggested.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidmediaencoder_jni.cc:692:
quality_scaler_.GetScaledFrame(frame.video_frame_buffer()));
On 2016/06/01 15:55:41, pbos-webrtc wrote:
> On 2016/06/01 09:15:17, nisse-webrtc wrote:
> > On 2016/05/31 14:14:01, pbos-webrtc wrote:
> > > GetScaledVideoFrameBuffer?
> > 
> > If you really want, I can rename the quality scaler method. To me, the exact
> > type used to represent a frame doesn't seem central to what this class and
> > method does.
> 
> No but I think you should know what you get out of it. GetScaledBuffer would
> also work for me, but you don't get a VideoFrame, so GetScaledFrame sounds
> weird.

Done, renamed to GetScaledBuffer.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/common_video/liby...
webrtc/common_video/libyuv/webrtc_libyuv.cc:149: if (!input_frame)
On 2016/06/01 09:15:18, nisse-webrtc wrote:
> On 2016/05/31 14:14:01, pbos-webrtc wrote:
> > Seems to me like all of these could be DCHECKs, I don't see us needing a
> return
> > code for this function
> 
> Agree. Not fixed yet.

Actually, return value is kind-of needed, since it's the size of the data
written.

But looking at the users of this function, it would make some sense to delete
both return value and the size argument, and leave it to the caller to know that
it writes exactly CalcBufferSize bytes. But maybe not in this cl? 

Could convert some of the checks to RTC_CHECK (callers don't recover from
failures anyway), but I'm leaving it unchanged for the moment.

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/100001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:345:
up_image->CropAndScale(image.video_frame_buffer(), 0, 0,
On 2016/06/01 15:55:41, pbos-webrtc wrote:
> On 2016/06/01 09:15:18, nisse-webrtc wrote:
> > On 2016/05/31 14:14:02, pbos-webrtc wrote:
> > > Should this be the center crop?
> > 
> > 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?
> 
> I just meant that the center crop method has easier-to-read arguments, does
that
> make sense?

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?

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/120001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:100:
RTC_DCHECK(low_qp_threshold_ >= 0);
On 2016/06/01 15:55:41, pbos-webrtc wrote:
> GE

Done.

[perkj_webrtc, 2016-06-02 09:25:36]

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:56: double rel_width,
what is rel_width hight?

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:116: // Crop the input buffer,
then scale it to the size of this buffer.
Can we rephrase this to make it easier to understand that this modifies |this|
and not |buffer|?

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:120: int crop_width,
When are these arguments usable? It seems like you use this by creating the
destination buffer first and then crop/scale to dst from src=buffer.
Can we just skip them and have this as CropAndScaleFrom(const
rtc::scoped_refptr<VideoFrameBuffer>& buffer) ?

[nisse-webrtc, 2016-06-02 12:17:50]

(No new patchset now, only comments).

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:56: double rel_width,
On 2016/06/02 09:25:36, perkj_webrtc wrote:
> what is rel_width hight?

I'm adding a comment. They describe the size of the area cropped out from the
original gradient image, with 1.0 meaning no cropping along that dimension.

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:116: // Crop the input buffer,
then scale it to the size of this buffer.
On 2016/06/02 09:25:36, perkj_webrtc wrote:
> Can we rephrase this to make it easier to understand that this modifies |this|
> and not |buffer|? 

I'm trying rephrasing the comment, and renaming the argument buffer --> src.

Do you want me to rename it to CropAndScaleFrom too?

https://codereview.webrtc.org/2020593002/diff/140001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:120: int crop_width,
On 2016/06/02 09:25:36, perkj_webrtc wrote:
> When are these arguments usable? It seems like you use this by creating the
> destination buffer first and then crop/scale to dst from src=buffer.
> Can we just skip them and have this as CropAndScaleFrom(const
> rtc::scoped_refptr<VideoFrameBuffer>& buffer) ? 

To use this, one first creates a destination buffer of the desired size, using
either the new operator or I420BufferPool::CreateBuffer. And then one calls this
function with the src buffer and cropping parameters, the latter typically
chosen to get the same scale factor in both dimensions. 

We could drop cropping parameters and always to a center crop when needed to
avoid distorting the image. The reason I have them, is that I don't think this
is the right place to decide which portion of the image should be left out.

We discussed this a bit more off-line. It will simplify the interface if we let
this method decide on the appropriate cropping. However, the logic in
VideoAdapter (added by magjed) a few weeks back, is a bit hairy. And after a
second look, it tweaks both crop area size and destination size if needed to get
the scale factor exactly as desired. And this method really can't tweak the
destination size, and any caller using a buffer pool also need to know the
destination size exactly.

So I'd prefer to keep the crop_width and crop_height input arguments.

It is easier to get rid of the offset_x and offset_y, if desired. They can be
implied as (src->width() - crop_width) / 2. To me, it makes sense to have all
the crop parameters decided at the same place, but I don't have a very strong
opinion about that; currently nothing but center crop is ever used.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:135:
I420Buffer::CenterCropAndScale(scaled, frame);
Looking closer, I don't think this should ever do any cropping. Callers should
always call OnEncodeFrame followed by GetScaledFrame (why aren't those combined
into a single public method, BTW?), and then scaling is always by a power of
two, and same in both dimensions.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_pro...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_pro...
webrtc/modules/video_processing/spatial_resampler.cc:59:
I420Buffer::CenterCropAndScale(scaled, inFrame.video_frame_buffer());
Not sure if this call needs cropping? The target size seems to come from
ViEEncoder::SetEncoder, set from video_codec.width and .height. Can that use a
different aspect ratio than incoming frames?

Scaling appears to be unaware of the scale factors preferred by libyuv?

[perkj_webrtc, 2016-06-02 13:43:08]

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidvideocapturer_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidvideocapturer_jni.cc:220:
scaled->CropAndScale(buffer, 0, 0, rotated_width, rotated_height);
please use buffer->width() and buffer->height()

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:65: // for initialization.
for ...{
  for {
  }
}

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:112: // Only the most generally
useful operations should be methods on
I don't think this comment belong here. please remove.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:116: // Scale the cropped area
of |src|, writing into this buffer.
Please comment that the result width and height will be the size of the this
buffer.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:125: static void
CenterCropAndScale(
I would prefer this to be just  
void CropAndScale(const rtc::scoped_refptr<VideoFrameBuffer>& src). There is no
need for this to be static.

[nisse-webrtc, 2016-06-03 13:55:02]

I've addressed Per's comments. I've also added another convenience method,
I420Buffer::Scale, used at the call sites where no cropping logic is needed
(because the aspect ratio is unchanged).

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/api/java/jni/andr...
File webrtc/api/java/jni/androidvideocapturer_jni.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/api/java/jni/andr...
webrtc/api/java/jni/androidvideocapturer_jni.cc:220:
scaled->CropAndScale(buffer, 0, 0, rotated_width, rotated_height);
On 2016/06/02 13:43:08, perkj_webrtc wrote:
> please use buffer->width() and buffer->height()

Done.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:65: // for initialization.
On 2016/06/02 13:43:08, perkj_webrtc wrote:
> for ...{
>   for {
>   }
> }

Done.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:112: // Only the most generally
useful operations should be methods on
On 2016/06/02 13:43:08, perkj_webrtc wrote:
> I don't think this comment belong here. please remove. 

Done.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:116: // Scale the cropped area
of |src|, writing into this buffer.
On 2016/06/02 13:43:08, perkj_webrtc wrote:
> Please comment that the result width and height will be the size of the this
> buffer.

Done.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:125: static void
CenterCropAndScale(
On 2016/06/02 13:43:08, perkj_webrtc wrote:
> I would prefer this to be just  
> void CropAndScale(const rtc::scoped_refptr<VideoFrameBuffer>& src). There is
no
> need for this to be static.

Done.

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2020593002/diff/160001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.cc:135:
I420Buffer::CenterCropAndScale(scaled, frame);
On 2016/06/02 12:17:50, nisse-webrtc wrote:
> Looking closer, I don't think this should ever do any cropping. Callers should
> always call OnEncodeFrame followed by GetScaledFrame (why aren't those
combined
> into a single public method, BTW?), and then scaling is always by a power of
> two, and same in both dimensions.

So I'm dropping croppign logic here.

[pbos-webrtc, 2016-06-03 14:53:23]

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:55: // the original (gradient)
frame, in relative coordinates where
Put comment outside argument list

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:315: scaled->Scale(buf);
scaled_buffer please

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:114: void CropAndScale(const
rtc::scoped_refptr<VideoFrameBuffer>& src,
I think ScaleFrom is a better name here and below, since it more clearly conveys
what goes where.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:341: // Should be the
same aspect ratio, no cropping needed.
I still think the DCHECKs are good here.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_pro...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_pro...
webrtc/modules/video_processing/spatial_resampler.cc:56:
rtc::scoped_refptr<I420Buffer> scaled(buffer_pool_.CreateBuffer
scaled_buffer

[nisse-webrtc, 2016-06-09 08:10:06]

Not sure what to do about the DCHECKs, it would make some sense with a DCHECK
saying that aspect ratio is the same except for rounding errors when scaling
down, but I'm not sure how to express that in a way which is correct, concise
and readable.

I've also changed some uses of scoped_refptr to use initialization rather than
assignment.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
File webrtc/common_video/i420_video_frame_unittest.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:55: // the original (gradient)
frame, in relative coordinates where
On 2016/06/03 14:53:22, pbos-webrtc wrote:
> Put comment outside argument list

Done.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/i420...
webrtc/common_video/i420_video_frame_unittest.cc:315: scaled->Scale(buf);
On 2016/06/03 14:53:22, pbos-webrtc wrote:
> scaled_buffer please

Done.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:114: void CropAndScale(const
rtc::scoped_refptr<VideoFrameBuffer>& src,
On 2016/06/03 14:53:23, pbos-webrtc wrote:
> I think ScaleFrom is a better name here and below, since it more clearly
conveys
> what goes where.

Done.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/codecs/test/videoprocessor.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_cod...
webrtc/modules/video_coding/codecs/test/videoprocessor.cc:341: // Should be the
same aspect ratio, no cropping needed.
On 2016/06/03 14:53:23, pbos-webrtc wrote:
> I still think the DCHECKs are good here.

Hmm. I'm afraid the DCHECKs in the earlier patchset were too strict, and will
trigger incorrectly if the original size is odd.

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_pro...
File webrtc/modules/video_processing/spatial_resampler.cc (right):

https://codereview.webrtc.org/2020593002/diff/180001/webrtc/modules/video_pro...
webrtc/modules/video_processing/spatial_resampler.cc:56:
rtc::scoped_refptr<I420Buffer> scaled(buffer_pool_.CreateBuffer
On 2016/06/03 14:53:23, pbos-webrtc wrote:
> scaled_buffer

Done.

[perkj_webrtc, 2016-06-10 09:01:24]

There is an EXPECT_NEAR macro you can consider. 

Nice cleanup.
lgtm

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/moving_average.h (right):

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/moving_average.h:14: #include <stddef.h>
why this  now? git cl lint?

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.h (right):

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.h:15: #include
"webrtc/common_video/include/i420_buffer_pool.h"
wrong include order. Should be in alphabetical order.

[nisse-webrtc, 2016-06-10 11:36:57]

There's no _NEAR macro in the DCHECK family, as far as I can find.

pbos, do you have any additional comments?

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/moving_average.h (right):

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/moving_average.h:14: #include <stddef.h>
On 2016/06/10 09:01:24, perkj_webrtc wrote:
> why this  now? git cl lint? 

I got a compilation error from one of the bots (possibly windows, I don't
remember now) about size_t being undefined.

Most likely it happened now because moving_average became the first include in
quality_scaler.h. It could perhaps be remoevd when I reorder the includes as you
suggest in your other comment.

But it seems reasonable to include stddef.h in every header file using size_t
(and not including anything else which surely should define size_t).

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
File webrtc/modules/video_coding/utility/quality_scaler.h (right):

https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
webrtc/modules/video_coding/utility/quality_scaler.h:15: #include
"webrtc/common_video/include/i420_buffer_pool.h"
On 2016/06/10 09:01:24, perkj_webrtc wrote:
> wrong include order. Should be in alphabetical order.

Done.

[pbos-webrtc, 2016-06-10 11:55:11]

On 2016/06/10 11:36:57, nisse-webrtc wrote:
> There's no _NEAR macro in the DCHECK family, as far as I can find.
> 
> pbos, do you have any additional comments?
> 
>
https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
> File webrtc/modules/video_coding/utility/moving_average.h (right):
> 
>
https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
> webrtc/modules/video_coding/utility/moving_average.h:14: #include <stddef.h>
> On 2016/06/10 09:01:24, perkj_webrtc wrote:
> > why this  now? git cl lint? 
> 
> I got a compilation error from one of the bots (possibly windows, I don't
> remember now) about size_t being undefined.
> 
> Most likely it happened now because moving_average became the first include in
> quality_scaler.h. It could perhaps be remoevd when I reorder the includes as
you
> suggest in your other comment.
> 
> But it seems reasonable to include stddef.h in every header file using size_t
> (and not including anything else which surely should define size_t).
> 
>
https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
> File webrtc/modules/video_coding/utility/quality_scaler.h (right):
> 
>
https://codereview.webrtc.org/2020593002/diff/200001/webrtc/modules/video_cod...
> webrtc/modules/video_coding/utility/quality_scaler.h:15: #include
> "webrtc/common_video/include/i420_buffer_pool.h"
> On 2016/06/10 09:01:24, perkj_webrtc wrote:
> > wrong include order. Should be in alphabetical order.
> 
> Done.

Nothing good, you could check and see if they're within 5% of eachother using
DCHECK_GE or something with absolute values, but that sounds tricky. Up to you
whether to do something.

lgtm

[nisse-webrtc, 2016-06-10 12:01:14]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-06-10 12:01:15]

The patchset sent to the CQ was uploaded after l-g-t-m from perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2020593002/#ps220001 (title:
"Fix include order.")

[commit-bot: I haz the power, 2016-06-10 12:01:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/220001

[commit-bot: I haz the power, 2016-06-10 12:02:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 12:02:36]

Try jobs failed on following builders:
  android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)
  ios64_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_dbg/builds/435)
  ios64_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_rel/builds/437)
  ios_api_framework on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_api_framework/build...)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/10500)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/14370)
  linux_ubsan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/2742)
  linux_ubsan_vptr on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/3881)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/10101)

[nisse-webrtc, 2016-06-10 13:00:18]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-06-10 13:00:19]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2020593002/#ps240001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-06-10 13:00:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/240001

[commit-bot: I haz the power, 2016-06-10 13:02:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 13:02:36]

Try jobs failed on following builders:
  linux_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/11600)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/3882)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/10102)
  win_x64_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_gn_dbg/builds/1...)

[nisse-webrtc, 2016-06-13 06:58:56]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-13 06:59:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/260001

[nisse-webrtc, 2016-06-13 07:01:23]

nisse@webrtc.org changed reviewers:
+ holmer@chromium.org

[nisse-webrtc, 2016-06-13 07:01:24]

Stefan, can you have a look? In particular, I need your approval for the changes
under modules/video_processing.

[commit-bot: I haz the power, 2016-06-13 07:13:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-13 07:13:52]

Dry run: Try jobs failed on following builders:
  android_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14117)

[stefan-webrtc, 2016-06-13 08:51:48]

stefan@webrtc.org changed reviewers:
+ stefan@webrtc.org

[stefan-webrtc, 2016-06-13 08:51:49]

lgtm for video_processing

[nisse-webrtc, 2016-06-13 08:58:25]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-06-13 08:58:26]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2020593002/#ps260001 (title:
"Drop scaler_unitttest.cc from GN test build.")

[commit-bot: I haz the power, 2016-06-13 08:58:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2020593002/260001

[commit-bot: I haz the power, 2016-06-13 09:03:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-13 09:03:26]

Try jobs failed on following builders:
  android_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/14112)

[commit-bot: I haz the power, 2016-06-13 11:06:18]

Description was changed from

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
==========

to

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
R=pbos@webrtc.org, perkj@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/718a763d5924d5a9948a4e5ecbf0fde1da11129f
Cr-Commit-Position: refs/heads/master@{#13110}
==========

[commit-bot: I haz the power, 2016-06-13 11:06:20]

Patchset 15 (id:??) landed as
https://crrev.com/718a763d5924d5a9948a4e5ecbf0fde1da11129f
Cr-Commit-Position: refs/heads/master@{#13110}

[nisse-webrtc, 2016-06-13 11:06:20]

Description was changed from

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
R=pbos@webrtc.org, perkj@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/718a763d5924d5a9948a4e5ecbf0fde1da11129f
Cr-Commit-Position: refs/heads/master@{#13110}
==========

to

==========
Refactor scaling.

Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.

Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.

BUG=webrtc:5682
R=pbos@webrtc.org, perkj@webrtc.org, stefan@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/718a763d5924d5a9948a4e5ec...
==========

[nisse-webrtc, 2016-06-13 11:06:22]

Committed patchset #15 (id:280001) manually as
718a763d5924d5a9948a4e5ecbf0fde1da11129f (presubmit successful).