ConvertToI420 output has green layer : android

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Call ConvertToI420 with NV21 input
2. src_frame = NV21 from android camera(nexus 5 running lollipop),
   src_size = w * h * 1.5,
   dst_y = pDestY(Y pointer of destination I420),
   dst_stride_y = h,
   dst_u = pDestU(U pointer of destination I420),
   dst_stride_u = h/2,
   dst_v = pDestV(V pointer of destination I420),
   dst_stride_v = h/2,
   crop_x = 0,
   crop_y = 0,
   src_width = w,
   src_height = -h,
   crop_width = w,
   crop_height = h,
   rotation = libyuv::kRotate90 (or libyuv::kRotate270),
   format = libyuv::FOURCC_NV21
3. w and h are the width and height respectively, in my case it is 176 and 144.

What is the expected output? 
The expected output should be the I420 with proper color reproduction, same as 
input provided.

What do you see instead?
- The output is I420 and rotated by 90 degrees with dimensions 144*176
- The output is inverted since src_height provided is negative.
- But the output has a layer of green color.

What version of the product are you using? 
Name: libyuv
Version: 1406

On what operating system?
Android 5.0 Lollipop

Please provide any additional information below.

Everything is working fine except for the green colored layer that is present 
in the output I420.

Original issue reported on code.google.com by inamdar....@gmail.com on 1 Jun 2015 at 6:25

[GoogleCodeExporter]

Your parameters look okay, assuming you set your dst_u and dst_v correctly.
Are you sure the source buffer is contiguous - the NV21 uv plane is immediately 
after the y plane?
1. Does it work without rotation?

If so, it would be likely a bug in the NV21 rotation code.  Is it a 32 bit or 
64 bit (aarch64) build?
2. Try disabling Neon and see if that works.

3. Rotation can be done in 2 steps as a work around.  NV21ToI420 and then 
I420Rotate.

Original comment by fbarch...@chromium.org on 1 Jun 2015 at 5:49

[GoogleCodeExporter]

Your parameters look okay, assuming you set your dst_u and dst_v correctly.
 - I can confirm they are correct.
Are you sure the source buffer is contiguous - the NV21 uv plane is immediately 
after the y plane?
 - Yes it is NV21 for sure since it is obained directly from android camera.

1. Does it work without rotation?
 - The problem isn't with rotation, rotation works perfect. In addition to rotation i want to invert the yuv, to achieve inversion when i provide negative height then the output I420 has a green layer, with positive height it works great.

If so, it would be likely a bug in the NV21 rotation code.  Is it a 32 bit or 
64 bit (aarch64) build?
 - It is 32 bit architecture

2. Try disabling Neon and see if that works.
 - Tried disabling neon, still the same result.

3. Rotation can be done in 2 steps as a work around.  NV21ToI420 and then 
I420Rotate.
 - The problem isn't with rotation, rotation works perfect. In addition to rotation i want to invert the yuv, to achieve inversion when i provide negative height then the output I420 has a green layer, with positive height it works great.

I have a fully working sample app here https://github.com/zenith22/libyuvDemo - 
it demos the issue am facing
The assets folder contains yuvs that i obtained

nv21.yuv - input yuv, NV21 format

nv21-I420-One.yuv - output I420 using ConvertToI420 - no rotation no inversion
nv21-I420-Two.yuv - output I420 using ConvertToI420 - no rotation with inversion
nv21-I420-Three.yuv - output I420 using ConvertToI420 - 90 deg rotation no 
inversion
nv21-I420-Four.yuv - output I420 using ConvertToI420 - 90 deg rotation with 
inversion

Original comment by inamdar....@gmail.com on 2 Jun 2015 at 6:18

[GoogleCodeExporter]

Wow... thanks for the app.  I have a repro - its the negative height.
As a work around, an equivalent way to achieve that is pass in a negative 
stride, and point to the last row.

Original comment by fbarch...@chromium.org on 2 Jun 2015 at 7:46

• Changed state: Accepted

[GoogleCodeExporter]

I did not get the work around.
Could you please explain it with the function below, like what changes would be 
required in the parameters?

        // w = 176 and h = 144
    int nv21Len = env->GetArrayLength(nv21);
    unsigned char* nv21Buf = new unsigned char[nv21Len];
    env->GetByteArrayRegion(nv21, 0, nv21Len,reinterpret_cast<jbyte*>(nv21Buf));

    int Ysize = w * h;
    size_t src_size = Ysize * 1.5;

    unsigned char* I420 = new unsigned char[nv21Len];

    unsigned char* pDstY = I420;
    unsigned char* pDstU = I420 + Ysize;
    unsigned char* pDstV = pDstU + (Ysize / 4);

    libyuv::RotationMode mode = libyuv::kRotate90;

    libyuv::ConvertToI420(nv21Buf, src_size, pDstY, h, pDstU, h / 2, pDstV,
                h / 2, 0, 0, w, -h, w, h, mode, libyuv::FOURCC_NV21);

Original comment by inamdar....@gmail.com on 3 Jun 2015 at 6:00

[GoogleCodeExporter]

Since ConvertToI420 does not expose the source pointers and stride, you'd have 
to flip the destination, and change the rotation to 270

 // w = 176 and h = 144
    int nv21Len = env->GetArrayLength(nv21);
    unsigned char* nv21Buf = new unsigned char[nv21Len];
    env->GetByteArrayRegion(nv21, 0, nv21Len,reinterpret_cast<jbyte*>(nv21Buf));

    int Ysize = w * h;
    size_t src_size = Ysize * 1.5;

    unsigned char* I420 = new unsigned char[nv21Len];

    unsigned char* pDstY = I420 + (h - 1) * w;
    unsigned char* pDstU = I420 + Ysize + (h / 2 - 1) * (w / 2);
    unsigned char* pDstV = pDstU + (Ysize / 4);

    libyuv::RotationMode mode = libyuv::kRotate270;

    libyuv::ConvertToI420(nv21Buf, src_size, pDstY, -h, pDstU, -h / 2, pDstV,
                -h / 2, 0, 0, w, -h, w, h, mode, libyuv::FOURCC_NV21);

Original comment by fbarch...@chromium.org on 4 Jun 2015 at 7:43

[GoogleCodeExporter]

Did negative stride work?
I fixed the test failure with negative height, but it did not reproduce an 
issue.
May need more information to proceed.

Original comment by fbarch...@google.com on 26 Jun 2015 at 9:48

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

"Since ConvertToI420 does not expose the source pointers and stride, you'd have 
to flip the destination, and change the rotation to 270"

I tried the way you explained but it does not work, with 270 rotation it 
crashed and with 0 rotation the output was completely green.

I have updated the code if you wish to 
try.....https://github.com/zenith22/libyuvDemo 

Original comment by inamdar....@gmail.com on 13 Jul 2015 at 6:04

[GoogleCodeExporter]

In your new code, the function 
Java_com_demo_libyuvdemo_MainActivity_ConvertToI420NegativeStride

still passes -h to ConvertToI420:

int retVal = libyuv::ConvertToI420(nv21Buf, src_size, pDstY, -h, pDstU, -h / 2, 
pDstV,
      -h / 2, 0, 0, w, -h, w, h, mode, libyuv::FOURCC_NV21);

when rotating by 90 or 270, the source is w x h, but the destination is h x w.
with width/height values are for source... prerotated.
But the destination buffer needs pointers and strides that consider the 
rotation.
It may be less confusing if you create variables for the destination buffer 
width/height and strides.
But in particular, this line needs an adjustment
  unsigned char* pDstU = I420 + Ysize + (h / 2 - 1) * (w / 2);

  unsigned char* pDstU = I420 + Ysize + (w / 2 - 1) * (h / 2);
would point to the beginning of the last row.

I dont currently have a repro of a bug... it was just my test.
Suggest you use NV12ToI420Rotate, which lets you pass the source pointers too.

// Rotate NV12 input and store in I420.
LIBYUV_API
int NV12ToI420Rotate(const uint8* src_y, int src_stride_y,
                     const uint8* src_uv, int src_stride_uv,
                     uint8* dst_y, int dst_stride_y,
                     uint8* dst_u, int dst_stride_u,
                     uint8* dst_v, int dst_stride_v,
                     int src_width, int src_height, enum RotationMode mode);

Unless your camera/capturer give you upside down frames, its not normal to 
rotate and flip - that would be a transpose.
If regular rotate is working when you dont flip, you could do 2 steps and 
rotate by 90 or 270, then vertically flip using I420Copy and negative height.  
(to a new buffer)

Original comment by fbarch...@chromium.org on 16 Jul 2015 at 10:55