diff --git a/ChangeLog b/ChangeLog index 20144c67..814c9492 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,8 +24,9 @@ behaviour with alpha channels - improve bioformats support with read and write of tiff subifd pyramids - thumbnail exploits subifd pyramids -- handle all EXIF orientation cases, deprecate - vips_autorot_get_angle() [Elad-Laufer] +- handle all EXIF orientation cases, deprecate vips_autorot_get_angle() + [Elad-Laufer] +- deprecate heifload autorotate -- it's now always on 24/4/20 started 8.9.3 - better iiif tile naming [IllyaMoskvin] diff --git a/configure.ac b/configure.ac index d38393ee..0c1e5c76 100644 --- a/configure.ac +++ b/configure.ac @@ -928,18 +928,6 @@ if test x"$with_heif" = x"yes"; then LIBS="$save_LIBS" fi -# fetch untransformed width/height added in 1.3.4 -if test x"$with_heif" = x"yes"; then - save_LIBS="$LIBS" - LIBS="$LIBS $HEIF_LIBS" - AC_CHECK_FUNCS(heif_image_handle_get_ispe_width,[ - AC_DEFINE(HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH,1, - [define if you have heif_image_handle_get_ispe_width.]) - ],[] - ) - LIBS="$save_LIBS" -fi - # heif_decoding_options.convert_hdr_to_8bit added in 1.7.0 if test x"$with_heif" = x"yes"; then save_CFLAGS="$CFLAGS" diff --git a/libvips/foreign/heifload.c b/libvips/foreign/heifload.c index 49a90960..aaa09005 100644 --- a/libvips/foreign/heifload.c +++ b/libvips/foreign/heifload.c @@ -16,6 +16,8 @@ * - restart after minimise * 15/3/20 * - revise for new VipsSource API + * 10/5/20 + * - deprecate autorotate -- it's too difficult to support properly */ /* @@ -99,6 +101,11 @@ typedef struct _VipsForeignLoadHeif { gboolean thumbnail; /* Apply any orientation tags in the header. + * + * This is deprecated and does nothing. Non-autorotated reads from + * libheif are surprisingly hard to support well, since orientation can + * be represented in several different ways in HEIC files and devices + * vary in how they do this. */ gboolean autorotate; @@ -364,7 +371,7 @@ vips_foreign_load_heif_set_header( VipsForeignLoadHeif *heif, VipsImage *out ) #endif /*DEBUG*/ bands = heif->has_alpha ? 4 : 3; - /* FIXME .. need to test XMP and IPCT. + /* FIXME .. IPTC as well? */ n_metadata = heif_image_handle_get_list_of_metadata_block_IDs( heif->handle, NULL, id, VIPS_NUMBER( id ) ); @@ -414,10 +421,20 @@ vips_foreign_load_heif_set_header( VipsForeignLoadHeif *heif, VipsImage *out ) vips_image_set_blob( out, name, (VipsCallbackFn) NULL, data, length ); - if( g_ascii_strcasecmp( type, "exif" ) == 0 ) - (void) vips__exif_parse( out ); + /* image_set will automatically parse EXIF, if necessary. + */ } + /* We use libheif's autorotate, so we need to remove any EXIF + * orientaion tags. + * + * According to the HEIF standard, EXIF orientation tags are only + * informational and images should not be rotated because of them. + * Unless we strip these tags, there's a danger downstream processing + * could double-rotate. + */ + vips_autorot_remove_angle( out ); + #ifdef HAVE_HEIF_COLOR_PROFILE enum heif_color_profile_type profile_type = heif_image_handle_get_color_profile_type( heif->handle ); @@ -480,13 +497,6 @@ vips_foreign_load_heif_set_header( VipsForeignLoadHeif *heif, VipsImage *out ) } #endif /*HAVE_HEIF_COLOR_PROFILE*/ - /* If we are using libheif's autorotate, remove the exif one. - */ -#ifdef HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH - if( heif->autorotate ) - vips_autorot_remove_angle( out ); -#endif /*HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH*/ - vips_image_set_int( out, "heif-primary", heif->primary_page ); vips_image_set_int( out, "n-pages", heif->n_top ); if( vips_object_argument_isset( VIPS_OBJECT( heif ), "n" ) ) @@ -508,40 +518,6 @@ vips_foreign_load_heif_set_header( VipsForeignLoadHeif *heif, VipsImage *out ) return( 0 ); } -static int -vips_foreign_load_heif_get_width( VipsForeignLoadHeif *heif, - struct heif_image_handle *handle ) -{ - int width; - - /* _get_ipse_width() fetches the untransformed dimension, but was only - * added in 1.3.4. Without it, we just use the transformed dimension - * and have to autorotate. - */ - width = heif_image_handle_get_width( handle ); -#ifdef HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH - if( !heif->autorotate ) - width = heif_image_handle_get_ispe_width( handle ); -#endif /*HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH*/ - - return( width ); -} - -static int -vips_foreign_load_heif_get_height( VipsForeignLoadHeif *heif, - struct heif_image_handle *handle ) -{ - int height; - - height = heif_image_handle_get_height( handle ); -#ifdef HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH - if( !heif->autorotate ) - height = heif_image_handle_get_ispe_height( handle ); -#endif /*HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH*/ - - return( height ); -} - static int vips_foreign_load_heif_header( VipsForeignLoad *load ) { @@ -585,10 +561,6 @@ vips_foreign_load_heif_header( VipsForeignLoad *load ) } #ifdef DEBUG -#ifdef HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH - if( !heif->autorotate ) - printf( "using _get_ispe_width() / _height()\n" ); -#endif /*HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH*/ for( i = heif->page; i < heif->page + heif->n; i++ ) { heif_item_id thumb_ids[1]; int n_items; @@ -619,11 +591,9 @@ vips_foreign_load_heif_header( VipsForeignLoad *load ) printf( " thumb %d\n", j ); printf( " width = %d\n", - vips_foreign_load_heif_get_width( heif, - thumb_handle ) ); + heif_image_handle_get_width( thumb_handle ) ); printf( " height = %d\n", - vips_foreign_load_heif_get_height( heif, - thumb_handle ) ); + heif_image_handle_get_height( thumb_handle ) ); } } #endif /*DEBUG*/ @@ -633,18 +603,16 @@ vips_foreign_load_heif_header( VipsForeignLoad *load ) if( vips_foreign_load_heif_set_page( heif, heif->page, heif->thumbnail ) ) return( -1 ); - heif->page_width = vips_foreign_load_heif_get_width( heif, - heif->handle ); - heif->page_height = vips_foreign_load_heif_get_height( heif, - heif->handle ); + heif->page_width = heif_image_handle_get_width( heif->handle ); + heif->page_height = heif_image_handle_get_height( heif->handle ); for( i = heif->page + 1; i < heif->page + heif->n; i++ ) { if( vips_foreign_load_heif_set_page( heif, i, heif->thumbnail ) ) return( -1 ); - if( vips_foreign_load_heif_get_width( heif, - heif->handle ) != heif->page_width || - vips_foreign_load_heif_get_height( heif, - heif->handle ) != heif->page_height ) { + if( heif_image_handle_get_width( heif->handle ) + != heif->page_width || + heif_image_handle_get_height( heif->handle ) + != heif->page_height ) { vips_error( class->nickname, "%s", _( "not all pages are the same size" ) ); return( -1 ); @@ -658,11 +626,9 @@ vips_foreign_load_heif_header( VipsForeignLoad *load ) if( vips_foreign_load_heif_set_page( heif, i, FALSE ) ) return( -1 ); printf( " width = %d\n", - vips_foreign_load_heif_get_width( heif, - heif->handle ) ); + heif_image_handle_get_width( heif->handle ) ); printf( " height = %d\n", - vips_foreign_load_heif_get_height( heif, - heif->handle ) ); + heif_image_handle_get_height( heif->handle ) ); printf( " has_depth = %d\n", heif_image_handle_has_depth_image( heif->handle ) ); printf( " has_alpha = %d\n", @@ -713,13 +679,7 @@ vips_foreign_load_heif_generate( VipsRegion *or, heif_chroma_interleaved_RGBA : heif_chroma_interleaved_RGB; - /* Only disable transforms if we have been able to fetch the - * untransformed dimensions. - */ options = heif_decoding_options_alloc(); -#ifdef HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH - options->ignore_transformations = !heif->autorotate; -#endif /*HAVE_HEIF_IMAGE_HANDLE_GET_ISPE_WIDTH*/ #ifdef HAVE_HEIF_DECODING_OPTIONS_CONVERT_HDR_TO_8BIT /* VIPS_FORMAT_UCHAR is assumed so downsample HDR to 8bpc */ @@ -891,7 +851,7 @@ vips_foreign_load_heif_class_init( VipsForeignLoadHeifClass *class ) VIPS_ARG_BOOL( class, "autorotate", 21, _( "Autorotate" ), _( "Rotate image using exif orientation" ), - VIPS_ARGUMENT_OPTIONAL_INPUT, + VIPS_ARGUMENT_OPTIONAL_INPUT | VIPS_ARGUMENT_DEPRECATED, G_STRUCT_OFFSET( VipsForeignLoadHeif, autorotate ), FALSE ); @@ -913,9 +873,12 @@ vips_foreign_load_heif_read( void *data, size_t size, void *userdata ) gint64 result; result = vips_source_read( heif->source, data, size ); + /* On EOF, make a note of the file length. + */ if( result == 0 && heif->length == -1 ) - heif->length = vips_source_seek( heif->source, 0L, SEEK_CUR ); + result = heif->length = + vips_source_seek( heif->source, 0L, SEEK_CUR ); if( result < 0 ) return( -1 ); @@ -1211,7 +1174,6 @@ vips_foreign_load_heif_source_init( VipsForeignLoadHeifSource *source ) * * @page: %gint, page (top-level image number) to read * * @n: %gint, load this many pages * * @thumbnail: %gboolean, fetch thumbnail instead of image - * * @autorotate: %gboolean, rotate image upright during load * * Read a HEIF image file into a VIPS image. * @@ -1228,17 +1190,6 @@ vips_foreign_load_heif_source_init( VipsForeignLoadHeifSource *source ) * If @thumbnail is %TRUE, then fetch a stored thumbnail rather than the * image. * - * Setting @autorotate to %TRUE will make the loader interpret the - * orientation tag and automatically rotate the image appropriately during - * load. - * - * If @autorotate is %FALSE, the metadata field #VIPS_META_ORIENTATION is set - * to the value of the orientation tag. Applications may read and interpret - * this field - * as they wish later in processing. See vips_autorot(). Save - * operations will use #VIPS_META_ORIENTATION, if present, to set the - * orientation of output images. - * * See also: vips_image_new_from_file(). * * Returns: 0 on success, -1 on error. @@ -1268,7 +1219,6 @@ vips_heifload( const char *filename, VipsImage **out, ... ) * * @page: %gint, page (top-level image number) to read * * @n: %gint, load this many pages * * @thumbnail: %gboolean, fetch thumbnail instead of image - * * @autorotate: %gboolean, rotate image upright during load * * Read a HEIF image file into a VIPS image. * Exactly as vips_heifload(), but read from a memory buffer. @@ -1311,7 +1261,6 @@ vips_heifload_buffer( void *buf, size_t len, VipsImage **out, ... ) * * @page: %gint, page (top-level image number) to read * * @n: %gint, load this many pages * * @thumbnail: %gboolean, fetch thumbnail instead of image - * * @autorotate: %gboolean, rotate image upright during load * * Exactly as vips_heifload(), but read from a source. * diff --git a/test/test-suite/helpers/helpers.py b/test/test-suite/helpers/helpers.py index c493d55e..e6eb3251 100644 --- a/test/test-suite/helpers/helpers.py +++ b/test/test-suite/helpers/helpers.py @@ -36,7 +36,7 @@ DICOM_FILE = os.path.join(IMAGES, "dicom_test_image.dcm") BMP_FILE = os.path.join(IMAGES, "MARBLES.BMP") NIFTI_FILE = os.path.join(IMAGES, "avg152T1_LR_nifti.nii.gz") ICO_FILE = os.path.join(IMAGES, "favicon.ico") -HEIC_FILE = os.path.join(IMAGES, "Example1.heic") +HEIC_FILE = os.path.join(IMAGES, "heic-orientation-6.heic") unsigned_formats = [pyvips.BandFormat.UCHAR, pyvips.BandFormat.USHORT, diff --git a/test/test-suite/images/heic-orientation-6.heic b/test/test-suite/images/heic-orientation-6.heic new file mode 100644 index 00000000..b5c44cc3 Binary files /dev/null and b/test/test-suite/images/heic-orientation-6.heic differ diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py index 3b8a4b6f..c0bf6391 100644 --- a/test/test-suite/test_foreign.py +++ b/test/test-suite/test_foreign.py @@ -968,9 +968,9 @@ class TestForeign: a = im(10, 10) # different versions of HEIC decode have slightly different # rounding - assert_almost_equal_objects(a, [75.0, 86.0, 81.0], threshold=2) - assert im.width == 4032 - assert im.height == 3024 + assert_almost_equal_objects(a, [197.0, 181.0, 158.0], threshold=2) + assert im.width == 3024 + assert im.height == 4032 assert im.bands == 3 self.file_loader("heifload", HEIC_FILE, heif_valid) diff --git a/test/test-suite/test_resample.py b/test/test-suite/test_resample.py index 04c81580..0b0916ff 100644 --- a/test/test-suite/test_resample.py +++ b/test/test-suite/test_resample.py @@ -171,9 +171,6 @@ class TestResample: im = pyvips.Image.new_from_file(HEIC_FILE) thumb = pyvips.Image.thumbnail(HEIC_FILE, 100) - # original is landscape - assert im.width > im.height - # thumb should be portrait assert thumb.width < thumb.height assert thumb.height == 100