From 56a1f39d984933e3024fe2bb6843debeb9d52ca1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 11 Mar 2024 17:34:43 +0000 Subject: [PATCH 1/2] JPEG: Factor out the middle of IMG_SaveJPG_RW_jpeglib No functional change intended. We'll need to use setjmp() in this function in a subsequent commit, so ensure that its state doesn't include any local variables that are used both "above" and "below" the stack level at which we will call setjmp(). Signed-off-by: Simon McVittie --- src/IMG_jpg.c | 72 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/src/IMG_jpg.c b/src/IMG_jpg.c index 004e9517..bb772189 100644 --- a/src/IMG_jpg.c +++ b/src/IMG_jpg.c @@ -478,15 +478,52 @@ static void jpeg_SDL_RW_dest(j_compress_ptr cinfo, SDL_RWops *ctx) dest->pub.free_in_buffer = OUTPUT_BUFFER_SIZE; } +struct savejpeg_vars +{ + struct jpeg_compress_struct cinfo; + struct my_error_mgr jerr; +}; + +static int JPEG_SaveJPEG_RW(struct savejpeg_vars *vars, SDL_Surface *jpeg_surface, SDL_RWops *dst, int quality) +{ + /* Create a compression structure and load the JPEG header */ + vars->cinfo.err = lib.jpeg_std_error(&vars->jerr.errmgr); + vars->jerr.errmgr.error_exit = my_error_exit; + vars->jerr.errmgr.output_message = output_no_message; + + lib.jpeg_create_compress(&vars->cinfo); + jpeg_SDL_RW_dest(&vars->cinfo, dst); + + vars->cinfo.image_width = jpeg_surface->w; + vars->cinfo.image_height = jpeg_surface->h; + vars->cinfo.in_color_space = JCS_RGB; + vars->cinfo.input_components = 3; + + lib.jpeg_set_defaults(&vars->cinfo); + lib.jpeg_set_quality(&vars->cinfo, quality, TRUE); + lib.jpeg_start_compress(&vars->cinfo, TRUE); + + while (vars->cinfo.next_scanline < vars->cinfo.image_height) { + JSAMPROW row_pointer[1]; + int offset = vars->cinfo.next_scanline * jpeg_surface->pitch; + + row_pointer[0] = ((Uint8*)jpeg_surface->pixels) + offset; + lib.jpeg_write_scanlines(&vars->cinfo, row_pointer, 1); + } + + lib.jpeg_finish_compress(&vars->cinfo); + lib.jpeg_destroy_compress(&vars->cinfo); + return 0; +} + static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int quality) { /* The JPEG library reads bytes in R,G,B order, so this is the right * encoding for either endianness */ + struct savejpeg_vars vars; static const Uint32 jpg_format = SDL_PIXELFORMAT_RGB24; - struct jpeg_compress_struct cinfo; - struct my_error_mgr jerr; - JSAMPROW row_pointer[1]; SDL_Surface* jpeg_surface = surface; + int ret; if (!IMG_Init(IMG_INIT_JPG)) { return -1; @@ -500,36 +537,13 @@ static int IMG_SaveJPG_RW_jpeglib(SDL_Surface *surface, SDL_RWops *dst, int qual } } - /* Create a decompression structure and load the JPEG header */ - cinfo.err = lib.jpeg_std_error(&jerr.errmgr); - jerr.errmgr.error_exit = my_error_exit; - jerr.errmgr.output_message = output_no_message; - - lib.jpeg_create_compress(&cinfo); - jpeg_SDL_RW_dest(&cinfo, dst); - - cinfo.image_width = jpeg_surface->w; - cinfo.image_height = jpeg_surface->h; - cinfo.in_color_space = JCS_RGB; - cinfo.input_components = 3; - - lib.jpeg_set_defaults(&cinfo); - lib.jpeg_set_quality(&cinfo, quality, TRUE); - lib.jpeg_start_compress(&cinfo, TRUE); - - while (cinfo.next_scanline < cinfo.image_height) { - int offset = cinfo.next_scanline * jpeg_surface->pitch; - row_pointer[0] = ((Uint8*)jpeg_surface->pixels) + offset; - lib.jpeg_write_scanlines(&cinfo, row_pointer, 1); - } - - lib.jpeg_finish_compress(&cinfo); - lib.jpeg_destroy_compress(&cinfo); + SDL_zero(vars); + ret = JPEG_SaveJPEG_RW(&vars, jpeg_surface, dst, quality); if (jpeg_surface != surface) { SDL_DestroySurface(jpeg_surface); } - return 0; + return ret; } #elif defined(USE_STBIMAGE) From ac5f692328292b6133c75b5f3aa210123e147af8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 11 Mar 2024 19:25:27 +0000 Subject: [PATCH 2/2] JPEG: Add error-recovery when saving with libjpeg Because we have set up libjpeg to use my_error_exit, we need to call setjmp() before the first time it might possibly call longjmp(). Otherwise, on error we'll do a non-local goto to an uninitialized pointer and crash. Resolves: https://github.com/libsdl-org/SDL_image/issues/429 Signed-off-by: Simon McVittie --- src/IMG_jpg.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/IMG_jpg.c b/src/IMG_jpg.c index bb772189..6e78a1d5 100644 --- a/src/IMG_jpg.c +++ b/src/IMG_jpg.c @@ -482,6 +482,7 @@ struct savejpeg_vars { struct jpeg_compress_struct cinfo; struct my_error_mgr jerr; + Sint64 original_offset; }; static int JPEG_SaveJPEG_RW(struct savejpeg_vars *vars, SDL_Surface *jpeg_surface, SDL_RWops *dst, int quality) @@ -490,6 +491,14 @@ static int JPEG_SaveJPEG_RW(struct savejpeg_vars *vars, SDL_Surface *jpeg_surfac vars->cinfo.err = lib.jpeg_std_error(&vars->jerr.errmgr); vars->jerr.errmgr.error_exit = my_error_exit; vars->jerr.errmgr.output_message = output_no_message; + vars->original_offset = SDL_RWtell(dst); + + if(setjmp(vars->jerr.escape)) { + /* If we get here, libjpeg found an error */ + lib.jpeg_destroy_compress(&vars->cinfo); + SDL_RWseek(dst, vars->original_offset, SDL_RW_SEEK_SET); + return IMG_SetError("Error saving JPEG with libjpeg"); + } lib.jpeg_create_compress(&vars->cinfo); jpeg_SDL_RW_dest(&vars->cinfo, dst);