From 34427d83a028690b098fd1f67e956f78bc724e22 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Sun, 14 Aug 2022 16:53:33 +0200 Subject: [PATCH 1/4] jxlsave: correctly mark frame as last (#2988) It's required to close the input, otherwise the encoder can't know what the last frame is, resulting in an improper codestream. Resolves: #2987. --- libvips/foreign/jxlsave.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libvips/foreign/jxlsave.c b/libvips/foreign/jxlsave.c index 94ed4fa2..cd2c51b7 100644 --- a/libvips/foreign/jxlsave.c +++ b/libvips/foreign/jxlsave.c @@ -438,6 +438,11 @@ vips_foreign_save_jxl_build( VipsObject *object ) return( -1 ); } + /* This function must be called after the final frame and/or box, + * otherwise the codestream will not be encoded correctly. + */ + JxlEncoderCloseInput( jxl->encoder ); + do { uint8_t *out; size_t avail_out; From f36196db1b6a4a9ca0958b0f7e10d21ca5a5ccb1 Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Sun, 14 Aug 2022 16:54:21 +0200 Subject: [PATCH 2/4] jxlsave: avoid using deprecated functions (#2989) * jxlsave: avoid using deprecated functions The upcoming libjxl 0.7 has deprecated a number of functions. * Prefer to use `ifdef` instead --- configure.ac | 7 +++++++ libvips/foreign/jxlsave.c | 30 ++++++++++++++++++++++-------- meson.build | 3 +++ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index 04a0da51..3573a76e 100644 --- a/configure.ac +++ b/configure.ac @@ -847,6 +847,13 @@ if test x"$with_libjxl" != x"no"; then with_libjxl=yes AS_IF([test x"$with_libjxl_module" = x"no"], [PACKAGES_USED="$PACKAGES_USED libjxl"]) + + PKG_CHECK_MODULES(LIBJXL_0_7, libjxl >= 0.7, + [AC_DEFINE(HAVE_LIBJXL_0_7,1,[define if you have libjxl >= 0.7]) + ], + [: + ] + ) ], [AS_IF([test x"$with_libjxl" = x"test"], AC_MSG_WARN([libjxl not found; disabling libjxl support]), diff --git a/libvips/foreign/jxlsave.c b/libvips/foreign/jxlsave.c index cd2c51b7..9c49b95d 100644 --- a/libvips/foreign/jxlsave.c +++ b/libvips/foreign/jxlsave.c @@ -229,7 +229,11 @@ vips_foreign_save_jxl_build( VipsObject *object ) VipsForeignSaveJxl *jxl = (VipsForeignSaveJxl *) object; VipsImage **t = (VipsImage **) vips_object_local_array( object, 5 ); - JxlEncoderOptions *options; +#ifdef HAVE_LIBJXL_0_7 + JxlEncoderFrameSettings *frame_settings; +#else + JxlEncoderOptions *frame_settings; +#endif JxlEncoderStatus status; VipsImage *in; VipsBandFormat format; @@ -415,23 +419,33 @@ vips_foreign_save_jxl_build( VipsObject *object ) if( vips_image_wio_input( in ) ) return( -1 ); - options = JxlEncoderOptionsCreate( jxl->encoder, NULL ); - JxlEncoderOptionsSetDecodingSpeed( options, jxl->tier ); - JxlEncoderOptionsSetDistance( options, jxl->distance ); - JxlEncoderOptionsSetEffort( options, jxl->effort ); - JxlEncoderOptionsSetLossless( options, jxl->lossless ); +#ifdef HAVE_LIBJXL_0_7 + frame_settings = JxlEncoderFrameSettingsCreate( jxl->encoder, NULL ); + JxlEncoderFrameSettingsSetOption( frame_settings, + JXL_ENC_FRAME_SETTING_DECODING_SPEED, jxl->tier ); + JxlEncoderSetFrameDistance( frame_settings, jxl->distance ); + JxlEncoderFrameSettingsSetOption( frame_settings, + JXL_ENC_FRAME_SETTING_EFFORT, jxl->effort ); + JxlEncoderSetFrameLossless( frame_settings, jxl->lossless ); +#else + frame_settings = JxlEncoderOptionsCreate( jxl->encoder, NULL ); + JxlEncoderOptionsSetDecodingSpeed( frame_settings, jxl->tier ); + JxlEncoderOptionsSetDistance( frame_settings, jxl->distance ); + JxlEncoderOptionsSetEffort( frame_settings, jxl->effort ); + JxlEncoderOptionsSetLossless( frame_settings, jxl->lossless ); +#endif #ifdef DEBUG vips_foreign_save_jxl_print_info( &jxl->info ); vips_foreign_save_jxl_print_format( &jxl->format ); - printf( "JxlEncoderOptions:\n" ); + printf( "JxlEncoderFrameSettings:\n" ); printf( " tier = %d\n", jxl->tier ); printf( " distance = %g\n", jxl->distance ); printf( " effort = %d\n", jxl->effort ); printf( " lossless = %d\n", jxl->lossless ); #endif /*DEBUG*/ - if( JxlEncoderAddImageFrame( options, &jxl->format, + if( JxlEncoderAddImageFrame( frame_settings, &jxl->format, VIPS_IMAGE_ADDR( in, 0, 0 ), VIPS_IMAGE_SIZEOF_IMAGE( in ) ) ) { vips_foreign_save_jxl_error( jxl, "JxlEncoderAddImageFrame" ); diff --git a/meson.build b/meson.build index d5fd4dfe..1040b6a8 100644 --- a/meson.build +++ b/meson.build @@ -484,6 +484,9 @@ if libjxl_found if cc.has_function('JxlEncoderInitBasicInfo', prefix: '#include ', dependencies: libjxl_dep) cfg_var.set('HAVE_LIBJXL_JXLENCODERINITBASICINFO', '1') endif + if libjxl_dep.version().version_compare('>=0.7') + cfg_var.set('HAVE_LIBJXL_0_7', '1') + endif endif libpoppler_dep = dependency('poppler-glib', version: '>=0.16.0', required: get_option('poppler')) From 99e6bb833e9fc1bb380efef8a4ba8e442bc7ef92 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 14 Aug 2022 16:29:00 +0100 Subject: [PATCH 3/4] ooops --- libvips/foreign/spngsave.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvips/foreign/spngsave.c b/libvips/foreign/spngsave.c index d408fe29..7ff47739 100644 --- a/libvips/foreign/spngsave.c +++ b/libvips/foreign/spngsave.c @@ -42,9 +42,9 @@ */ /* - */ #define DEBUG_VERBOSE #define DEBUG + */ #ifdef HAVE_CONFIG_H #include From e88db2ceb423f0307b201ae0f8869bfb399c1bf9 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 14 Aug 2022 17:27:13 +0100 Subject: [PATCH 4/4] fix jxl load rewind Looks like there's no way to reset the decoder state, so we have to remake it. --- ChangeLog | 1 + libvips/foreign/jxlload.c | 65 ++++++++++++++++++++++++--------------- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 87add23f..4687189e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,7 @@ - better 0 detection in unpremultiply - fix low bitdepth spng save [jeffska] - fix PNG low bitdepth save of high bitdepth images +- add support for libjxl 0.7 [kleisauke] 21/11/21 started 8.13 - configure fails for requested but unmet dependencies [remicollet] diff --git a/libvips/foreign/jxlload.c b/libvips/foreign/jxlload.c index 521d6de2..343c2402 100644 --- a/libvips/foreign/jxlload.c +++ b/libvips/foreign/jxlload.c @@ -158,21 +158,6 @@ vips_foreign_load_jxl_build( VipsObject *object ) jxl->runner = JxlThreadParallelRunnerCreate( NULL, vips_concurrency_get() ); - jxl->decoder = JxlDecoderCreate( NULL ); - - if( JxlDecoderSubscribeEvents( jxl->decoder, - JXL_DEC_COLOR_ENCODING | - JXL_DEC_BASIC_INFO | - JXL_DEC_FULL_IMAGE ) ) { - vips_foreign_load_jxl_error( jxl, "JxlDecoderSubscribeEvents" ); - return( -1 ); - } - if( JxlDecoderSetParallelRunner( jxl->decoder, - JxlThreadParallelRunner, jxl->runner ) ) { - vips_foreign_load_jxl_error( jxl, - "JxlDecoderSetParallelRunner" ); - return( -1 ); - } if( VIPS_OBJECT_CLASS( vips_foreign_load_jxl_parent_class )-> build( object ) ) @@ -378,10 +363,13 @@ vips_foreign_load_jxl_process( VipsForeignLoadJxl *jxl ) JXL_DEC_NEED_MORE_INPUT ) { size_t bytes_remaining; +#ifdef DEBUG + printf( "vips_foreign_load_jxl_process: reading ...\n" ); +#endif /*DEBUG*/ + bytes_remaining = JxlDecoderReleaseInput( jxl->decoder ); if( vips_foreign_load_jxl_fill_input( jxl, bytes_remaining ) ) return( JXL_DEC_ERROR ); - JxlDecoderSetInput( jxl->decoder, jxl->input_buffer, jxl->bytes_in_buffer ); } @@ -505,10 +493,24 @@ vips_foreign_load_jxl_header( VipsForeignLoad *load ) printf( "vips_foreign_load_jxl_header:\n" ); #endif /*DEBUG*/ - if( vips_source_rewind( jxl->source ) ) - return( -1 ); + /* Build the decoder we will use for the header. + */ + jxl->decoder = JxlDecoderCreate( NULL ); + if( JxlDecoderSubscribeEvents( jxl->decoder, + JXL_DEC_COLOR_ENCODING | + JXL_DEC_BASIC_INFO ) ) { + vips_foreign_load_jxl_error( jxl, "JxlDecoderSubscribeEvents" ); + return( -1 ); + } + if( JxlDecoderSetParallelRunner( jxl->decoder, + JxlThreadParallelRunner, jxl->runner ) ) { + vips_foreign_load_jxl_error( jxl, + "JxlDecoderSetParallelRunner" ); + return( -1 ); + } - if( vips_foreign_load_jxl_fill_input( jxl, 0 ) ) + if( vips_source_rewind( jxl->source ) || + vips_foreign_load_jxl_fill_input( jxl, 0 ) ) return( -1 ); JxlDecoderSetInput( jxl->decoder, jxl->input_buffer, jxl->bytes_in_buffer ); @@ -618,13 +620,26 @@ vips_foreign_load_jxl_load( VipsForeignLoad *load ) if( vips_foreign_load_jxl_set_header( jxl, t[0] ) ) return( -1 ); - /* We have to reset the reader ... we can't reply onb the read point - * being left just after the header. - */ - if( vips_source_rewind( jxl->source ) ) - return( -1 ); + /* We have to make a new decoder ... we can't be certain the header + * decoder left the input in the correct place. + */ + VIPS_FREEF( JxlDecoderDestroy, jxl->decoder ); - if( vips_foreign_load_jxl_fill_input( jxl, 0 ) ) + jxl->decoder = JxlDecoderCreate( NULL ); + if( JxlDecoderSubscribeEvents( jxl->decoder, + JXL_DEC_FULL_IMAGE ) ) { + vips_foreign_load_jxl_error( jxl, "JxlDecoderSubscribeEvents" ); + return( -1 ); + } + if( JxlDecoderSetParallelRunner( jxl->decoder, + JxlThreadParallelRunner, jxl->runner ) ) { + vips_foreign_load_jxl_error( jxl, + "JxlDecoderSetParallelRunner" ); + return( -1 ); + } + + if( vips_source_rewind( jxl->source ) || + vips_foreign_load_jxl_fill_input( jxl, 0 ) ) return( -1 ); JxlDecoderSetInput( jxl->decoder, jxl->input_buffer, jxl->bytes_in_buffer );