heifload ignores clean aperture information #1808
Comments
|
This seems to be working for me on ubuntu 20.04 with libvips 8.10 and the system-installed libheif, x265 etc.: Are you using a more recent libheif/x265/etc.? |
|
Could you try |
|
Ooop! Sorry, I missed the renaming there. Yes, I see the error too:
|
|
Ah, you're right. Processing the test image with The reason that older versions of libheif created the test image without ( |
|
I get the same error when reverting commit strukturag/libheif@cd3506d on libheif v1.8.0. So the After further investigation, it's only the PatchFrom 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Kleis Auke Wolthuizen <github@kleisauke.nl>
Date: Thu, 10 Sep 2020 15:50:00 +0200
Subject: [PATCH 1/1] Disable image dimensions check for libheif >= v1.8.0
See: https://github.com/libvips/libvips/issues/1808.
diff --git a/libvips/foreign/heifload.c b/libvips/foreign/heifload.c
index 1111111..2222222 100644
--- a/libvips/foreign/heifload.c
+++ b/libvips/foreign/heifload.c
@@ -790,6 +790,7 @@ vips_foreign_load_heif_generate( VipsRegion *or,
}
if( !heif->data ) {
+#if LIBHEIF_NUMERIC_VERSION < 0x01080000
int image_width = heif_image_get_width( heif->img,
heif_channel_interleaved );
int image_height = heif_image_get_height( heif->img,
@@ -805,6 +806,7 @@ vips_foreign_load_heif_generate( VipsRegion *or,
"%s", _( "bad image dimensions on decode" ) );
return( -1 );
}
+#endif
if( !(heif->data = heif_image_get_plane_readonly( heif->img,
heif_channel_interleaved, &heif->stride )) ) {But I'm not sure if this might cause side effects, do you perhaps have a test image for the segv mentioned here?: libvips/libvips/foreign/heifload.c Lines 798 to 801 in 4073f80 |
|
Was the "bad image dimensions on decode" to handle erroneously double-auto-rotated images, where both EXIF and HEIF contained an orientation? If so I think more recent versions of libheif, certainly 1.8.0, no longer do this so Kleis' patch will be safe to add. |
|
I think (from memory!) the check was for images smaller than one encoding tile. These used to not be clipped correctly. |
|
Oh, you're right, it was the autorotate stuff that was breaking. That's gone now, fortunately. I'm not sure about this patch though. If that test fails, something is badly wrong, and the image sizes for libheif and libvips are not matched. Isn't removing this check dangerous? Do we need to add extra clipping code? I'm probably missing something. |
|
I'm not sure about the patch above either, I just opened strukturag/libheif#365 which might give us some more information on how to handle this (note that processing |
|
The above mentioned patch does not appear to be safe. For example, processing the file corresponding to CVE-2020-10251 with the patch applied above causes an out-of-bounds read: $ wget https://t0.nl/poc.heic
$ vips copy poc.heic x.png
Segmentation fault (core dumped)Probably the best way to handle this is to clip the dimensions against |
I had an attempt to do this, but unfortunately it did not succeed. The problem is that the $ heif-info poc.heic
MIME type: image/heif
image: 1280x4439 (id=20004), primary
thumbnail: 320x212
color profile: no
alpha channel: no
depth channel: no
image: 1280x4439 (id=20006)
thumbnail: 320x212
color profile: no
alpha channel: no
depth channel: no
$ vipsheader poc.heic[page=0]
poc.heic: 1280x4439 uchar, 3 bands, srgb, heifload
$ vipsheader poc.heic[page=1]
poc.heic: 1280x4439 uchar, 3 bands, srgb, heifloadBut the actual encoded images are 1280×854 in size (i.e. the dimensions returned from $ heif-convert poc.heic x.jpg
File contains 2 images
Written to x-1.jpg
Written to x-2.jpg
$ vipsheader x-1.jpg
x-1.jpg: 1280x854 uchar, 3 bands, srgb, jpegload
$ vipsheader x-2.jpg
x-2.jpg: 1280x854 uchar, 3 bands, srgb, jpegloadAFAIK, there's currently no API available within libheif to "sniff" these encoded dimensions without decoding the whole image (i.e. without calling |
This appears to be the approach taken by ImageMagick, which would imply the following might suffice. @@ -861,9 +861,8 @@ vips_foreign_load_heif_generate( VipsRegion *or,
*/
if( image_width != heif->page_width ||
image_height != heif->page_height ) {
- vips_error( class->nickname,
- "%s", _( "bad image dimensions on decode" ) );
- return( -1 );
+ heif->page_width = image_width;
+ heif->page_height = image_height;
} |
Unfortunately this calls heif_decode_image in *_header, which seems a bit confusing, and is at the expense of performance. See libvips#1808 for context.
|
WIP commit kleisauke@14c7d5e (on the Note that this invokes |
|
I had a quick go at another solution: https://github.com/libvips/libvips/compare/fix-heifload-clipping This clips the decoded image against the reported image size or pads the output, as required. It works with
What do you think? The advantage is that we don't need to decode the whole file just to get basic dimensions. If libheif adds something in the future to give access to the decoded size, we could switch to that. |
|
Related: libheif 1.10 seem to have a bug setting the image dimensions. See: You need to test against 1.9.1. |
|
I can confirm that your solution works as well. Perhaps we should ask upstream for a method to get the dimensions of the encoded image without having to decode the whole image? It's a bit confusing that |
|
I've reverted to libheif 1.9.1 (as suggested reading through the issues here). However, ysing libvips 8.10.5 with libheif 1.9.1. I'm having an interesting issue where HEIF files are being corrupted unless, as far as I can tell:
Edit: The width being a multiple of 16 doesn't seem to hold. I can output my test images at 600, 900 and a few others, but no other combinations, using only HEVC (the other encoders report a |
|
Update - I did more testing and as long as the image dimensions are even (multiples of 2) libvips/libheif will output a really nice looking HEIF image. Should I force my dimensions to even numbers? I would really prefer to resize to exact ratios, but if this is expected behaviour, I can live with it. |
|
There's a new libheif due in a couple of days which should improve this. I think I'd try again with that. |
|
I noticed that the Though, on that branch it will also produce lots of black.
But at least it doesn't cause an error. |
|
I wasn't sure we'd arrived at consensus on this. Do you think that branch is the best solution for now? The huge black areas are very unfortunate. |
This issue is similar to #1574, but found a way to generate a HEIC image with a non-faulty
clapbox. This image is processable withheif-convertbut fails with libvips.Example image: https://t0.nl/x.heic
Generated with (with libheif v1.8.0 and libvips v8.10)
Reproduction
Image info
Note that the above
ispebox contains an image height of 2747 pixels which is cropped to 2746 pixels with theclapbox info. This is therefore a valid HEIC image.The text was updated successfully, but these errors were encountered: