Commit 196e2e72 authored by Frank Barchard's avatar Frank Barchard Committed by Commit Bot

Revert "Allow negative height when ConvertToI420/ARGB is called with NV12/NV21"

This reverts commit a8aa921c.

Reason for revert: breaks a webrtc unittest on Windows.

https://bugs.chromium.org/p/webrtc/issues/detail?id=9263&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20ReleaseBlock%20Component%20Status%20Owner%20Summary&groupby=&sort=

Original change's description:
> Allow negative height when ConvertToI420/ARGB is called with NV12/NV21
> 
> ConvertToI420 and ConvertToARGB support the use of a negative height
> parameter to flip the image vertically. When converting from NV12 or
> NV21 this parameter was misinterpreted, resulting in invalid output.
> This CL introduces the use of abs_src_height to correctly calculate
> the location of the source UV plane.
> 
> The sign of crop_height is not used, to reduce confusion ConvertToI420
> and ConvertToARGB no longer accept negative crop height.
> 
> Unit tests for Android420ToI420 are updated to fix miscalculation of
> src_stride_uv, fix incorrect pixel strides, and to test inversion.
> New unit tests are included to test inversion for ConvertToARGB,
> ConvertToI420, Android420ToARGB, and Android420ToABGR.
> For consistency the test NV12Crop is renamed ConvertToI420_NV12_Crop.
> 
> Bug: libyuv:446
> Test: out/Release/libyuv_unittest --gtest_filter=*.ConvertTo*:*.Android420To*
> Change-Id: Idc98e62671cb30272cfa7e24fafbc8b73712f7c6
> Reviewed-on: https://chromium-review.googlesource.com/994074
> Commit-Queue: Frank Barchard <fbarchard@chromium.org>
> Reviewed-by: Frank Barchard <fbarchard@chromium.org>

TBR=fbarchard@chromium.org,robert@bares.me

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: libyuv:446, chromium:9263, libyuv:801
Change-Id: I7c55b3fcb477f9754c249b9c2c54b24da2c29283
Reviewed-on: https://chromium-review.googlesource.com/1081267Reviewed-by: 's avatarFrank Barchard <fbarchard@chromium.org>
Reviewed-by: 's avatarWeiyong Yao <braveyao@chromium.org>
Commit-Queue: Frank Barchard <fbarchard@chromium.org>
parent a7fb978e
# Names should be added to this file like so: # Names should be added to this file like so:
# Name or Organization <email address> # Name or Organization <email address>
Robert Bares <robert@bares.me>
Google Inc. Google Inc.
...@@ -46,7 +46,7 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -46,7 +46,7 @@ int ConvertToARGB(const uint8_t* sample,
const uint8_t* src; const uint8_t* src;
const uint8_t* src_uv; const uint8_t* src_uv;
int abs_src_height = (src_height < 0) ? -src_height : src_height; int abs_src_height = (src_height < 0) ? -src_height : src_height;
int inv_crop_height; int inv_crop_height = (crop_height < 0) ? -crop_height : crop_height;
int r = 0; int r = 0;
// One pass rotation is available for some formats. For the rest, convert // One pass rotation is available for some formats. For the rest, convert
...@@ -59,16 +59,18 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -59,16 +59,18 @@ int ConvertToARGB(const uint8_t* sample,
uint8_t* dest_argb = dst_argb; uint8_t* dest_argb = dst_argb;
int dest_dst_stride_argb = dst_stride_argb; int dest_dst_stride_argb = dst_stride_argb;
uint8_t* rotate_buffer = NULL; uint8_t* rotate_buffer = NULL;
int abs_crop_height = (crop_height < 0) ? -crop_height : crop_height;
if (dst_argb == NULL || sample == NULL || src_width <= 0 || crop_width <= 0 || if (dst_argb == NULL || sample == NULL || src_width <= 0 || crop_width <= 0 ||
src_height == 0 || crop_height <= 0) { src_height == 0 || crop_height == 0) {
return -1; return -1;
} }
if (src_height < 0) {
inv_crop_height = (src_height < 0) ? -crop_height : crop_height; inv_crop_height = -inv_crop_height;
}
if (need_buf) { if (need_buf) {
int argb_size = crop_width * 4 * crop_height; int argb_size = crop_width * 4 * abs_crop_height;
rotate_buffer = (uint8_t*)malloc(argb_size); /* NOLINT */ rotate_buffer = (uint8_t*)malloc(argb_size); /* NOLINT */
if (!rotate_buffer) { if (!rotate_buffer) {
return 1; // Out of memory runtime error. return 1; // Out of memory runtime error.
...@@ -145,13 +147,13 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -145,13 +147,13 @@ int ConvertToARGB(const uint8_t* sample,
// Biplanar formats // Biplanar formats
case FOURCC_NV12: case FOURCC_NV12:
src = sample + (src_width * crop_y + crop_x); src = sample + (src_width * crop_y + crop_x);
src_uv = sample + aligned_src_width * (abs_src_height + crop_y / 2) + crop_x; src_uv = sample + aligned_src_width * (src_height + crop_y / 2) + crop_x;
r = NV12ToARGB(src, src_width, src_uv, aligned_src_width, dst_argb, r = NV12ToARGB(src, src_width, src_uv, aligned_src_width, dst_argb,
dst_stride_argb, crop_width, inv_crop_height); dst_stride_argb, crop_width, inv_crop_height);
break; break;
case FOURCC_NV21: case FOURCC_NV21:
src = sample + (src_width * crop_y + crop_x); src = sample + (src_width * crop_y + crop_x);
src_uv = sample + aligned_src_width * (abs_src_height + crop_y / 2) + crop_x; src_uv = sample + aligned_src_width * (src_height + crop_y / 2) + crop_x;
// Call NV12 but with u and v parameters swapped. // Call NV12 but with u and v parameters swapped.
r = NV21ToARGB(src, src_width, src_uv, aligned_src_width, dst_argb, r = NV21ToARGB(src, src_width, src_uv, aligned_src_width, dst_argb,
dst_stride_argb, crop_width, inv_crop_height); dst_stride_argb, crop_width, inv_crop_height);
...@@ -250,7 +252,7 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -250,7 +252,7 @@ int ConvertToARGB(const uint8_t* sample,
if (need_buf) { if (need_buf) {
if (!r) { if (!r) {
r = ARGBRotate(dst_argb, dst_stride_argb, dest_argb, dest_dst_stride_argb, r = ARGBRotate(dst_argb, dst_stride_argb, dest_argb, dest_dst_stride_argb,
crop_width, crop_height, rotation); crop_width, abs_crop_height, rotation);
} }
free(rotate_buffer); free(rotate_buffer);
} else if (rotation) { } else if (rotation) {
......
...@@ -46,6 +46,8 @@ int ConvertToI420(const uint8_t* sample, ...@@ -46,6 +46,8 @@ int ConvertToI420(const uint8_t* sample,
const uint8_t* src; const uint8_t* src;
const uint8_t* src_uv; const uint8_t* src_uv;
const int abs_src_height = (src_height < 0) ? -src_height : src_height; const int abs_src_height = (src_height < 0) ? -src_height : src_height;
// TODO(nisse): Why allow crop_height < 0?
const int abs_crop_height = (crop_height < 0) ? -crop_height : crop_height;
int r = 0; int r = 0;
LIBYUV_BOOL need_buf = LIBYUV_BOOL need_buf =
(rotation && format != FOURCC_I420 && format != FOURCC_NV12 && (rotation && format != FOURCC_I420 && format != FOURCC_NV12 &&
...@@ -58,23 +60,22 @@ int ConvertToI420(const uint8_t* sample, ...@@ -58,23 +60,22 @@ int ConvertToI420(const uint8_t* sample,
int tmp_u_stride = dst_stride_u; int tmp_u_stride = dst_stride_u;
int tmp_v_stride = dst_stride_v; int tmp_v_stride = dst_stride_v;
uint8_t* rotate_buffer = NULL; uint8_t* rotate_buffer = NULL;
int inv_crop_height; const int inv_crop_height =
(src_height < 0) ? -abs_crop_height : abs_crop_height;
if (!dst_y || !dst_u || !dst_v || !sample || src_width <= 0 || if (!dst_y || !dst_u || !dst_v || !sample || src_width <= 0 ||
crop_width <= 0 || src_height == 0 || crop_height <= 0) { crop_width <= 0 || src_height == 0 || crop_height == 0) {
return -1; return -1;
} }
inv_crop_height = (src_height < 0) ? -crop_height : crop_height;
// One pass rotation is available for some formats. For the rest, convert // One pass rotation is available for some formats. For the rest, convert
// to I420 (with optional vertical flipping) into a temporary I420 buffer, // to I420 (with optional vertical flipping) into a temporary I420 buffer,
// and then rotate the I420 to the final destination buffer. // and then rotate the I420 to the final destination buffer.
// For in-place conversion, if destination dst_y is same as source sample, // For in-place conversion, if destination dst_y is same as source sample,
// also enable temporary buffer. // also enable temporary buffer.
if (need_buf) { if (need_buf) {
int y_size = crop_width * crop_height; int y_size = crop_width * abs_crop_height;
int uv_size = ((crop_width + 1) / 2) * ((crop_height + 1) / 2); int uv_size = ((crop_width + 1) / 2) * ((abs_crop_height + 1) / 2);
rotate_buffer = (uint8_t*)malloc(y_size + uv_size * 2); /* NOLINT */ rotate_buffer = (uint8_t*)malloc(y_size + uv_size * 2); /* NOLINT */
if (!rotate_buffer) { if (!rotate_buffer) {
return 1; // Out of memory runtime error. return 1; // Out of memory runtime error.
...@@ -162,7 +163,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -162,7 +163,7 @@ int ConvertToI420(const uint8_t* sample,
// Biplanar formats // Biplanar formats
case FOURCC_NV12: case FOURCC_NV12:
src = sample + (src_width * crop_y + crop_x); src = sample + (src_width * crop_y + crop_x);
src_uv = sample + (src_width * abs_src_height) + src_uv = sample + (src_width * src_height) +
((crop_y / 2) * aligned_src_width) + ((crop_x / 2) * 2); ((crop_y / 2) * aligned_src_width) + ((crop_x / 2) * 2);
r = NV12ToI420Rotate(src, src_width, src_uv, aligned_src_width, dst_y, r = NV12ToI420Rotate(src, src_width, src_uv, aligned_src_width, dst_y,
dst_stride_y, dst_u, dst_stride_u, dst_v, dst_stride_y, dst_u, dst_stride_u, dst_v,
...@@ -170,7 +171,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -170,7 +171,7 @@ int ConvertToI420(const uint8_t* sample,
break; break;
case FOURCC_NV21: case FOURCC_NV21:
src = sample + (src_width * crop_y + crop_x); src = sample + (src_width * crop_y + crop_x);
src_uv = sample + (src_width * abs_src_height) + src_uv = sample + (src_width * src_height) +
((crop_y / 2) * aligned_src_width) + ((crop_x / 2) * 2); ((crop_y / 2) * aligned_src_width) + ((crop_x / 2) * 2);
// Call NV12 but with dst_u and dst_v parameters swapped. // Call NV12 but with dst_u and dst_v parameters swapped.
r = NV12ToI420Rotate(src, src_width, src_uv, aligned_src_width, dst_y, r = NV12ToI420Rotate(src, src_width, src_uv, aligned_src_width, dst_y,
...@@ -260,7 +261,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -260,7 +261,7 @@ int ConvertToI420(const uint8_t* sample,
if (!r) { if (!r) {
r = I420Rotate(dst_y, dst_stride_y, dst_u, dst_stride_u, dst_v, r = I420Rotate(dst_y, dst_stride_y, dst_u, dst_stride_u, dst_v,
dst_stride_v, tmp_y, tmp_y_stride, tmp_u, tmp_u_stride, dst_stride_v, tmp_y, tmp_y_stride, tmp_u, tmp_u_stride,
tmp_v, tmp_v_stride, crop_width, crop_height, tmp_v, tmp_v_stride, crop_width, abs_crop_height,
rotation); rotation);
} }
free(rotate_buffer); free(rotate_buffer);
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment