libheif icon indicating copy to clipboard operation
libheif copied to clipboard

crash when converting a heic image when height is odd

Open vtolkov opened this issue 3 years ago • 6 comments

With the images from iPhone, image height is 1713, but u and v planes are only 856 high, so the last scan line is out of the u/v planes, so if we compute like size_t offset_u = (cinfo.next_scanline / 2) * stride_u; const uint8_t* start_u = &row_u[offset_u]; we reading data are out of limits. That's explains why we have green line at the bottom sometimes. I suggest that the call like const uint8_t* row_u = heif_image_get_plane_readonly(image, heif_channel_Cb, &stride_u); returns plane height as well in addition to stride, in this case we can check the height in the decoding loop and reuse the last u/v line. I can suggest a patch, but I would prefer to know what author prefer in interface versioning.

vtolkov avatar Jun 16 '21 18:06 vtolkov

Those sizes would be an invalid input image. libheif should probably refuse to load this image with an error. Can you send me the image so that I can reproduce this?

farindk avatar Jun 16 '21 18:06 farindk

After looking more today, I think this is a result of the resizing feature of the library. iPhone produces image with even dimensions. But if I apply heif_image_scale_image to the original image with the odd size (which is taken from the app viewport size, which can be anything), the internal u/v planes seems to be resized incorrectly.

Like we have image 20x10. Its color planes are 10x5. Then we require it to scale two times to 10x5. Then if we produce color planes 5x2, then when converting last scan-line 4 we access vertical index 2 and it out of limits.

Valeri

On Jun 16, 2021, at 11:29 AM, Dirk Farin @.***> wrote:

Those sizes would be an invalid input image. libheif should probably refuse to load this image with an error. Can you send me the image so that I can reproduce this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

vtolkov avatar Jun 17 '21 02:06 vtolkov

It's not that the chroma is resized incorrectly, it's that an odd size is illegal in 4:2:0 chroma. (Or odd height for 4:2:2.) If you want both odd size and subsampled chroma, you have to set a crop box instead.

That said, rounding down for chroma is worse than rounding up as coded, of course. The 10x5->5x2 case is a rather pathological case that very obviously wouldn't be correct at 2 or 3, but at larger sizes isn't noticeable.

The basic method for all subsampled chroma is is YUV->RGB->resize to odd->pad to even with pixel duplication->YUV->crop box to odd, though you can also assume infinite padding to skip the color conversion steps.

silverbacknet avatar Jun 17 '21 03:06 silverbacknet

Ok,

Maybe the scale function should do something about the odd size, like returning error or returning even-sized image. I prefer later as it will work correctly in most of practical scenarios without complicating caller’s code.

The number i use are not single digits, it was just for example. My app just tries to display heic in a browser resampling/converting on the server, because original size is large enough to transfer. I use viewport size as a target size for all supported image formats, because the knowledge of internal structure isn’t available or interesting at the app level.

Sent from my iPad

On Jun 16, 2021, at 8:01 PM, Emily Bowman @.***> wrote:

 It's not that the chroma is resized incorrectly, it's that an odd size is illegal in 4:2:0 chroma. (Or odd height for 4:2:2.) If you want both odd size and subsampled chroma, you have to set a crop box instead.

That said, rounding down for chroma is worse than rounding up as coded, of course. The 10x5->5x2 case is a rather pathological case that very obviously wouldn't be correct at 2 or 3, but at larger sizes isn't noticeable.

The basic method for all subsampled chroma is is YUV->RGB->resize to odd->pad to even with pixel duplication->YUV->crop box to odd, though you can also assume infinite padding to skip the color conversion steps.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

vtolkov avatar Jun 17 '21 03:06 vtolkov

In that case, you should probably always convert everything to RGB first. Resizing with subsampled chroma for display will throw away color fidelity unnecessarily. (It's still a good bug to fix, though.)

libheif has its very basic resize function, but you might look into using a third-party linear RGB resize (as opposed to the standard gamma resize, not bilinear) as well for downsizing everything; it's not always noticeable, but when it is, like white points on a dark background, the difference is enormous.

silverbacknet avatar Jun 17 '21 03:06 silverbacknet

Scaling of 4:2:0 images should now be fixed. Thanks @vtolkov for reporting this. Please confirm that the current version is working.

farindk avatar Jul 12 '21 17:07 farindk