2753536587 / libyuv

Automatically exported from code.google.com/p/libyuv
0 stars 0 forks source link

I420Rotate + kRotate180 doesn't work. #25

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Call I420Rotate with kRotate180.
2.
3.

What is the expected output? What do you see instead?
The frame should be rotated 180 degrees. The frame is not rotated.

What version of the product are you using? On what operating system?
Latest SVN. Windows 7, Mac OS X and iOS

Please provide any additional information below.

I have a c++ function that is supposed to rotate the given frame 180 degrees. 
The function is:

bool video_conversion::rotate_180(
    unsigned char * src_frame, int src_stride, int width, int height
    )
{
    unsigned char * src_yplane = src_frame;
    unsigned char * src_uplane = src_yplane + width * height;
    unsigned char * src_vplane = src_uplane + (width * height / 4);

    return libyuv::I420Rotate(
        src_yplane, src_stride, src_uplane, (src_stride + 1) / 2,
        src_vplane, (src_stride + 1) / 2, src_yplane, src_stride, src_uplane,
        (src_stride + 1) / 2, src_vplane, (src_stride + 1) / 2, 0, 0, libyuv::kRotate180  
    );
}

Original issue reported on code.google.com by J...@junglecat.org on 26 Mar 2012 at 2:45

GoogleCodeExporter commented 9 years ago
Note on this line: src_uplane + (width * height / 4);
it wont be quite right for odd width/height.
best to compute
int half_width = (width + 1) / 2;
int half_height = (height + 1) / 2;
unsigned char * src_vplane = src_uplane + half_width * half_height;

The main issue is you're passing src as destination.  Unfortunately I didn't 
anticipate that one and it won't work in the current code.

The way the function works internally is it writes each row of destination, 
starting at the top left, with a horizontally mirrored row, starting the reads 
at the last row.
It would be difficult to make 90 and 270 degree rotation work, but it would be 
possible to make 180 work, if it swapped 2 rows at a time.

Original comment by fbarch...@google.com on 28 Mar 2012 at 12:28

GoogleCodeExporter commented 9 years ago
r227 adds I420Rotate by 180 in place

1. ConvertToI420 now has the ability to rotate anything (ie MJPG) using a 
temporary buffer.  So it was easy to add a check for destination==source and 
use the buffer to allow rotate to work 'in-place'.  But it wont really be 
inplace.

2. I420Rotate does 2 rows at a time, swapping them and mirroring.
Performance is only marginally slower, but more flexible, so I always use the 
new approach for better code coverage.

Before:
ConstructI420Rotate0 (455 ms)
ConstructI420Rotate90 (921 ms)
ConstructI420Rotate180 (425 ms)
ConstructI420Rotate270 (922 ms)

After
ConstructI420Rotate0 (460 ms)
ConstructI420Rotate90 (917 ms)
ConstructI420Rotate180 (461 ms)
ConstructI420Rotate270 (911 ms)

Original comment by fbarch...@google.com on 28 Mar 2012 at 3:03

GoogleCodeExporter commented 9 years ago

Original comment by fbarch...@google.com on 28 Mar 2012 at 8:05

GoogleCodeExporter commented 9 years ago
r247 fixes a Neon rotate issue.

Original comment by fbarch...@google.com on 22 Apr 2012 at 3:08