Add SplitUVPlanes and MergeUVPlanes

Add SplitUVPlanes and MergeUVPlanes

Add public methods SplitUVPlanes and MergeUVPlanes based on the
optimized assembly functions that already exists.

BUG=libyuv:629
R=braveyao@chromium.org

Committed: https://chromium.googlesource.com/libyuv/libyuv/+/c244a3e9a03920cee977e88d4c1b3925f27e87da

[fbarchard1, 2016-08-24 21:50:57]

fbarchard@google.com changed reviewers:
+ magjed@chromium.org

[fbarchard1, 2016-08-24 21:50:57]

clone of https://codereview.chromium.org/2266533003/ with reabse to head, no
changes applied.

[fbarchard1, 2016-08-24 23:04:11]

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert.h
File include/libyuv/convert.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert.h#ne...
include/libyuv/convert.h:206: int SplitUVPlane(const uint8* src_uv, int
src_stride_uv,
move to planar_functions.h

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert_from.h
File include/libyuv/convert_from.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert_from...
include/libyuv/convert_from.h:165: int MergeUVPlanes(const uint8* src_u, int
src_stride_u,
move to planar_functions.h

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/row.h
File include/libyuv/row.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/row.h#newcod...
include/libyuv/row.h:846: typedef void (*SplitUVRowFunction)(const uint8
*src_uv, uint8 *dst_u,
make private to planar_functions.cc

https://codereview.chromium.org/2277603004/diff/1/source/convert_from_argb.cc
File source/convert_from_argb.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/convert_from_argb.cc...
source/convert_from_argb.cc:268: MergeUVRowFunction MergeUVRow_ =
GetOptimizedMergeUVRowFunction(width);
prefer no getter for consistency.

https://codereview.chromium.org/2277603004/diff/1/source/planar_functions.cc
File source/planar_functions.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/planar_functions.cc#...
source/planar_functions.cc:34: // Negative height means invert the image.
this change isnt needed? remove

https://codereview.chromium.org/2277603004/diff/1/source/planar_functions.cc#...
source/planar_functions.cc:2556: memcpy(dst_uv, rows + awidth, awidth);
this change isnt needed?  remove.
if this change is needed, do it in different CL

https://codereview.chromium.org/2277603004/diff/1/source/row_common.cc
File source/row_common.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/row_common.cc#newcod...
source/row_common.cc:1804: SplitUVRowFunction GetOptimizedSplitUVRowFunction(
these are not row functions... should remove or if kept, move to
planar_functions.cc, row_any.cc or a new file.
if needed, open bug and do get functions for all row functions.

https://codereview.chromium.org/2277603004/diff/20001/include/libyuv/row.h
File include/libyuv/row.h (right):

https://codereview.chromium.org/2277603004/diff/20001/include/libyuv/row.h#ne...
include/libyuv/row.h:846: typedef void (*SplitUVRowFunction)(const uint8
*src_uv, uint8 *dst_u,
change not needed in row.h

https://codereview.chromium.org/2277603004/diff/20001/source/convert.cc
File source/convert.cc (right):

https://codereview.chromium.org/2277603004/diff/20001/source/convert.cc#newco...
source/convert.cc:47: // TODO(fbarchard): support NULL for dst_y
check dst_y

https://codereview.chromium.org/2277603004/diff/20001/source/convert_from_arg...
File source/convert_from_argb.cc (right):

https://codereview.chromium.org/2277603004/diff/20001/source/convert_from_arg...
source/convert_from_argb.cc:268: MergeUVRowFunction MergeUVRow_ =
GetOptimizedMergeUVRowFunction(width);
change not needed here.

https://codereview.chromium.org/2277603004/diff/20001/source/planar_functions.cc
File source/planar_functions.cc (left):

https://codereview.chromium.org/2277603004/diff/20001/source/planar_functions...
source/planar_functions.cc:2485: void (*SplitUVRow)(const uint8* src_uv, uint8*
dst_u, uint8* dst_v,
change not needed for yuy2

https://codereview.chromium.org/2277603004/diff/20001/source/planar_functions.cc
File source/planar_functions.cc (right):

https://codereview.chromium.org/2277603004/diff/20001/source/planar_functions...
source/planar_functions.cc:34: // Negative height means invert the image.
remove

https://codereview.chromium.org/2277603004/diff/20001/source/planar_functions...
source/planar_functions.cc:271: int MergeUVPlanes(const uint8* src_u, int
src_stride_u,
consider void return

https://codereview.chromium.org/2277603004/diff/20001/source/row_common.cc
File source/row_common.cc (right):

https://codereview.chromium.org/2277603004/diff/20001/source/row_common.cc#ne...
source/row_common.cc:1804: SplitUVRowFunction GetOptimizedSplitUVRowFunction(
remove - row common is for C versions of row functions.

[fbarchard1, 2016-08-24 23:11:51]

[ RUN      ] LibYUVConvertTest.NV21ToI420_NullY
[       OK ] LibYUVConvertTest.NV21ToI420_NullY (65 ms)

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert.h
File include/libyuv/convert.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert.h#ne...
include/libyuv/convert.h:206: int SplitUVPlane(const uint8* src_uv, int
src_stride_uv,
On 2016/08/24 23:04:10, fbarchard1 wrote:
> move to planar_functions.h

Done.

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert_from.h
File include/libyuv/convert_from.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/convert_from...
include/libyuv/convert_from.h:165: int MergeUVPlanes(const uint8* src_u, int
src_stride_u,
On 2016/08/24 23:04:10, fbarchard1 wrote:
> move to planar_functions.h

Done.

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/row.h
File include/libyuv/row.h (right):

https://codereview.chromium.org/2277603004/diff/1/include/libyuv/row.h#newcod...
include/libyuv/row.h:846: typedef void (*SplitUVRowFunction)(const uint8
*src_uv, uint8 *dst_u,
On 2016/08/24 23:04:10, fbarchard1 wrote:
> make private to planar_functions.cc

Done.

https://codereview.chromium.org/2277603004/diff/1/source/convert_from_argb.cc
File source/convert_from_argb.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/convert_from_argb.cc...
source/convert_from_argb.cc:268: MergeUVRowFunction MergeUVRow_ =
GetOptimizedMergeUVRowFunction(width);
On 2016/08/24 23:04:11, fbarchard1 wrote:
> prefer no getter for consistency.

Done.

https://codereview.chromium.org/2277603004/diff/1/source/planar_functions.cc
File source/planar_functions.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/planar_functions.cc#...
source/planar_functions.cc:34: // Negative height means invert the image.
On 2016/08/24 23:04:11, fbarchard1 wrote:
> this change isnt needed? remove

Acknowledged.  Currently needed in this CL and consistent with SplitUVPlane. 
But CopyPlane_16 does not support negative height, so added todo to reconsider
this.

https://codereview.chromium.org/2277603004/diff/1/source/row_common.cc
File source/row_common.cc (right):

https://codereview.chromium.org/2277603004/diff/1/source/row_common.cc#newcod...
source/row_common.cc:1804: SplitUVRowFunction GetOptimizedSplitUVRowFunction(
On 2016/08/24 23:04:11, fbarchard1 wrote:
> these are not row functions... should remove or if kept, move to
> planar_functions.cc, row_any.cc or a new file.
> if needed, open bug and do get functions for all row functions.

Done.

[fbarchard1, 2016-08-24 23:19:11]

mergeuv needs a test.

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
File include/libyuv/planar_functions.h (right):

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
include/libyuv/planar_functions.h:51: void MergeUVPlanes(const uint8* src_u, int
src_stride_u,
caveat Plane functions usually deal with a single plane. e.g. the UV plane of
NV12 is a single plane.
while Planes functions usually do all 3 or 4 planes of a complete image.
The plural form here may be inconsistent with other Planes functions.

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
include/libyuv/planar_functions.h:52: const uint8* src_v, int src_stride_v,
indent 1 off

[fbarchard1, 2016-08-24 23:22:52]

locally ran existing unittests on intel linux and arm android.

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
File include/libyuv/planar_functions.h (right):

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
include/libyuv/planar_functions.h:51: void MergeUVPlanes(const uint8* src_u, int
src_stride_u,
On 2016/08/24 23:19:11, fbarchard1 wrote:
> caveat Plane functions usually deal with a single plane. e.g. the UV plane of
> NV12 is a single plane.
> while Planes functions usually do all 3 or 4 planes of a complete image.
> The plural form here may be inconsistent with other Planes functions.

Done.

https://codereview.chromium.org/2277603004/diff/40001/include/libyuv/planar_f...
include/libyuv/planar_functions.h:52: const uint8* src_v, int src_stride_v,
On 2016/08/24 23:19:11, fbarchard1 wrote:
> indent 1 off

Done.

[braveyao, 2016-08-24 23:23:21]

braveyao@chromium.org changed reviewers:
+ braveyao@chromium.org

[braveyao, 2016-08-24 23:23:21]

lgtm

[fbarchard1, 2016-08-24 23:47:26]

Description was changed from

==========
Add SplitUVPlanes and MergeUVPlanes

Add public methods SplitUVPlanes and MergeUVPlanes based on the
optimized assembly functions that already exists. Also, de-duplicate the
CPU dispatching code for these functions by moving them to helper
functions.

BUG=libyuv:629
==========

to

==========
Add SplitUVPlanes and MergeUVPlanes

Add public methods SplitUVPlanes and MergeUVPlanes based on the
optimized assembly functions that already exists. Also, de-duplicate the
CPU dispatching code for these functions by moving them to helper
functions.

BUG=libyuv:629
R=braveyao@chromium.org

Committed:
https://chromium.googlesource.com/libyuv/libyuv/+/c244a3e9a03920cee977e88d4c1...
==========

[fbarchard1, 2016-08-24 23:47:27]

Committed patchset #4 (id:60001) manually as
c244a3e9a03920cee977e88d4c1b3925f27e87da (presubmit successful).

[fbarchard1, 2016-08-25 00:03:35]

Description was changed from

==========
Add SplitUVPlanes and MergeUVPlanes

Add public methods SplitUVPlanes and MergeUVPlanes based on the
optimized assembly functions that already exists. Also, de-duplicate the
CPU dispatching code for these functions by moving them to helper
functions.

BUG=libyuv:629
R=braveyao@chromium.org

Committed:
https://chromium.googlesource.com/libyuv/libyuv/+/c244a3e9a03920cee977e88d4c1...
==========

to

==========
Add SplitUVPlanes and MergeUVPlanes

Add public methods SplitUVPlanes and MergeUVPlanes based on the
optimized assembly functions that already exists.

BUG=libyuv:629
R=braveyao@chromium.org

Committed:
https://chromium.googlesource.com/libyuv/libyuv/+/c244a3e9a03920cee977e88d4c1...
==========