From 02327b621415059078272ef621b7832e5fac4dae Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 1 May 2020 16:05:48 +0100 Subject: [PATCH 01/19] add an experimental libspng reader it compiles, but I've not tried running it yet heh --- ChangeLog | 1 + README.md | 24 ++ configure.ac | 22 +- libvips/foreign/Makefile.am | 1 + libvips/foreign/spngload.c | 674 ++++++++++++++++++++++++++++++++++++ 5 files changed, 720 insertions(+), 2 deletions(-) create mode 100644 libvips/foreign/spngload.c diff --git a/ChangeLog b/ChangeLog index 350c63f3..4e70da6f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -18,6 +18,7 @@ - add all to smartcrop - flood fill could stop half-way for some very complex shapes - better handling of unaligned reads in multipage tiffs [petoor] +- add experimental libspng reader 24/4/20 started 8.9.3 - better iiif tile naming [IllyaMoskvin] diff --git a/README.md b/README.md index 03bab492..1b4c4e6d 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,30 @@ [![Fuzzing Status](https://oss-fuzz-build-logs.storage.googleapis.com/badges/libvips.svg)](https://bugs.chromium.org/p/oss-fuzz/issues/list?sort=-opened&can=2&q=proj:libvips) [![Coverity Status](https://scan.coverity.com/projects/6503/badge.svg)](https://scan.coverity.com/projects/jcupitt-libvips) +# This branch + +Is for experiemtning with [libspng](https://github.com/randy408/libspng). + +## Notes + +Build libspng: + +``` +cd libspng +meson build --prefix=/home/john/vips --libdir=/home/john/vips/lib +cd build +ninja +ninja install +``` + +Installs `spng.pc`. + +Sample code: + +https://github.com/randy408/libspng/blob/master/examples/example.c + +# Introduction + libvips is a [demand-driven, horizontally threaded](https://github.com/libvips/libvips/wiki/Why-is-libvips-quick) image processing library. Compared to similar diff --git a/configure.ac b/configure.ac index 2c712caf..e44866e9 100644 --- a/configure.ac +++ b/configure.ac @@ -1175,6 +1175,22 @@ FIND_GIFLIB( ] ) +# Look for libspng first +AC_ARG_WITH([spng], + AS_HELP_STRING([--without-spng], [build without libspng (default: test)])) + +if test x"$with_spng" != x"no"; then + PKG_CHECK_MODULES(SPNG, spng >= 0.6, + [AC_DEFINE(HAVE_SPNG,1,[define if you have libspng installed.]) + with_spng=yes + PACKAGES_USED="$PACKAGES_USED spng" + with_png=no + ], + [ + ] + ) +fi + # look for PNG with pkg-config ... fall back to our tester AC_ARG_WITH([png], AS_HELP_STRING([--without-png], [build without libpng (default: test)])) @@ -1298,10 +1314,10 @@ if test x"$LIB_FUZZING_ENGINE" = x; then fi # Gather all up for VIPS_CFLAGS, VIPS_INCLUDES, VIPS_LIBS -VIPS_CFLAGS="$VIPS_CFLAGS $GTHREAD_CFLAGS $GIO_CFLAGS $REQUIRED_CFLAGS $EXPAT_CFLAGS $ZLIB_CFLAGS $PANGOFT2_CFLAGS $GSF_CFLAGS $FFTW_CFLAGS $MAGICK_CFLAGS $JPEG_CFLAGS $PNG_CFLAGS $IMAGEQUANT_CFLAGS $EXIF_CFLAGS $MATIO_CFLAGS $CFITSIO_CFLAGS $LIBWEBP_CFLAGS $LIBWEBPMUX_CFLAGS $GIFLIB_INCLUDES $RSVG_CFLAGS $PDFIUM_INCLUDES $POPPLER_CFLAGS $OPENEXR_CFLAGS $OPENSLIDE_CFLAGS $ORC_CFLAGS $TIFF_CFLAGS $LCMS_CFLAGS $HEIF_CFLAGS" +VIPS_CFLAGS="$VIPS_CFLAGS $GTHREAD_CFLAGS $GIO_CFLAGS $REQUIRED_CFLAGS $EXPAT_CFLAGS $ZLIB_CFLAGS $PANGOFT2_CFLAGS $GSF_CFLAGS $FFTW_CFLAGS $MAGICK_CFLAGS $JPEG_CFLAGS $SPNG_CFLAGS $PNG_CFLAGS $IMAGEQUANT_CFLAGS $EXIF_CFLAGS $MATIO_CFLAGS $CFITSIO_CFLAGS $LIBWEBP_CFLAGS $LIBWEBPMUX_CFLAGS $GIFLIB_INCLUDES $RSVG_CFLAGS $PDFIUM_INCLUDES $POPPLER_CFLAGS $OPENEXR_CFLAGS $OPENSLIDE_CFLAGS $ORC_CFLAGS $TIFF_CFLAGS $LCMS_CFLAGS $HEIF_CFLAGS" VIPS_CFLAGS="$VIPS_DEBUG_FLAGS $VIPS_CFLAGS" VIPS_INCLUDES="$ZLIB_INCLUDES $PNG_INCLUDES $TIFF_INCLUDES $JPEG_INCLUDES $NIFTI_INCLUDES" -VIPS_LIBS="$ZLIB_LIBS $HEIF_LIBS $MAGICK_LIBS $PNG_LIBS $IMAGEQUANT_LIBS $TIFF_LIBS $JPEG_LIBS $GTHREAD_LIBS $GIO_LIBS $REQUIRED_LIBS $EXPAT_LIBS $PANGOFT2_LIBS $GSF_LIBS $FFTW_LIBS $ORC_LIBS $LCMS_LIBS $GIFLIB_LIBS $RSVG_LIBS $NIFTI_LIBS $PDFIUM_LIBS $POPPLER_LIBS $OPENEXR_LIBS $OPENSLIDE_LIBS $CFITSIO_LIBS $LIBWEBP_LIBS $LIBWEBPMUX_LIBS $MATIO_LIBS $EXIF_LIBS -lm" +VIPS_LIBS="$ZLIB_LIBS $HEIF_LIBS $MAGICK_LIBS $SPNG_LIBS $PNG_LIBS $IMAGEQUANT_LIBS $TIFF_LIBS $JPEG_LIBS $GTHREAD_LIBS $GIO_LIBS $REQUIRED_LIBS $EXPAT_LIBS $PANGOFT2_LIBS $GSF_LIBS $FFTW_LIBS $ORC_LIBS $LCMS_LIBS $GIFLIB_LIBS $RSVG_LIBS $NIFTI_LIBS $PDFIUM_LIBS $POPPLER_LIBS $OPENEXR_LIBS $OPENSLIDE_LIBS $CFITSIO_LIBS $LIBWEBP_LIBS $LIBWEBPMUX_LIBS $MATIO_LIBS $EXIF_LIBS -lm" AC_SUBST(VIPS_LIBDIR) @@ -1397,6 +1413,8 @@ file import with cfitsio: $with_cfitsio file import/export with libwebp: $with_libwebp (requires libwebp, libwebpmux, libwebpdemux 0.6.0 or later) text rendering with pangoft2: $with_pangoft2 +file import/export with libspng: $with_spng + (requires libspng-0.6 or later) file import/export with libpng: $with_png (requires libpng-1.2.9 or later) support 8bpp PNG quantisation: $with_imagequant diff --git a/libvips/foreign/Makefile.am b/libvips/foreign/Makefile.am index d9a68585..759df2e5 100644 --- a/libvips/foreign/Makefile.am +++ b/libvips/foreign/Makefile.am @@ -39,6 +39,7 @@ libforeign_la_SOURCES = \ magickload.c \ magick7load.c \ magicksave.c \ + spngload.c \ pngload.c \ pngsave.c \ vipspng.c \ diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c new file mode 100644 index 00000000..e89566d2 --- /dev/null +++ b/libvips/foreign/spngload.c @@ -0,0 +1,674 @@ +/* load PNG with libspng + * + * 5/12/11 + * - from pngload.c + */ + +/* + + This file is part of VIPS. + + VIPS is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301 USA + + */ + +/* + + These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk + + */ + +/* Notes: + * + * an enum for interlace_method would be nice ... ADAM7 == 1, + * no interlace == 0. + * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) + */ + +/* +#define DEBUG + */ + +#ifdef HAVE_CONFIG_H +#include +#endif /*HAVE_CONFIG_H*/ +#include + +#include +#include +#include + +#include +#include +#include + +#include "pforeign.h" + +#ifdef HAVE_SPNG + +#include + +typedef struct _VipsForeignLoadPng { + VipsForeignLoad parent_object; + + /* Set by subclasses. + */ + VipsSource *source; + + spng_ctx *ctx; + struct spng_ihdr ihdr; + enum spng_format fmt; + int bands; + VipsInterpretation interpretation; + VipsBandFormat format; + size_t out_size; + size_t out_line_size; + int y_pos; + +} VipsForeignLoadPng; + +typedef VipsForeignLoadClass VipsForeignLoadPngClass; + +G_DEFINE_ABSTRACT_TYPE( VipsForeignLoadPng, vips_foreign_load_png, + VIPS_TYPE_FOREIGN_LOAD ); + +static void +vips_foreign_load_png_dispose( GObject *gobject ) +{ + VipsForeignLoadPng *png = (VipsForeignLoadPng *) gobject; + + VIPS_FREEF( spng_ctx_free, png->ctx ); + VIPS_UNREF( png->source ); + + G_OBJECT_CLASS( vips_foreign_load_png_parent_class )-> + dispose( gobject ); +} + +/* libspng read callbacks should copy length bytes to dest and return 0 + * or SPNG_IO_EOF/SPNG_IO_ERROR on error. + */ +static int +vips_foreign_load_png_stream( spng_ctx *ctx, void *user, + void *dest, size_t length ) +{ + VipsSource *source = VIPS_SOURCE( user ); + + gint64 bytes_read; + + bytes_read = vips_source_read( source, dest, length ); + if( bytes_read < 0 ) + return( SPNG_IO_ERROR ); + if( bytes_read < length ) + return( SPNG_IO_EOF); + + return( 0 ); +} + +static VipsForeignFlags +vips_foreign_load_png_get_flags_source( VipsSource *source ) +{ + spng_ctx *ctx; + struct spng_ihdr ihdr; + VipsForeignFlags flags; + + ctx = spng_ctx_new( 0 ); + spng_set_crc_action( ctx, SPNG_CRC_USE, SPNG_CRC_USE ); + spng_set_png_stream( ctx, + vips_foreign_load_png_stream, source ); + if( spng_get_ihdr( ctx, &ihdr ) ) { + spng_ctx_free( ctx ); + return( 0 ); + } + spng_ctx_free( ctx ); + + flags = 0; + if( ihdr.interlace_method != 0 ) + flags |= VIPS_FOREIGN_PARTIAL; + else + flags |= VIPS_FOREIGN_SEQUENTIAL; + + return( flags ); +} + +static VipsForeignFlags +vips_foreign_load_png_get_flags( VipsForeignLoad *load ) +{ + VipsForeignLoadPng *png = (VipsForeignLoadPng *) load; + + return( vips_foreign_load_png_get_flags_source( png->source ) ); +} + +static VipsForeignFlags +vips_foreign_load_png_get_flags_filename( const char *filename ) +{ + VipsSource *source; + VipsForeignFlags flags; + + if( !(source = vips_source_new_from_file( filename )) ) + return( 0 ); + flags = vips_foreign_load_png_get_flags_source( source ); + VIPS_UNREF( source ); + + return( flags ); +} + +static void +vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) +{ + vips_image_init_fields( image, + png->ihdr.width, png->ihdr.height, png->bands, + png->format, VIPS_CODING_NONE, png->interpretation, + 1.0, 1.0 ); + VIPS_SETSTR( image->filename, + vips_connection_filename( VIPS_CONNECTION( png->source ) ) ); + + /* 0 is no interlace. + */ + if( png->ihdr.interlace_method == 0 ) + /* Sequential mode needs thinstrip to work with things like + * vips_shrink(). + */ + vips_image_pipelinev( image, + VIPS_DEMAND_STYLE_THINSTRIP, NULL ); + else + /* Interlaced images are read via a huge memory buffer. + */ + vips_image_pipelinev( image, VIPS_DEMAND_STYLE_ANY, NULL ); +} + +static int +vips_foreign_load_png_header( VipsForeignLoad *load ) +{ + VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load ); + VipsForeignLoadPng *png = (VipsForeignLoadPng *) load; + + int error; + + /* Flags can be eg. SPNG_CTX_IGNORE_ADLER32 to ignore CRC chaekcs in + * deflate. + * + * FIXME ... fail handling + */ + png->ctx = spng_ctx_new( 0 ); + spng_set_crc_action( png->ctx, SPNG_CRC_USE, SPNG_CRC_USE ); + + spng_set_png_stream( png->ctx, + vips_foreign_load_png_stream, png->source ); + if( (error = spng_get_ihdr( png->ctx, &png->ihdr )) ) { + vips_error( class->nickname, "%s", spng_strerror( error ) ); + return( -1 ); + } + + printf(" width: %d\nheight: %d\nbit depth: %d\ncolor type: %d\n", + png->ihdr.width, png->ihdr.height, + png->ihdr.bit_depth, png->ihdr.color_type ); + printf( "compression method: %d\nfilter method: %d\n" + "interlace method: %d\n", + png->ihdr.compression_method, png->ihdr.filter_method, + png->ihdr.interlace_method ); + + switch( png->ihdr.color_type ) { + case SPNG_COLOR_TYPE_GRAYSCALE: + png->interpretation = VIPS_INTERPRETATION_B_W; + png->bands = 1; + break; + + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: + png->interpretation = VIPS_INTERPRETATION_B_W; + png->bands = 2; + break; + + case SPNG_COLOR_TYPE_TRUECOLOR: + case SPNG_COLOR_TYPE_INDEXED: + png->interpretation = VIPS_INTERPRETATION_sRGB; + png->bands = 3; + break; + + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: + png->interpretation = VIPS_INTERPRETATION_sRGB; + png->bands = 4; + break; + + default: + vips_error( class->nickname, "%s", "unknown color_type" ); + return( -1 ); + } + + if( png->ihdr.bit_depth == 16 ) { + png->fmt = SPNG_FMT_RGBA16; + png->format = VIPS_FORMAT_USHORT; + if( png->interpretation == VIPS_INTERPRETATION_B_W ) + png->interpretation = VIPS_INTERPRETATION_GREY16; + if( png->interpretation == VIPS_INTERPRETATION_sRGB ) + png->interpretation = VIPS_INTERPRETATION_RGB16; + } + else { + png->fmt = SPNG_FMT_RGBA8; + png->format = VIPS_FORMAT_UCHAR; + } + + /* FIXME ... get resolution. + */ + + vips_source_minimise( png->source ); + + vips_foreign_load_png_set_header( png, load->out ); + + return( 0 ); +} + +static int +vips_foreign_load_png_interlace( VipsForeignLoadPng *png, VipsImage *image ) +{ + /* FIXME ... load interlaced PNG to huge memory image. + */ + + return( 0 ); +} + +static int +vips_foreign_load_png_generate( VipsRegion *or, + void *seq, void *a, void *b, gboolean *stop ) +{ + VipsRect *r = &or->valid; + VipsForeignLoad *load = VIPS_FOREIGN_LOAD( a ); + VipsForeignLoadPng *png = (VipsForeignLoadPng *) load; + VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( png ); + + int y; + int error; + +#ifdef DEBUG + printf( "vips_foreign_load_png_generate: line %d, %d rows\n", + r->top, r->height ); + printf( "vips_foreign_load_png_generate: y_top = %d\n", png->y_pos ); +#endif /*DEBUG*/ + + /* We're inside a tilecache where tiles are the full image width, so + * this should always be true. + */ + g_assert( r->left == 0 ); + g_assert( r->width == or->im->Xsize ); + g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize ); + + /* Tiles should always be a strip in height, unless it's the final + * strip. + */ + g_assert( r->height == VIPS_MIN( VIPS__FATSTRIP_HEIGHT, + or->im->Ysize - r->top ) ); + + /* And check that y_pos is correct. It should be, since we are inside + * a vips_sequential(). + */ + if( r->top != png->y_pos ) { + vips_error( class->nickname, + _( "out of order read at line %d" ), png->y_pos ); + return( -1 ); + } + + for( y = 0; y < r->height; y++ ) { + error = spng_decode_row( png->ctx, + VIPS_REGION_ADDR( or, 0, r->top + y ), + VIPS_REGION_SIZEOF_LINE( or ) ); + + /* FIXME .. should allow SPNG_EOI here? + */ + if( error ) { + /* We've failed to read some pixels. Knock this + * operation out of cache. + */ + vips_operation_invalidate( VIPS_OPERATION( png ) ); + +#ifdef DEBUG + printf( "vips_foreign_load_png_generate:\n" ); + printf( " spng_decode_row() failed, line %d\n", + r->top + y ); + printf( " file %s\n", read->name ); + printf( " thread %p\n", g_thread_self() ); +#endif /*DEBUG*/ + + /* And bail if fail is on. We have to add an error + * message, since the handler we install just does + * g_warning(). + */ + if( load->fail ) { + vips_error( class->nickname, + "%s", _( "libpng read error" ) ); + return( -1 ); + } + } + + png->y_pos += 1; + } + + return( 0 ); +} + +static int +vips_foreign_load_png_load( VipsForeignLoad *load ) +{ + VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load ); + VipsForeignLoadPng *png = (VipsForeignLoadPng *) load; + VipsImage **t = (VipsImage **) + vips_object_local_array( VIPS_OBJECT( load ), 3 ); + + int error; + + if( (error = spng_decoded_image_size( png->ctx, + png->fmt, &png->out_size )) ) { + vips_error( class->nickname, "%s", spng_strerror( error ) ); + return( -1 ); + } + png->out_line_size = png->out_size / png->ihdr.height; + + /* Initialize for progressive decoding. + */ + if( (error = spng_decode_image( png->ctx, NULL, 0, + png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { + vips_error( class->nickname, "%s", spng_strerror( error ) ); + return( -1 ); + } + + t[0] = vips_image_new_memory(); + vips_foreign_load_png_set_header( png, t[0] ); + if( vips_source_decode( png->source ) ) + return( -1 ); + + if( png->ihdr.interlace_method != 0 ) { + /* Arg awful interlaced image. We have to load to a huge mem + * buffer, then copy to out. + */ + if( vips_foreign_load_png_interlace( png, t[0] ) || + vips_image_write( t[0], load->real ) ) + return( -1 ); + } + else { + if( vips_image_generate( t[0], + NULL, vips_foreign_load_png_generate, NULL, + read, NULL ) || + vips_sequential( t[0], &t[1], + "tile_height", VIPS__FATSTRIP_HEIGHT, + NULL ) || + vips_image_write( t[1], load->real ) ) + return( -1 ); + } + + return( 0 ); +} + +static void +vips_foreign_load_png_class_init( VipsForeignLoadPngClass *class ) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS( class ); + VipsObjectClass *object_class = (VipsObjectClass *) class; + VipsForeignClass *foreign_class = (VipsForeignClass *) class; + VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class; + + gobject_class->dispose = vips_foreign_load_png_dispose; + + object_class->nickname = "pngload_base"; + object_class->description = _( "load png base class" ); + + /* We are fast at is_a(), so high priority. + */ + foreign_class->priority = 200; + + load_class->get_flags_filename = + vips_foreign_load_png_get_flags_filename; + load_class->get_flags = vips_foreign_load_png_get_flags; + load_class->header = vips_foreign_load_png_header; + load_class->load = vips_foreign_load_png_load; + +} + +static void +vips_foreign_load_png_init( VipsForeignLoadPng *png ) +{ +} + +typedef struct _VipsForeignLoadPngSource { + VipsForeignLoadPng parent_object; + + /* Load from a source. + */ + VipsSource *source; + +} VipsForeignLoadPngSource; + +typedef VipsForeignLoadPngClass VipsForeignLoadPngSourceClass; + +G_DEFINE_TYPE( VipsForeignLoadPngSource, vips_foreign_load_png_source, + vips_foreign_load_png_get_type() ); + +static int +vips_foreign_load_png_source_build( VipsObject *object ) +{ + VipsForeignLoadPng *png = (VipsForeignLoadPng *) object; + VipsForeignLoadPngSource *source = (VipsForeignLoadPngSource *) object; + + if( source->source ) { + png->source = source->source; + g_object_ref( png->source ); + } + + if( VIPS_OBJECT_CLASS( vips_foreign_load_png_source_parent_class )-> + build( object ) ) + return( -1 ); + + return( 0 ); +} + +static gboolean +vips_foreign_load_png_source_is_a_source( VipsSource *source ) +{ + static unsigned char signature[8] = { 137, 80, 78, 71, 13, 10, 26, 10 }; + + const unsigned char *p; + + if( (p = vips_source_sniff( source, 8 )) && + memcmp( p, signature, 8 ) == 0 ) + return( TRUE ); + + return( FALSE ); +} + +static void +vips_foreign_load_png_source_class_init( VipsForeignLoadPngSourceClass *class ) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS( class ); + VipsObjectClass *object_class = (VipsObjectClass *) class; + VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class; + + gobject_class->set_property = vips_object_set_property; + gobject_class->get_property = vips_object_get_property; + + object_class->nickname = "pngload_source"; + object_class->description = _( "load png from source" ); + object_class->build = vips_foreign_load_png_source_build; + + load_class->is_a_source = vips_foreign_load_png_source_is_a_source; + + VIPS_ARG_OBJECT( class, "source", 1, + _( "Source" ), + _( "Source to load from" ), + VIPS_ARGUMENT_REQUIRED_INPUT, + G_STRUCT_OFFSET( VipsForeignLoadPngSource, source ), + VIPS_TYPE_SOURCE ); + +} + +static void +vips_foreign_load_png_source_init( VipsForeignLoadPngSource *source ) +{ +} + +typedef struct _VipsForeignLoadPngFile { + VipsForeignLoadPng parent_object; + + /* Filename for load. + */ + char *filename; + +} VipsForeignLoadPngFile; + +typedef VipsForeignLoadPngClass VipsForeignLoadPngFileClass; + +G_DEFINE_TYPE( VipsForeignLoadPngFile, vips_foreign_load_png_file, + vips_foreign_load_png_get_type() ); + +static int +vips_foreign_load_png_file_build( VipsObject *object ) +{ + VipsForeignLoadPng *png = (VipsForeignLoadPng *) object; + VipsForeignLoadPngFile *file = (VipsForeignLoadPngFile *) object; + + if( file->filename && + !(png->source = vips_source_new_from_file( file->filename )) ) + return( -1 ); + + if( VIPS_OBJECT_CLASS( vips_foreign_load_png_file_parent_class )-> + build( object ) ) + return( -1 ); + + return( 0 ); +} + +static gboolean +vips_foreign_load_png_file_is_a( const char *filename ) +{ + VipsSource *source; + gboolean result; + + if( !(source = vips_source_new_from_file( filename )) ) + return( FALSE ); + result = vips_foreign_load_png_source_is_a_source( source ); + VIPS_UNREF( source ); + + return( result ); +} + +const char *vips_foreign_load_png_file_suffs[] = { ".png", NULL }; + +static void +vips_foreign_load_png_file_class_init( VipsForeignLoadPngFileClass *class ) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS( class ); + VipsObjectClass *object_class = (VipsObjectClass *) class; + VipsForeignClass *foreign_class = (VipsForeignClass *) class; + VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class; + + gobject_class->set_property = vips_object_set_property; + gobject_class->get_property = vips_object_get_property; + + object_class->nickname = "pngload"; + object_class->description = _( "load png from file" ); + object_class->build = vips_foreign_load_png_file_build; + + foreign_class->suffs = vips_foreign_load_png_file_suffs; + + load_class->is_a = vips_foreign_load_png_file_is_a; + + VIPS_ARG_STRING( class, "filename", 1, + _( "Filename" ), + _( "Filename to load from" ), + VIPS_ARGUMENT_REQUIRED_INPUT, + G_STRUCT_OFFSET( VipsForeignLoadPngFile, filename ), + NULL ); +} + +static void +vips_foreign_load_png_file_init( VipsForeignLoadPngFile *file ) +{ +} + +typedef struct _VipsForeignLoadPngBuffer { + VipsForeignLoadPng parent_object; + + /* Load from a buffer. + */ + VipsBlob *blob; + +} VipsForeignLoadPngBuffer; + +typedef VipsForeignLoadPngClass VipsForeignLoadPngBufferClass; + +G_DEFINE_TYPE( VipsForeignLoadPngBuffer, vips_foreign_load_png_buffer, + vips_foreign_load_png_get_type() ); + +static int +vips_foreign_load_png_buffer_build( VipsObject *object ) +{ + VipsForeignLoadPng *png = (VipsForeignLoadPng *) object; + VipsForeignLoadPngBuffer *buffer = (VipsForeignLoadPngBuffer *) object; + + if( buffer->blob && + !(png->source = vips_source_new_from_memory( + VIPS_AREA( buffer->blob )->data, + VIPS_AREA( buffer->blob )->length )) ) + return( -1 ); + + if( VIPS_OBJECT_CLASS( vips_foreign_load_png_buffer_parent_class )-> + build( object ) ) + return( -1 ); + + return( 0 ); +} + +static gboolean +vips_foreign_load_png_buffer_is_a_buffer( const void *buf, size_t len ) +{ + VipsSource *source; + gboolean result; + + if( !(source = vips_source_new_from_memory( buf, len )) ) + return( FALSE ); + result = vips_foreign_load_png_source_is_a_source( source ); + VIPS_UNREF( source ); + + return( result ); +} + +static void +vips_foreign_load_png_buffer_class_init( VipsForeignLoadPngBufferClass *class ) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS( class ); + VipsObjectClass *object_class = (VipsObjectClass *) class; + VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class; + + gobject_class->set_property = vips_object_set_property; + gobject_class->get_property = vips_object_get_property; + + object_class->nickname = "pngload_buffer"; + object_class->description = _( "load png from buffer" ); + object_class->build = vips_foreign_load_png_buffer_build; + + load_class->is_a_buffer = vips_foreign_load_png_buffer_is_a_buffer; + + VIPS_ARG_BOXED( class, "buffer", 1, + _( "Buffer" ), + _( "Buffer to load from" ), + VIPS_ARGUMENT_REQUIRED_INPUT, + G_STRUCT_OFFSET( VipsForeignLoadPngBuffer, blob ), + VIPS_TYPE_BLOB ); + +} + +static void +vips_foreign_load_png_buffer_init( VipsForeignLoadPngBuffer *buffer ) +{ +} + +#endif /*HAVE_SPNG*/ From 53101929c5c7004a8c3a642e94795e4a3abb0cad Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 1 May 2020 20:28:03 +0100 Subject: [PATCH 02/19] workinig! --- libvips/foreign/foreign.c | 6 ++ libvips/foreign/spngload.c | 123 ++++++++++++++++--------------------- 2 files changed, 60 insertions(+), 69 deletions(-) diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c index 895c999b..e0367c4d 100644 --- a/libvips/foreign/foreign.c +++ b/libvips/foreign/foreign.c @@ -2206,6 +2206,12 @@ vips_foreign_operation_init( void ) vips_foreign_save_png_target_get_type(); #endif /*HAVE_PNG*/ +#ifdef HAVE_SPNG + vips_foreign_load_png_file_get_type(); + vips_foreign_load_png_buffer_get_type(); + vips_foreign_load_png_source_get_type(); +#endif /*HAVE_SPNG*/ + #ifdef HAVE_MATIO vips_foreign_load_mat_get_type(); #endif /*HAVE_MATIO*/ diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index e89566d2..dad90fc0 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -70,12 +70,11 @@ typedef struct _VipsForeignLoadPng { spng_ctx *ctx; struct spng_ihdr ihdr; + struct spng_trns trns; enum spng_format fmt; int bands; VipsInterpretation interpretation; VipsBandFormat format; - size_t out_size; - size_t out_line_size; int y_pos; } VipsForeignLoadPng; @@ -195,16 +194,22 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( load ); VipsForeignLoadPng *png = (VipsForeignLoadPng *) load; + int flags; int error; - /* Flags can be eg. SPNG_CTX_IGNORE_ADLER32 to ignore CRC chaekcs in - * deflate. - * - * FIXME ... fail handling + /* In non-fail mode, ignore CRC errors. */ - png->ctx = spng_ctx_new( 0 ); - spng_set_crc_action( png->ctx, SPNG_CRC_USE, SPNG_CRC_USE ); + flags = 0; + if( !load->fail ) + flags |= SPNG_CTX_IGNORE_ADLER32; + png->ctx = spng_ctx_new( flags ); + if( !load->fail ) + /* Ignore and don't calculate checksums. + */ + spng_set_crc_action( png->ctx, SPNG_CRC_USE, SPNG_CRC_USE ); + if( vips_source_rewind( png->source ) ) + return( -1 ); spng_set_png_stream( png->ctx, vips_foreign_load_png_stream, png->source ); if( (error = spng_get_ihdr( png->ctx, &png->ihdr )) ) { @@ -212,40 +217,20 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } - printf(" width: %d\nheight: %d\nbit depth: %d\ncolor type: %d\n", + /* + printf( "width: %d\nheight: %d\nbit depth: %d\ncolor type: %d\n", png->ihdr.width, png->ihdr.height, png->ihdr.bit_depth, png->ihdr.color_type ); printf( "compression method: %d\nfilter method: %d\n" "interlace method: %d\n", png->ihdr.compression_method, png->ihdr.filter_method, png->ihdr.interlace_method ); + */ - switch( png->ihdr.color_type ) { - case SPNG_COLOR_TYPE_GRAYSCALE: - png->interpretation = VIPS_INTERPRETATION_B_W; - png->bands = 1; - break; - - case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: - png->interpretation = VIPS_INTERPRETATION_B_W; - png->bands = 2; - break; - - case SPNG_COLOR_TYPE_TRUECOLOR: - case SPNG_COLOR_TYPE_INDEXED: - png->interpretation = VIPS_INTERPRETATION_sRGB; - png->bands = 3; - break; - - case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: - png->interpretation = VIPS_INTERPRETATION_sRGB; - png->bands = 4; - break; - - default: - vips_error( class->nickname, "%s", "unknown color_type" ); - return( -1 ); - } + /* For now, libspng always outputs RGBA. + */ + png->interpretation = VIPS_INTERPRETATION_sRGB; + png->bands = 4; if( png->ihdr.bit_depth == 16 ) { png->fmt = SPNG_FMT_RGBA16; @@ -260,7 +245,7 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->format = VIPS_FORMAT_UCHAR; } - /* FIXME ... get resolution. + /* FIXME ... get resolution, profile, exif, xmp, etc. etc. */ vips_source_minimise( png->source ); @@ -270,15 +255,6 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( 0 ); } -static int -vips_foreign_load_png_interlace( VipsForeignLoadPng *png, VipsImage *image ) -{ - /* FIXME ... load interlaced PNG to huge memory image. - */ - - return( 0 ); -} - static int vips_foreign_load_png_generate( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) @@ -323,10 +299,11 @@ vips_foreign_load_png_generate( VipsRegion *or, error = spng_decode_row( png->ctx, VIPS_REGION_ADDR( or, 0, r->top + y ), VIPS_REGION_SIZEOF_LINE( or ) ); - - /* FIXME .. should allow SPNG_EOI here? + /* libspng returns EOI when successfully reading the + * final line of input. */ - if( error ) { + if( error != 0 && + error != SPNG_EOI ) { /* We've failed to read some pixels. Knock this * operation out of cache. */ @@ -336,8 +313,8 @@ vips_foreign_load_png_generate( VipsRegion *or, printf( "vips_foreign_load_png_generate:\n" ); printf( " spng_decode_row() failed, line %d\n", r->top + y ); - printf( " file %s\n", read->name ); printf( " thread %p\n", g_thread_self() ); + printf( " error %s\n", spng_strerror( error ) ); #endif /*DEBUG*/ /* And bail if fail is on. We have to add an error @@ -367,23 +344,6 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) int error; - if( (error = spng_decoded_image_size( png->ctx, - png->fmt, &png->out_size )) ) { - vips_error( class->nickname, "%s", spng_strerror( error ) ); - return( -1 ); - } - png->out_line_size = png->out_size / png->ihdr.height; - - /* Initialize for progressive decoding. - */ - if( (error = spng_decode_image( png->ctx, NULL, 0, - png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { - vips_error( class->nickname, "%s", spng_strerror( error ) ); - return( -1 ); - } - - t[0] = vips_image_new_memory(); - vips_foreign_load_png_set_header( png, t[0] ); if( vips_source_decode( png->source ) ) return( -1 ); @@ -391,14 +351,39 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) /* Arg awful interlaced image. We have to load to a huge mem * buffer, then copy to out. */ - if( vips_foreign_load_png_interlace( png, t[0] ) || - vips_image_write( t[0], load->real ) ) + t[0] = vips_image_new_memory(); + vips_foreign_load_png_set_header( png, t[0] ); + if( vips_image_write_prepare( t[0] ) ) + return( -1 ); + + if( (error = spng_decode_image( png->ctx, + VIPS_IMAGE_ADDR( t[0], 0, 0 ), + VIPS_IMAGE_SIZEOF_IMAGE( t[0] ), + png->fmt, 0 )) ) { + vips_error( class->nickname, + "%s", spng_strerror( error ) ); + return( -1 ); + } + + if( vips_image_write( t[0], load->real ) ) return( -1 ); } else { + t[0] = vips_image_new(); + vips_foreign_load_png_set_header( png, t[0] ); + + /* Initialize for progressive decoding. + */ + if( (error = spng_decode_image( png->ctx, NULL, 0, + png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { + vips_error( class->nickname, + "%s", spng_strerror( error ) ); + return( -1 ); + } + if( vips_image_generate( t[0], NULL, vips_foreign_load_png_generate, NULL, - read, NULL ) || + png, NULL ) || vips_sequential( t[0], &t[1], "tile_height", VIPS__FATSTRIP_HEIGHT, NULL ) || From 10f1352f6fd798b832d03f8fc1ac693d88b81cbd Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 1 May 2020 23:20:48 +0100 Subject: [PATCH 03/19] clean up, add benchmarks --- README.md | 27 ++++++++++++++++++++++++++- configure.ac | 10 +++++----- libvips/foreign/spngload.c | 17 +++++++++-------- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 1b4c4e6d..44b5d3ce 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,8 @@ Build libspng: ``` cd libspng -meson build --prefix=/home/john/vips --libdir=/home/john/vips/lib +meson build --prefix=/home/john/vips --libdir=/home/john/vips/lib \ + --buildtype=release cd build ninja ninja install @@ -26,6 +27,30 @@ Sample code: https://github.com/randy408/libspng/blob/master/examples/example.c +libspng benchmark: + +``` +$ time vips avg wtc.png +151.549325 + +real 0m3.167s +user 0m3.714s +sys 0m0.194s +``` + +And for libpng: + +``` +$ time vips avg wtc.png +117.065766 + +real 0m3.816s +user 0m4.177s +sys 0m0.221s +``` + +The avg is different since libspng is generating alpha 255. + # Introduction libvips is a [demand-driven, horizontally diff --git a/configure.ac b/configure.ac index e44866e9..90ca3d0a 100644 --- a/configure.ac +++ b/configure.ac @@ -1176,13 +1176,13 @@ FIND_GIFLIB( ) # Look for libspng first -AC_ARG_WITH([spng], - AS_HELP_STRING([--without-spng], [build without libspng (default: test)])) +AC_ARG_WITH([libspng], + AS_HELP_STRING([--without-libspng], [build without libspng (default: test)])) -if test x"$with_spng" != x"no"; then +if test x"$with_libspng" != x"no"; then PKG_CHECK_MODULES(SPNG, spng >= 0.6, [AC_DEFINE(HAVE_SPNG,1,[define if you have libspng installed.]) - with_spng=yes + with_libspng=yes PACKAGES_USED="$PACKAGES_USED spng" with_png=no ], @@ -1413,7 +1413,7 @@ file import with cfitsio: $with_cfitsio file import/export with libwebp: $with_libwebp (requires libwebp, libwebpmux, libwebpdemux 0.6.0 or later) text rendering with pangoft2: $with_pangoft2 -file import/export with libspng: $with_spng +file import/export with libspng: $with_libspng (requires libspng-0.6 or later) file import/export with libpng: $with_png (requires libpng-1.2.9 or later) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index dad90fc0..7d9e36ab 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -36,6 +36,13 @@ * an enum for interlace_method would be nice ... ADAM7 == 1, * no interlace == 0. * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) + * + * This always reads RGBA8 or RGBA16. Other formst G8/G16/GA8 etc. etc. are + * in the roadmap. + * + * Most metadata support (eg. XMP, ICC, etc. etc.) is missing. + * + * Load only, there's no save support for now. */ /* @@ -70,7 +77,6 @@ typedef struct _VipsForeignLoadPng { spng_ctx *ctx; struct spng_ihdr ihdr; - struct spng_trns trns; enum spng_format fmt; int bands; VipsInterpretation interpretation; @@ -96,9 +102,6 @@ vips_foreign_load_png_dispose( GObject *gobject ) dispose( gobject ); } -/* libspng read callbacks should copy length bytes to dest and return 0 - * or SPNG_IO_EOF/SPNG_IO_ERROR on error. - */ static int vips_foreign_load_png_stream( spng_ctx *ctx, void *user, void *dest, size_t length ) @@ -123,7 +126,7 @@ vips_foreign_load_png_get_flags_source( VipsSource *source ) struct spng_ihdr ihdr; VipsForeignFlags flags; - ctx = spng_ctx_new( 0 ); + ctx = spng_ctx_new( SPNG_CTX_IGNORE_ADLER32 ); spng_set_crc_action( ctx, SPNG_CRC_USE, SPNG_CRC_USE ); spng_set_png_stream( ctx, vips_foreign_load_png_stream, source ); @@ -317,9 +320,7 @@ vips_foreign_load_png_generate( VipsRegion *or, printf( " error %s\n", spng_strerror( error ) ); #endif /*DEBUG*/ - /* And bail if fail is on. We have to add an error - * message, since the handler we install just does - * g_warning(). + /* And bail if fail is on. */ if( load->fail ) { vips_error( class->nickname, From d78480c84679a63f14365d6edf2638d39a052f6d Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 13 May 2020 15:25:01 +0100 Subject: [PATCH 04/19] update for SPNG_FMT_PNG We can now decode PNG to g8/g16/ga8/ga16/rgb8/rgb16 etc. --- libvips/foreign/spngload.c | 62 +++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 7d9e36ab..d7b767d4 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -37,10 +37,10 @@ * no interlace == 0. * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) * - * This always reads RGBA8 or RGBA16. Other formst G8/G16/GA8 etc. etc. are - * in the roadmap. + * test indexed decode * - * Most metadata support (eg. XMP, ICC, etc. etc.) is missing. + * Most metadata support (eg. XMP, ICC, etc. etc.) is missing. We will need + * to set a chunk size limit with spng_set_chunks_limits(). * * Load only, there's no save support for now. */ @@ -211,6 +211,14 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) */ spng_set_crc_action( png->ctx, SPNG_CRC_USE, SPNG_CRC_USE ); + /* Set limits to avoid decompression bombs. We should set + * spng_set_chunks_limits() as well, once that API goes in. + * + * No need to test the decoded image size -- the user can do that if + * they wish. + */ + spng_set_image_limits( png->ctx, VIPS_MAX_COORD, VIPS_MAX_COORD ); + if( vips_source_rewind( png->source ) ) return( -1 ); spng_set_png_stream( png->ctx, @@ -230,23 +238,55 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->ihdr.interlace_method ); */ - /* For now, libspng always outputs RGBA. - */ - png->interpretation = VIPS_INTERPRETATION_sRGB; - png->bands = 4; + + switch( png->ihdr.color_type ) { + case SPNG_COLOR_TYPE_GRAYSCALE: + png->bands = 1; + png->interpretation = VIPS_INTERPRETATION_B_W; + png->fmt = SPNG_FMT_PNG; + break; + + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: + png->bands = 2; + png->interpretation = VIPS_INTERPRETATION_B_W; + png->fmt = SPNG_FMT_PNG; + break; + + case SPNG_COLOR_TYPE_TRUECOLOR: + png->bands = 3; + png->interpretation = VIPS_INTERPRETATION_sRGB; + png->fmt = SPNG_FMT_PNG; + break; + + case SPNG_COLOR_TYPE_INDEXED: + png->bands = 3; + png->interpretation = VIPS_INTERPRETATION_sRGB; + + /* Expand indexed images to RGB8. + */ + png->fmt = SPNG_FMT_RGB8; + break; + + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: + png->bands = 4; + png->interpretation = VIPS_INTERPRETATION_sRGB; + png->fmt = SPNG_FMT_PNG; + break; + + default: + vips_error( class->nickname, "%s", _( "unknown color type" ) ); + return( -1 ); + } if( png->ihdr.bit_depth == 16 ) { - png->fmt = SPNG_FMT_RGBA16; png->format = VIPS_FORMAT_USHORT; if( png->interpretation == VIPS_INTERPRETATION_B_W ) png->interpretation = VIPS_INTERPRETATION_GREY16; if( png->interpretation == VIPS_INTERPRETATION_sRGB ) png->interpretation = VIPS_INTERPRETATION_RGB16; } - else { - png->fmt = SPNG_FMT_RGBA8; + else png->format = VIPS_FORMAT_UCHAR; - } /* FIXME ... get resolution, profile, exif, xmp, etc. etc. */ From c2165671e25981a0e06925c36ddeb1e66610a9c1 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 13 May 2020 16:38:35 +0100 Subject: [PATCH 05/19] update benchmark result --- README.md | 10 ++++------ libvips/foreign/spngload.c | 13 +++++++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 44b5d3ce..89ae80a5 100644 --- a/README.md +++ b/README.md @@ -31,11 +31,11 @@ libspng benchmark: ``` $ time vips avg wtc.png -151.549325 +117.065766 -real 0m3.167s -user 0m3.714s -sys 0m0.194s +real 0m2.972s +user 0m3.376s +sys 0m0.197s ``` And for libpng: @@ -49,8 +49,6 @@ user 0m4.177s sys 0m0.221s ``` -The avg is different since libspng is generating alpha 255. - # Introduction libvips is a [demand-driven, horizontally diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index d7b767d4..a037ada4 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -31,18 +31,19 @@ */ -/* Notes: +/* TODO + * + * an enum for interlace_method would be nice ... ADAM7 == 1, no interlace == 0 * - * an enum for interlace_method would be nice ... ADAM7 == 1, - * no interlace == 0. * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) * * test indexed decode * - * Most metadata support (eg. XMP, ICC, etc. etc.) is missing. We will need - * to set a chunk size limit with spng_set_chunks_limits(). + * most metadata support (eg. XMP, ICC, etc. etc.) is missing ... we will need + * to set a chunk size limit with spng_set_chunks_limits() + * + * load only, there's no save support for now * - * Load only, there's no save support for now. */ /* From ef408d630bfec83ffe5446df4ddc656d85ed1b72 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 7 Jun 2020 14:00:10 +0100 Subject: [PATCH 06/19] start adding ICC and XMP support not quite working though ... seems to need a fix in libspng still --- libvips/foreign/spngload.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index a037ada4..0ed2330b 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -171,6 +171,11 @@ vips_foreign_load_png_get_flags_filename( const char *filename ) static void vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) { + struct spng_iccp iccp; + struct spng_text *text; + guint32 n_text; + int ret; + vips_image_init_fields( image, png->ihdr.width, png->ihdr.height, png->bands, png->format, VIPS_CODING_NONE, png->interpretation, @@ -178,6 +183,34 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) VIPS_SETSTR( image->filename, vips_connection_filename( VIPS_CONNECTION( png->source ) ) ); + /* Attach ICC profile. + */ + if( !spng_get_iccp( png->ctx, &iccp ) ) { + printf( "attaching profile %s, %zd bytes\n", + iccp.profile_name, iccp.profile_len ); + + vips_image_set_blob_copy( image, + VIPS_META_ICC_NAME, iccp.profile, iccp.profile_len ); + } + + /* + spng_get_text( png->ctx, NULL, &n_text ); + printf( "reading %" G_GUINT32_FORMAT " text chunks\n", n_text ); + text = VIPS_ARRAY( VIPS_OBJECT( png ), n_text, struct spng_text ); + if( !spng_get_text( png->ctx, text, &n_text ) ) { + guint32 i; + + for( i = 0; i < n_text; i++ ) { + printf( "chunk %d: keyword %s, type %d, length %zd\n", + i, text[i].keyword, text[i].type, + text[i].length ); + } + } + */ + + /* FIXME ... get resolution, exif, xmp, etc. etc. + */ + /* 0 is no interlace. */ if( png->ihdr.interlace_method == 0 ) @@ -289,9 +322,6 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) else png->format = VIPS_FORMAT_UCHAR; - /* FIXME ... get resolution, profile, exif, xmp, etc. etc. - */ - vips_source_minimise( png->source ); vips_foreign_load_png_set_header( png, load->out ); From fc872421aa879154c86574203bcc1942261a6343 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 7 Jun 2020 15:17:37 +0100 Subject: [PATCH 07/19] add exif/icc/text text chunk read using new libspng 0.6 features --- libvips/foreign/spngload.c | 102 ++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 29 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 0ed2330b..9d1a7a29 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -168,49 +168,56 @@ vips_foreign_load_png_get_flags_filename( const char *filename ) return( flags ); } +/* Set the png text data as metadata on the vips image. These are always + * null-terminated strings. + */ +static void +vips_foreign_load_png_set_text( VipsImage *out, + int i, const char *key, const char *value ) +{ +#ifdef DEBUG + printf( "vips_foreign_load_png_set_text: key %s, value %s\n", + key, value ); +#endif /*DEBUG*/ + + if( strcmp( key, "XML:com.adobe.xmp" ) == 0 ) { + /* Save as an XMP tag. This must be a BLOB, for compatibility + * for things like the XMP blob that the tiff loader adds. + * + * Note that this will remove the null-termination from the + * string. We must carefully reattach this. + */ + vips_image_set_blob_copy( out, + VIPS_META_XMP_NAME, value, strlen( value ) ); + } + else { + char name[256]; + + /* Save as a string comment. Some PNGs have EXIF data as + * text segments, unfortunately. + */ + vips_snprintf( name, 256, "png-comment-%d-%s", i, key ); + + vips_image_set_string( out, name, value ); + } +} + static void vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) { struct spng_iccp iccp; struct spng_text *text; + struct spng_exif exif; guint32 n_text; - int ret; vips_image_init_fields( image, png->ihdr.width, png->ihdr.height, png->bands, png->format, VIPS_CODING_NONE, png->interpretation, 1.0, 1.0 ); + VIPS_SETSTR( image->filename, vips_connection_filename( VIPS_CONNECTION( png->source ) ) ); - /* Attach ICC profile. - */ - if( !spng_get_iccp( png->ctx, &iccp ) ) { - printf( "attaching profile %s, %zd bytes\n", - iccp.profile_name, iccp.profile_len ); - - vips_image_set_blob_copy( image, - VIPS_META_ICC_NAME, iccp.profile, iccp.profile_len ); - } - - /* - spng_get_text( png->ctx, NULL, &n_text ); - printf( "reading %" G_GUINT32_FORMAT " text chunks\n", n_text ); - text = VIPS_ARRAY( VIPS_OBJECT( png ), n_text, struct spng_text ); - if( !spng_get_text( png->ctx, text, &n_text ) ) { - guint32 i; - - for( i = 0; i < n_text; i++ ) { - printf( "chunk %d: keyword %s, type %d, length %zd\n", - i, text[i].keyword, text[i].type, - text[i].length ); - } - } - */ - - /* FIXME ... get resolution, exif, xmp, etc. etc. - */ - /* 0 is no interlace. */ if( png->ihdr.interlace_method == 0 ) @@ -223,6 +230,43 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) /* Interlaced images are read via a huge memory buffer. */ vips_image_pipelinev( image, VIPS_DEMAND_STYLE_ANY, NULL ); + + if( !spng_get_iccp( png->ctx, &iccp ) ) + vips_image_set_blob_copy( image, + VIPS_META_ICC_NAME, iccp.profile, iccp.profile_len ); + + spng_get_text( png->ctx, NULL, &n_text ); + text = VIPS_ARRAY( VIPS_OBJECT( png ), n_text, struct spng_text ); + if( !spng_get_text( png->ctx, text, &n_text ) ) { + guint32 i; + + for( i = 0; i < n_text; i++ ) + /* .text is always a null-terminated C string. + */ + vips_foreign_load_png_set_text( image, + i, text[i].keyword, text[i].text ); + } + + if( !spng_get_exif( png->ctx, &exif ) ) + vips_image_set_blob_copy( image, VIPS_META_EXIF_NAME, + exif.data, exif.length ); + + /* Attach original palette bit depth, if any, as metadata. + * + * FIXME ... when we get palette support done + * + if( color_type == PNG_COLOR_TYPE_PALETTE ) + vips_image_set_int( out, "palette-bit-depth", bit_depth ); + */ + + /* Let our caller know. These are very expensive to decode. + */ + if( png->ihdr.interlace_method != 0 ) + vips_image_set_int( image, "interlaced", 1 ); + + /* FIXME ... get resolution, etc. etc. + */ + } static int From 6705ff85b5de76c49c1c527777a55a8ce1611ff0 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 7 Jun 2020 15:29:48 +0100 Subject: [PATCH 08/19] use INTERLACE_NONE --- libvips/foreign/spngload.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 9d1a7a29..319eafb2 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -32,15 +32,13 @@ */ /* TODO - * - * an enum for interlace_method would be nice ... ADAM7 == 1, no interlace == 0 * * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) + * would be useful * * test indexed decode * - * most metadata support (eg. XMP, ICC, etc. etc.) is missing ... we will need - * to set a chunk size limit with spng_set_chunks_limits() + * set a chunk size limit with spng_set_chunks_limits() when that API is done * * load only, there's no save support for now * @@ -138,7 +136,7 @@ vips_foreign_load_png_get_flags_source( VipsSource *source ) spng_ctx_free( ctx ); flags = 0; - if( ihdr.interlace_method != 0 ) + if( ihdr.interlace_method != SPNG_INTERLACE_NONE ) flags |= VIPS_FOREIGN_PARTIAL; else flags |= VIPS_FOREIGN_SEQUENTIAL; @@ -220,7 +218,7 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) /* 0 is no interlace. */ - if( png->ihdr.interlace_method == 0 ) + if( png->ihdr.interlace_method == SPNG_INTERLACE_NONE ) /* Sequential mode needs thinstrip to work with things like * vips_shrink(). */ @@ -261,7 +259,7 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) /* Let our caller know. These are very expensive to decode. */ - if( png->ihdr.interlace_method != 0 ) + if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) vips_image_set_int( image, "interlaced", 1 ); /* FIXME ... get resolution, etc. etc. @@ -463,7 +461,7 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) if( vips_source_decode( png->source ) ) return( -1 ); - if( png->ihdr.interlace_method != 0 ) { + if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) { /* Arg awful interlaced image. We have to load to a huge mem * buffer, then copy to out. */ From 20c9a7f7cc24965df486331738d181f641c075af Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 7 Jun 2020 17:27:24 +0100 Subject: [PATCH 09/19] improve transparency support --- libvips/foreign/spngload.c | 39 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 319eafb2..8b43aac9 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -41,12 +41,11 @@ * set a chunk size limit with spng_set_chunks_limits() when that API is done * * load only, there's no save support for now - * */ /* -#define DEBUG */ +#define DEBUG #ifdef HAVE_CONFIG_H #include @@ -316,37 +315,28 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) switch( png->ihdr.color_type ) { + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: case SPNG_COLOR_TYPE_GRAYSCALE: png->bands = 1; - png->interpretation = VIPS_INTERPRETATION_B_W; - png->fmt = SPNG_FMT_PNG; - break; - - case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: - png->bands = 2; + if( !spng_get_trns( png->ctx ) ) + png->bands += 1; png->interpretation = VIPS_INTERPRETATION_B_W; png->fmt = SPNG_FMT_PNG; break; case SPNG_COLOR_TYPE_TRUECOLOR: + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: png->bands = 3; + if( !spng_get_trns( png->ctx ) ) + png->bands += 1; png->interpretation = VIPS_INTERPRETATION_sRGB; png->fmt = SPNG_FMT_PNG; break; case SPNG_COLOR_TYPE_INDEXED: - png->bands = 3; - png->interpretation = VIPS_INTERPRETATION_sRGB; - - /* Expand indexed images to RGB8. - */ - png->fmt = SPNG_FMT_RGB8; - break; - - case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: png->bands = 4; png->interpretation = VIPS_INTERPRETATION_sRGB; - png->fmt = SPNG_FMT_PNG; + png->fmt = SPNG_FMT_RGBA8; break; default: @@ -457,10 +447,15 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) vips_object_local_array( VIPS_OBJECT( load ), 3 ); int error; + enum spng_decode_flags; if( vips_source_decode( png->source ) ) return( -1 ); + /* We want the alpha, if present. + */ + flags = SPNG_DECODE_TRNS; + if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) { /* Arg awful interlaced image. We have to load to a huge mem * buffer, then copy to out. @@ -473,7 +468,7 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) if( (error = spng_decode_image( png->ctx, VIPS_IMAGE_ADDR( t[0], 0, 0 ), VIPS_IMAGE_SIZEOF_IMAGE( t[0] ), - png->fmt, 0 )) ) { + png->fmt, flags )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 ); @@ -486,10 +481,12 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) t[0] = vips_image_new(); vips_foreign_load_png_set_header( png, t[0] ); - /* Initialize for progressive decoding. + /* We can decode regular PNGs progressively. */ + flags |= SPNG_DECODE_PROGRESSIVE; + if( (error = spng_decode_image( png->ctx, NULL, 0, - png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { + png->fmt, flags )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 ); From 70a1dc09262be4dbe4ef4e6df45b0c2ed1dafce8 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 9 Jun 2020 02:04:55 +0100 Subject: [PATCH 10/19] Revert "improve transparency support" This reverts commit 20c9a7f7cc24965df486331738d181f641c075af. --- libvips/foreign/spngload.c | 39 ++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 8b43aac9..319eafb2 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -41,11 +41,12 @@ * set a chunk size limit with spng_set_chunks_limits() when that API is done * * load only, there's no save support for now + * */ /* - */ #define DEBUG + */ #ifdef HAVE_CONFIG_H #include @@ -315,28 +316,37 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) switch( png->ihdr.color_type ) { - case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: case SPNG_COLOR_TYPE_GRAYSCALE: png->bands = 1; - if( !spng_get_trns( png->ctx ) ) - png->bands += 1; + png->interpretation = VIPS_INTERPRETATION_B_W; + png->fmt = SPNG_FMT_PNG; + break; + + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: + png->bands = 2; png->interpretation = VIPS_INTERPRETATION_B_W; png->fmt = SPNG_FMT_PNG; break; case SPNG_COLOR_TYPE_TRUECOLOR: - case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: png->bands = 3; - if( !spng_get_trns( png->ctx ) ) - png->bands += 1; png->interpretation = VIPS_INTERPRETATION_sRGB; png->fmt = SPNG_FMT_PNG; break; case SPNG_COLOR_TYPE_INDEXED: + png->bands = 3; + png->interpretation = VIPS_INTERPRETATION_sRGB; + + /* Expand indexed images to RGB8. + */ + png->fmt = SPNG_FMT_RGB8; + break; + + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: png->bands = 4; png->interpretation = VIPS_INTERPRETATION_sRGB; - png->fmt = SPNG_FMT_RGBA8; + png->fmt = SPNG_FMT_PNG; break; default: @@ -447,15 +457,10 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) vips_object_local_array( VIPS_OBJECT( load ), 3 ); int error; - enum spng_decode_flags; if( vips_source_decode( png->source ) ) return( -1 ); - /* We want the alpha, if present. - */ - flags = SPNG_DECODE_TRNS; - if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) { /* Arg awful interlaced image. We have to load to a huge mem * buffer, then copy to out. @@ -468,7 +473,7 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) if( (error = spng_decode_image( png->ctx, VIPS_IMAGE_ADDR( t[0], 0, 0 ), VIPS_IMAGE_SIZEOF_IMAGE( t[0] ), - png->fmt, flags )) ) { + png->fmt, 0 )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 ); @@ -481,12 +486,10 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) t[0] = vips_image_new(); vips_foreign_load_png_set_header( png, t[0] ); - /* We can decode regular PNGs progressively. + /* Initialize for progressive decoding. */ - flags |= SPNG_DECODE_PROGRESSIVE; - if( (error = spng_decode_image( png->ctx, NULL, 0, - png->fmt, flags )) ) { + png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 ); From f113e64515682f1bfca40cbf92914bbc48d32e86 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 9 Jun 2020 02:27:46 +0100 Subject: [PATCH 11/19] update for latest spng 1/2/4 bit handling --- libvips/foreign/spngload.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 319eafb2..05e33070 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -275,6 +275,7 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) int flags; int error; + struct spng_trns trns; /* In non-fail mode, ignore CRC errors. */ @@ -354,6 +355,23 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } + if( !spng_get_trns( png->ctx, &trns ) ) { + if( png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR ) { + if( png->ihdr.bit_depth == 16 ) + png->fmt = SPNG_FMT_RGBA16; + else + png->fmt = SPNG_FMT_RGBA8; + } + else if( png->ihdr.color_type == SPNG_COLOR_TYPE_INDEXED ) + png->fmt = SPNG_FMT_RGBA8; + else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE ) { + if( png->ihdr.bit_depth == 16 ) + png->fmt = SPNG_FMT_GA16; + else + png->fmt = SPNG_FMT_GA8; + } + } + if( png->ihdr.bit_depth == 16 ) { png->format = VIPS_FORMAT_USHORT; if( png->interpretation == VIPS_INTERPRETATION_B_W ) From ac96bb80b5a66b3662d405bb2e122e5f5f530b04 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 9 Jun 2020 02:38:38 +0100 Subject: [PATCH 12/19] try revising spng FMT handling again --- libvips/foreign/spngload.c | 52 +++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 05e33070..2af0229a 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -318,21 +318,33 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) switch( png->ihdr.color_type ) { case SPNG_COLOR_TYPE_GRAYSCALE: + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: png->bands = 1; png->interpretation = VIPS_INTERPRETATION_B_W; png->fmt = SPNG_FMT_PNG; - break; - case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: - png->bands = 2; - png->interpretation = VIPS_INTERPRETATION_B_W; - png->fmt = SPNG_FMT_PNG; + if( !spng_get_trns( png->ctx, &trns ) ) { + png->bands += 1; + + /* Expand 1/2/4 bit images to 8 bit. + */ + if( png->ihdr.bit_depth < 8 ) + png->fmt = SPNG_FMT_GA8; + } + else { + if( png->ihdr.bit_depth < 8 ) + png->fmt = SPNG_FMT_G8; + } break; case SPNG_COLOR_TYPE_TRUECOLOR: + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: png->bands = 3; png->interpretation = VIPS_INTERPRETATION_sRGB; png->fmt = SPNG_FMT_PNG; + + if( !spng_get_trns( png->ctx, &trns ) ) + png->bands += 1; break; case SPNG_COLOR_TYPE_INDEXED: @@ -341,13 +353,12 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) /* Expand indexed images to RGB8. */ - png->fmt = SPNG_FMT_RGB8; - break; - - case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: - png->bands = 4; - png->interpretation = VIPS_INTERPRETATION_sRGB; - png->fmt = SPNG_FMT_PNG; + if( !spng_get_trns( png->ctx, &trns ) ) { + png->bands += 1; + png->fmt = SPNG_FMT_RGBA8; + } + else + png->fmt = SPNG_FMT_RGB8; break; default: @@ -355,23 +366,6 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } - if( !spng_get_trns( png->ctx, &trns ) ) { - if( png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR ) { - if( png->ihdr.bit_depth == 16 ) - png->fmt = SPNG_FMT_RGBA16; - else - png->fmt = SPNG_FMT_RGBA8; - } - else if( png->ihdr.color_type == SPNG_COLOR_TYPE_INDEXED ) - png->fmt = SPNG_FMT_RGBA8; - else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE ) { - if( png->ihdr.bit_depth == 16 ) - png->fmt = SPNG_FMT_GA16; - else - png->fmt = SPNG_FMT_GA8; - } - } - if( png->ihdr.bit_depth == 16 ) { png->format = VIPS_FORMAT_USHORT; if( png->interpretation == VIPS_INTERPRETATION_B_W ) From ce63fc1145c51d5ddc8e9753c6067e7d595fd8b7 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 9 Jun 2020 14:38:04 +0100 Subject: [PATCH 13/19] use libspng for load, libpng for save --- configure.ac | 1 - libvips/foreign/pngload.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index dda267bd..9843224d 100644 --- a/configure.ac +++ b/configure.ac @@ -1188,7 +1188,6 @@ if test x"$with_libspng" != x"no"; then [AC_DEFINE(HAVE_SPNG,1,[define if you have libspng installed.]) with_libspng=yes PACKAGES_USED="$PACKAGES_USED spng" - with_png=no ], [ ] diff --git a/libvips/foreign/pngload.c b/libvips/foreign/pngload.c index 1c4d34b0..8092078e 100644 --- a/libvips/foreign/pngload.c +++ b/libvips/foreign/pngload.c @@ -50,7 +50,7 @@ #include "pforeign.h" -#ifdef HAVE_PNG +#if defined(HAVE_PNG) && !defined(HAVE_SPNG) typedef struct _VipsForeignLoadPng { VipsForeignLoad parent_object; @@ -387,7 +387,7 @@ vips_foreign_load_png_buffer_init( VipsForeignLoadPngBuffer *buffer ) { } -#endif /*HAVE_PNG*/ +#endif /*defined(HAVE_PNG) && !defined(HAVE_SPNG)*/ /** * vips_pngload: From 0ff30f972d736645f459d6a774f9f9c545f1b761 Mon Sep 17 00:00:00 2001 From: Randy Date: Tue, 9 Jun 2020 19:56:23 +0200 Subject: [PATCH 14/19] revise spngload --- libvips/foreign/spngload.c | 106 +++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 2af0229a..37ecbee8 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -315,50 +315,23 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->ihdr.interlace_method ); */ + /* Just convert to host-endian if nothing else applies. + */ + png->fmt = SPNG_FMT_PNG; switch( png->ihdr.color_type ) { - case SPNG_COLOR_TYPE_GRAYSCALE: - case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: - png->bands = 1; - png->interpretation = VIPS_INTERPRETATION_B_W; - png->fmt = SPNG_FMT_PNG; - - if( !spng_get_trns( png->ctx, &trns ) ) { - png->bands += 1; - - /* Expand 1/2/4 bit images to 8 bit. - */ - if( png->ihdr.bit_depth < 8 ) - png->fmt = SPNG_FMT_GA8; - } - else { - if( png->ihdr.bit_depth < 8 ) - png->fmt = SPNG_FMT_G8; - } + case SPNG_COLOR_TYPE_INDEXED: + bands = 3; break; - case SPNG_COLOR_TYPE_TRUECOLOR: - case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: - png->bands = 3; - png->interpretation = VIPS_INTERPRETATION_sRGB; - png->fmt = SPNG_FMT_PNG; - - if( !spng_get_trns( png->ctx, &trns ) ) - png->bands += 1; + case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: + case SPNG_COLOR_TYPE_GRAYSCALE: + bands = 1; break; - case SPNG_COLOR_TYPE_INDEXED: - png->bands = 3; - png->interpretation = VIPS_INTERPRETATION_sRGB; - - /* Expand indexed images to RGB8. - */ - if( !spng_get_trns( png->ctx, &trns ) ) { - png->bands += 1; - png->fmt = SPNG_FMT_RGBA8; - } - else - png->fmt = SPNG_FMT_RGB8; + case SPNG_COLOR_TYPE_RGB: + case SPNG_COLOR_TYPE_RGB_ALPHA: + bands = 3; break; default: @@ -366,15 +339,56 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } - if( png->ihdr.bit_depth == 16 ) { - png->format = VIPS_FORMAT_USHORT; - if( png->interpretation == VIPS_INTERPRETATION_B_W ) - png->interpretation = VIPS_INTERPRETATION_GREY16; - if( png->interpretation == VIPS_INTERPRETATION_sRGB ) - png->interpretation = VIPS_INTERPRETATION_RGB16; + if( bit_depth > 8 ) { + if( bands < 3 ) + interpretation = VIPS_INTERPRETATION_GREY16; + else + interpretation = VIPS_INTERPRETATION_RGB16; + } + else { + if( bands < 3 ) + interpretation = VIPS_INTERPRETATION_B_W; + else + interpretation = VIPS_INTERPRETATION_sRGB; + } + + /* Expand palette images. + */ + if( png->ihdr.color_type == SPNG_COLOR_TYPE_INDEXED ) + png->fmt = SPNG_FMT_RGB8; + + /* Expand <8 bit images to full bytes. + */ + if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE && + png->ihdr.bit_depth < 8 ) + png->fmt = SPNG_FMT_G8; + + /* Expand transparency. + */ + if( !spng_get_trns( png->ctx, &trns ) ) { + bands += 1; + if( png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR ) { + if( png->ihdr.bit_depth == 16 ) + png->fmt = SPNG_FMT_RGBA16; + else + png->fmt = SPNG_FMT_RGBA8; + } + else if( png->ihdr.color_type == SPNG_COLOR_TYPE_INDEXED ) + png->fmt = SPNG_FMT_RGBA8; + else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE ) { + if( png->ihdr.bit_depth == 16 ) + png->fmt = SPNG_FMT_GA16; + else + png->fmt = SPNG_FMT_GA8; + } + } + else if( png->ihdr.color_type == PNG_COLOR_TYPE_GRAY_ALPHA || + png->ihdr.color_type == PNG_COLOR_TYPE_RGB_ALPHA ) { + /* Some images have their own alpha channel, + * not just a transparent color. + */ + bands += 1; } - else - png->format = VIPS_FORMAT_UCHAR; vips_source_minimise( png->source ); From 2457e6768a51655853377411f05c3bd5c0b2eebd Mon Sep 17 00:00:00 2001 From: Randy Date: Tue, 9 Jun 2020 20:04:03 +0200 Subject: [PATCH 15/19] fix enums --- libvips/foreign/spngload.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 37ecbee8..74545756 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -382,8 +382,8 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->fmt = SPNG_FMT_GA8; } } - else if( png->ihdr.color_type == PNG_COLOR_TYPE_GRAY_ALPHA || - png->ihdr.color_type == PNG_COLOR_TYPE_RGB_ALPHA ) { + else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE_ALPHA || + png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR_ALPHA ) { /* Some images have their own alpha channel, * not just a transparent color. */ From 49df5f54c16e53d92aa2d1f9e250640931d6c9da Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 11 Jun 2020 12:17:58 +0100 Subject: [PATCH 16/19] Randy's loader patch compiles --- configure.ac | 2 +- libvips/foreign/spngload.c | 42 +++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/configure.ac b/configure.ac index 9843224d..3984100a 100644 --- a/configure.ac +++ b/configure.ac @@ -1189,7 +1189,7 @@ if test x"$with_libspng" != x"no"; then with_libspng=yes PACKAGES_USED="$PACKAGES_USED spng" ], - [ + [with_libspng=no ] ) fi diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 74545756..362d3aa8 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -315,23 +315,23 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->ihdr.interlace_method ); */ - /* Just convert to host-endian if nothing else applies. + /* Just convert to host-endian if nothing else applies. */ png->fmt = SPNG_FMT_PNG; switch( png->ihdr.color_type ) { case SPNG_COLOR_TYPE_INDEXED: - bands = 3; + png->bands = 3; break; case SPNG_COLOR_TYPE_GRAYSCALE_ALPHA: case SPNG_COLOR_TYPE_GRAYSCALE: - bands = 1; + png->bands = 1; break; - case SPNG_COLOR_TYPE_RGB: - case SPNG_COLOR_TYPE_RGB_ALPHA: - bands = 3; + case SPNG_COLOR_TYPE_TRUECOLOR: + case SPNG_COLOR_TYPE_TRUECOLOR_ALPHA: + png->bands = 3; break; default: @@ -339,17 +339,17 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } - if( bit_depth > 8 ) { - if( bands < 3 ) - interpretation = VIPS_INTERPRETATION_GREY16; + if( png->ihdr.bit_depth > 8 ) { + if( png->bands < 3 ) + png->interpretation = VIPS_INTERPRETATION_GREY16; else - interpretation = VIPS_INTERPRETATION_RGB16; + png->interpretation = VIPS_INTERPRETATION_RGB16; } else { - if( bands < 3 ) - interpretation = VIPS_INTERPRETATION_B_W; + if( png->bands < 3 ) + png->interpretation = VIPS_INTERPRETATION_B_W; else - interpretation = VIPS_INTERPRETATION_sRGB; + png->interpretation = VIPS_INTERPRETATION_sRGB; } /* Expand palette images. @@ -360,13 +360,14 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) /* Expand <8 bit images to full bytes. */ if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE && - png->ihdr.bit_depth < 8 ) - png->fmt = SPNG_FMT_G8; + png->ihdr.bit_depth < 8 ) + png->fmt = SPNG_FMT_G8; /* Expand transparency. */ if( !spng_get_trns( png->ctx, &trns ) ) { - bands += 1; + png->bands += 1; + if( png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR ) { if( png->ihdr.bit_depth == 16 ) png->fmt = SPNG_FMT_RGBA16; @@ -383,12 +384,11 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) } } else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE_ALPHA || - png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR_ALPHA ) { - /* Some images have their own alpha channel, - * not just a transparent color. + png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR_ALPHA ) + /* Some images have an alpha channel, just not a transparent + * colour. */ - bands += 1; - } + png->bands += 1; vips_source_minimise( png->source ); From b8be8ec65977e8008a9856c8cda8fab3ff30b1bc Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 11 Jun 2020 12:53:46 +0100 Subject: [PATCH 17/19] pngsuite passes --- libvips/foreign/spngload.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 362d3aa8..3a8a6d28 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -339,17 +339,23 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) return( -1 ); } + /* Set libvips format and interpretation. + */ if( png->ihdr.bit_depth > 8 ) { if( png->bands < 3 ) png->interpretation = VIPS_INTERPRETATION_GREY16; else png->interpretation = VIPS_INTERPRETATION_RGB16; + + png->format = VIPS_FORMAT_USHORT; } else { if( png->bands < 3 ) png->interpretation = VIPS_INTERPRETATION_B_W; else png->interpretation = VIPS_INTERPRETATION_sRGB; + + png->format = VIPS_FORMAT_UCHAR; } /* Expand palette images. @@ -364,8 +370,15 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->fmt = SPNG_FMT_G8; /* Expand transparency. + * + * The _ALPHA types should not have the optional trns chunk (they + * always have a transparent band), see + * https://www.w3.org/TR/2003/REC-PNG-20031110/#11tRNS */ - if( !spng_get_trns( png->ctx, &trns ) ) { + if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE_ALPHA || + png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR_ALPHA ) + png->bands += 1; + else if( !spng_get_trns( png->ctx, &trns ) ) { png->bands += 1; if( png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR ) { @@ -383,12 +396,6 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) png->fmt = SPNG_FMT_GA8; } } - else if( png->ihdr.color_type == SPNG_COLOR_TYPE_GRAYSCALE_ALPHA || - png->ihdr.color_type == SPNG_COLOR_TYPE_TRUECOLOR_ALPHA ) - /* Some images have an alpha channel, just not a transparent - * colour. - */ - png->bands += 1; vips_source_minimise( png->source ); From 61628eefdff38139ae0c4657e1555f9e0085e3b6 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 11 Jun 2020 13:24:27 +0100 Subject: [PATCH 18/19] final fixes for spng loader --- ChangeLog | 1 + libvips/foreign/spngload.c | 50 +++++++++++++++++--------------------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index e7968f23..31a52a17 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,7 @@ - thumbnail exploits subifd pyramids - handle all EXIF orientation cases, deprecate vips_autorot_get_angle() [Elad-Laufer] +- load PNGs with libspng, if possible 24/4/20 started 8.9.3 - better iiif tile naming [IllyaMoskvin] diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 3a8a6d28..8ba77a4f 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -1,6 +1,6 @@ /* load PNG with libspng * - * 5/12/11 + * 1/5/20 * - from pngload.c */ @@ -31,19 +31,6 @@ */ -/* TODO - * - * an equivalent of png_sig_cmp() from libpng (is_a_png on a memory area) - * would be useful - * - * test indexed decode - * - * set a chunk size limit with spng_set_chunks_limits() when that API is done - * - * load only, there's no save support for now - * - */ - /* #define DEBUG */ @@ -203,21 +190,33 @@ vips_foreign_load_png_set_text( VipsImage *out, static void vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) { + double xres, yres; struct spng_iccp iccp; struct spng_text *text; struct spng_exif exif; + struct spng_phys phys; guint32 n_text; + /* Get resolution. Default to 72 pixels per inch. + */ + xres = (72.0 / 2.54 * 100.0); + yres = (72.0 / 2.54 * 100.0); + if( !spng_get_phys( png->ctx, &phys ) ) { + /* There's phys.units, but it's always 0, meaning pixels per + * metre. + */ + xres = phys.ppu_x / 1000.0; + yres = phys.ppu_y / 1000.0; + } + vips_image_init_fields( image, png->ihdr.width, png->ihdr.height, png->bands, png->format, VIPS_CODING_NONE, png->interpretation, - 1.0, 1.0 ); + xres, yres ); VIPS_SETSTR( image->filename, vips_connection_filename( VIPS_CONNECTION( png->source ) ) ); - /* 0 is no interlace. - */ if( png->ihdr.interlace_method == SPNG_INTERLACE_NONE ) /* Sequential mode needs thinstrip to work with things like * vips_shrink(). @@ -250,21 +249,15 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image ) exif.data, exif.length ); /* Attach original palette bit depth, if any, as metadata. - * - * FIXME ... when we get palette support done - * - if( color_type == PNG_COLOR_TYPE_PALETTE ) - vips_image_set_int( out, "palette-bit-depth", bit_depth ); */ + if( png->ihdr.color_type == SPNG_COLOR_TYPE_INDEXED ) + vips_image_set_int( image, + "palette-bit-depth", png->ihdr.bit_depth ); /* Let our caller know. These are very expensive to decode. */ if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) vips_image_set_int( image, "interlaced", 1 ); - - /* FIXME ... get resolution, etc. etc. - */ - } static int @@ -288,13 +281,14 @@ vips_foreign_load_png_header( VipsForeignLoad *load ) */ spng_set_crc_action( png->ctx, SPNG_CRC_USE, SPNG_CRC_USE ); - /* Set limits to avoid decompression bombs. We should set - * spng_set_chunks_limits() as well, once that API goes in. + /* Set limits to avoid decompression bombs. Set chunk limits to 60mb + * -- we've seen 50mb XMP blocks in the wild. * * No need to test the decoded image size -- the user can do that if * they wish. */ spng_set_image_limits( png->ctx, VIPS_MAX_COORD, VIPS_MAX_COORD ); + spng_set_chunk_limits( png->ctx, 60 * 1024 * 1024, 60 * 1024 * 1024 ); if( vips_source_rewind( png->source ) ) return( -1 ); From f96f2d301492baaf206f5655694ca89c7f5c6085 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 11 Jun 2020 13:49:51 +0100 Subject: [PATCH 19/19] SPNG_DECODE_TRNS flag was missing see https://github.com/libvips/libvips/pull/1682#pullrequestreview-428866084 --- libvips/foreign/spngload.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libvips/foreign/spngload.c b/libvips/foreign/spngload.c index 8ba77a4f..ac576e65 100644 --- a/libvips/foreign/spngload.c +++ b/libvips/foreign/spngload.c @@ -483,11 +483,16 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) VipsImage **t = (VipsImage **) vips_object_local_array( VIPS_OBJECT( load ), 3 ); + enum spng_decode_flags flags; int error; if( vips_source_decode( png->source ) ) return( -1 ); + /* Decode transparency, if available. + */ + flags = SPNG_DECODE_TRNS; + if( png->ihdr.interlace_method != SPNG_INTERLACE_NONE ) { /* Arg awful interlaced image. We have to load to a huge mem * buffer, then copy to out. @@ -500,7 +505,7 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) if( (error = spng_decode_image( png->ctx, VIPS_IMAGE_ADDR( t[0], 0, 0 ), VIPS_IMAGE_SIZEOF_IMAGE( t[0] ), - png->fmt, 0 )) ) { + png->fmt, flags )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 ); @@ -513,10 +518,12 @@ vips_foreign_load_png_load( VipsForeignLoad *load ) t[0] = vips_image_new(); vips_foreign_load_png_set_header( png, t[0] ); - /* Initialize for progressive decoding. + /* We can decode these progressively. */ + flags |= SPNG_DECODE_PROGRESSIVE; + if( (error = spng_decode_image( png->ctx, NULL, 0, - png->fmt, SPNG_DECODE_PROGRESSIVE )) ) { + png->fmt, flags )) ) { vips_error( class->nickname, "%s", spng_strerror( error ) ); return( -1 );