Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[patch] NV21 to I420 bug #17

Closed
GoogleCodeExporter opened this issue Jul 19, 2015 · 4 comments
Closed

[patch] NV21 to I420 bug #17

GoogleCodeExporter opened this issue Jul 19, 2015 · 4 comments

Comments

@GoogleCodeExporter
Copy link

To reproduce :
Try to use ConvertToI420 with NV21 input.

Observe :
inverted U/V

Version :
Latest trunk (r204) 

Observation :
Indeed, regarding code there is a bug : it even doesn't match with the comment 
:D... I guess some bad copy/paste of the NV12 case.

Quick patch I did that solves the problem.
It's a naive working fix tested in my project - but maybe I have missed 
something, so it should be reviewed :

Index: convert.cc
===================================================================
--- convert.cc  (révision 202)
+++ convert.cc  (copie de travail)
@@ -1746,8 +1746,8 @@
       r = NV12ToI420Rotate(src, src_width,
                            src_uv, aligned_src_width,
                            y, y_stride,
+                           v, v_stride,
                            u, u_stride,
-                           v, v_stride,
                            dst_width, inv_dst_height, rotation);
       break;
     case FOURCC_M420:

Original issue reported on code.google.com by r3gis...@gmail.com on 6 Mar 2012 at 11:59

@GoogleCodeExporter
Copy link
Author

Are you sure you have NV12 and not NV21?
NV12 has a UV plane with U followed by V.  NV12ToI420Rotate outputs the U to 
the u, u_stride and V to the v, v_stride.
The same function can be used for NV21 by passing u and v swapped.

Original comment by fbarch...@google.com on 7 Mar 2012 at 6:36

@GoogleCodeExporter
Copy link
Author

oh wait... my bad.  Fixed in r208

Original comment by fbarch...@google.com on 7 Mar 2012 at 6:39

  • Changed state: Started

@GoogleCodeExporter
Copy link
Author

Original comment by fbarch...@google.com on 7 Mar 2012 at 7:20

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Thanks for the fast commit :) !

I'll continue to investigate the other point I mentioned on the libyuv google 
group about rotation. 
But that's another issue I'll open as soon as I'll get a clear idea of what's 
going wrong (and maybe a patch too). 
My first analysis is that the culprit is ~maybe~ webRTC that sends to libyuv 
request with a rotation of 270 or 90 while they don't invert width and height 
for dest frame. (even if that's actually what they want to do, rotate a 352 × 
288 into a new 352 × 288 with content rotate of 270° -- but that's maybe not 
supported by libyuv).
Anyway, libyuv probably should make sure that it doesn't try to write out of 
the dst frame in this kind of case. 
But it's probably more about sanity check than about real "bug" (unless it's 
something that could be considered as expected feature of libyuv).

But, I'll keep you in touch with a new issue :). Or if there is another mean 
you prefer me to use for this kind of point? 

Original comment by r3gis...@gmail.com on 8 Mar 2012 at 10:32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant