Commit a8aa921c authored by Robert Bares's avatar Robert Bares Committed by Commit Bot

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: 's avatarFrank Barchard <fbarchard@chromium.org>
parent 5669005f
# 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 = (crop_height < 0) ? -crop_height : crop_height; int inv_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,18 +59,16 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -59,18 +59,16 @@ 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 = -inv_crop_height; inv_crop_height = (src_height < 0) ? -crop_height : crop_height;
}
if (need_buf) { if (need_buf) {
int argb_size = crop_width * 4 * abs_crop_height; int argb_size = crop_width * 4 * 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.
...@@ -147,13 +145,13 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -147,13 +145,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 * (src_height + crop_y / 2) + crop_x; src_uv = sample + aligned_src_width * (abs_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 * (src_height + crop_y / 2) + crop_x; src_uv = sample + aligned_src_width * (abs_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);
...@@ -252,7 +250,7 @@ int ConvertToARGB(const uint8_t* sample, ...@@ -252,7 +250,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, abs_crop_height, rotation); crop_width, crop_height, rotation);
} }
free(rotate_buffer); free(rotate_buffer);
} else if (rotation) { } else if (rotation) {
......
...@@ -46,8 +46,6 @@ int ConvertToI420(const uint8_t* sample, ...@@ -46,8 +46,6 @@ 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 &&
...@@ -60,22 +58,23 @@ int ConvertToI420(const uint8_t* sample, ...@@ -60,22 +58,23 @@ 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;
const int inv_crop_height = 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 * abs_crop_height; int y_size = crop_width * crop_height;
int uv_size = ((crop_width + 1) / 2) * ((abs_crop_height + 1) / 2); int uv_size = ((crop_width + 1) / 2) * ((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.
...@@ -163,7 +162,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -163,7 +162,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 * src_height) + src_uv = sample + (src_width * abs_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,
...@@ -171,7 +170,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -171,7 +170,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 * src_height) + src_uv = sample + (src_width * abs_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,
...@@ -261,7 +260,7 @@ int ConvertToI420(const uint8_t* sample, ...@@ -261,7 +260,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, abs_crop_height, tmp_v, tmp_v_stride, crop_width, 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