iOS: Optimize video scaling and cropping

iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444

Committed: https://crrev.com/5a8724564c2245603ba59edf8d6d2736d0efa0ce
Cr-Commit-Position: refs/heads/master@{#14701}

[magjed_webrtc, 2016-10-06 13:48:39]

Description was changed from

==========
iOS/Mac: Optimize CPU adaptation scaling and conversion

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW vs HW encoding. For SW encoding, an I420
buffer is needed, and an efficient I420 -> NV12 cropping, scaling, and
conversion is performed. For HW encoding, a CVPixelBuffer is needed, and
an efficient NV12 -> NV12 cropping and scaling is performed. If no
cropping is required for the HW case, no expensive operation is
performed at all.

There might still be room for improvement in the CVPixelBuffer cropping
and scaling path using some HW support. This might be explored in a
future CL.

BUG=/30939444
==========

to

==========
iOS/Mac: Optimize CPU adaptation scaling and conversion

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW vs HW encoding. For SW encoding, an I420
buffer is needed, and an efficient I420 -> NV12 cropping, scaling, and
conversion is performed. For HW encoding, a CVPixelBuffer is needed, and
an efficient NV12 -> NV12 cropping and scaling is performed. If no
cropping is required for the HW case, no expensive operation is
performed at all.

There might still be room for improvement in the CVPixelBuffer cropping
and scaling path using some HW support. This might be explored in a
future CL.

BUG=/30939444
==========

[magjed_webrtc, 2016-10-18 14:10:35]

Description was changed from

==========
iOS/Mac: Optimize CPU adaptation scaling and conversion

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW vs HW encoding. For SW encoding, an I420
buffer is needed, and an efficient I420 -> NV12 cropping, scaling, and
conversion is performed. For HW encoding, a CVPixelBuffer is needed, and
an efficient NV12 -> NV12 cropping and scaling is performed. If no
cropping is required for the HW case, no expensive operation is
performed at all.

There might still be room for improvement in the CVPixelBuffer cropping
and scaling path using some HW support. This might be explored in a
future CL.

BUG=/30939444
==========

to

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 scaling and cropping is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 scaling and cropping is implemented in
CoreVideoFrameBuffer::CropAndScaleTo.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

[magjed_webrtc, 2016-10-18 14:11:02]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2016-10-18 14:21:42]

Description was changed from

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 scaling and cropping is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 scaling and cropping is implemented in
CoreVideoFrameBuffer::CropAndScaleTo.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

to

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane. The cropping and
scaling is now done in one combined step instead of making an
intermediary copy of the Y plane.


There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

[magjed_webrtc, 2016-10-18 15:34:30]

magjed@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[magjed_webrtc, 2016-10-18 15:34:55]

Description was changed from

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane. The cropping and
scaling is now done in one combined step instead of making an
intermediary copy of the Y plane.


There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

to

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

[magjed_webrtc, 2016-10-18 15:35:25]

Zeke - please take a look.

[tkchin_webrtc, 2016-10-19 00:48:32]

lgtm

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
File webrtc/common_video/corevideo_frame_buffer.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:94: std::vector<uint8_t>*
tmp_buffer,
is buffer passed in so that it can be re-used across multiple crop/scale ops?

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:99: CVReturn cvRet =
CVPixelBufferLockBaseAddress(output_pixel_buffer, 0);
nit: cv_ret

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:142:
CVPixelBufferUnlockBaseAddress(pixel_buffer_, kCVPixelBufferLock_ReadOnly);
not locked as readonly

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
File webrtc/common_video/include/corevideo_frame_buffer.h (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/corevideo_frame_buffer.h:45: const int
buffer_width_;
how is buffer width different from width/height of NativeHandleBuffer?

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:343: void
NV12Scale(std::vector<uint8_t>* tmp_buffer,
might be worth comparing performance against:
https://developer.apple.com/reference/accelerate

but likely outside the scope of this CL

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm
(right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:199:
return nil;
nullptr

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:208:
return nil;
CVPixelBufferRef isn't an object, so return nullptr instead.

[magjed_webrtc, 2016-10-19 13:19:34]

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
File webrtc/common_video/corevideo_frame_buffer.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:94: std::vector<uint8_t>*
tmp_buffer,
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> is buffer passed in so that it can be re-used across multiple crop/scale ops?

Yes exactly.

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:99: CVReturn cvRet =
CVPixelBufferLockBaseAddress(output_pixel_buffer, 0);
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> nit: cv_ret

Done.

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:142:
CVPixelBufferUnlockBaseAddress(pixel_buffer_, kCVPixelBufferLock_ReadOnly);
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> not locked as readonly

Yes it is?

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
File webrtc/common_video/include/corevideo_frame_buffer.h (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/corevideo_frame_buffer.h:45: const int
buffer_width_;
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> how is buffer width different from width/height of NativeHandleBuffer?

I agree this is a bit confusing, but I don't see how I can easily make it
clearer. It works like this: buffer_width/height is the actual pixel buffer
resolution, the width/height in NativeHandleBuffer is the resolution we will
scale to. A generic external user should be unaware of the internal pixel buffer
size and only have to know what resolution NativeToI420Buffer() will have. So:
buffer_width >= crop_width >= width().

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:343: void
NV12Scale(std::vector<uint8_t>* tmp_buffer,
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> might be worth comparing performance against:
> https://developer.apple.com/reference/accelerate
> 
> but likely outside the scope of this CL

Interesting, I will compare performance if I have time in the future. Optimally,
we will find some HW CVPixelBuffer -> CVPixelBuffer scaling using official apple
functions.

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm
(right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:199:
return nil;
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> nullptr

Done.

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_encoder.mm:208:
return nil;
On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> CVPixelBufferRef isn't an object, so return nullptr instead.

Done.

[tkchin_webrtc, 2016-10-19 22:50:17]

lgtm

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
File webrtc/common_video/corevideo_frame_buffer.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/corev...
webrtc/common_video/corevideo_frame_buffer.cc:142:
CVPixelBufferUnlockBaseAddress(pixel_buffer_, kCVPixelBufferLock_ReadOnly);
On 2016/10/19 13:19:34, magjed_webrtc wrote:
> On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> > not locked as readonly
> 
> Yes it is?

ah, misread. You are correct.

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
File webrtc/common_video/include/corevideo_frame_buffer.h (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/inclu...
webrtc/common_video/include/corevideo_frame_buffer.h:45: const int
buffer_width_;
On 2016/10/19 13:19:34, magjed_webrtc wrote:
> On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> > how is buffer width different from width/height of NativeHandleBuffer?
> 
> I agree this is a bit confusing, but I don't see how I can easily make it
> clearer. It works like this: buffer_width/height is the actual pixel buffer
> resolution, the width/height in NativeHandleBuffer is the resolution we will
> scale to. A generic external user should be unaware of the internal pixel
buffer
> size and only have to know what resolution NativeToI420Buffer() will have. So:
> buffer_width >= crop_width >= width().

gotcha. Maybe short comment along those lines somewhere? (If this already exists
in a superclass that's fine too)

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/2394483005/diff/40001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:343: void
NV12Scale(std::vector<uint8_t>* tmp_buffer,
On 2016/10/19 13:19:34, magjed_webrtc wrote:
> On 2016/10/19 00:48:32, tkchin_webrtc wrote:
> > might be worth comparing performance against:
> > https://developer.apple.com/reference/accelerate
> > 
> > but likely outside the scope of this CL
> 
> Interesting, I will compare performance if I have time in the future.
Optimally,
> we will find some HW CVPixelBuffer -> CVPixelBuffer scaling using official
apple
> functions.

Acknowledged.

[magjed_webrtc, 2016-10-20 08:51:37]

The CQ bit was checked by magjed@webrtc.org

[magjed_webrtc, 2016-10-20 08:51:37]

The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2394483005/#ps80001 (title:
"Add comment about the different resolution variables.")

[commit-bot: I haz the power, 2016-10-20 08:51:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-10-20 10:34:30]

Description was changed from

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

to

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

[commit-bot: I haz the power, 2016-10-20 10:34:31]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-20 10:34:39]

Description was changed from

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444
==========

to

==========
iOS: Optimize video scaling and cropping

This CL makes scaling and cropping lazy in AVFoundationVideoCapturer and
provides optimized paths for SW and HW encoding. For SW encoding, an
efficient NV12 -> I420 cropping and scaling is implemented in
CoreVideoFrameBuffer::NativeToI420. For HW encoding, an efficient NV12 ->
NV12 cropping and scaling is implemented in
CoreVideoFrameBuffer::CropAndScaleTo. The performance improvement over
the existing cropping and scaling is that it is now done in one step
instead of making an intermediary copy of the Y plane.

There might still be room for improvement in the HW path using some HW
support. That will be explored in a future CL.

BUG=b/30939444

Committed: https://crrev.com/5a8724564c2245603ba59edf8d6d2736d0efa0ce
Cr-Commit-Position: refs/heads/master@{#14701}
==========

[commit-bot: I haz the power, 2016-10-20 10:34:40]

Patchset 4 (id:??) landed as
https://crrev.com/5a8724564c2245603ba59edf8d6d2736d0efa0ce
Cr-Commit-Position: refs/heads/master@{#14701}