libsdl-org / SDL_image

Image decoding for many popular formats for Simple Directmedia Layer.
zlib License
513 stars 174 forks source link

JPEG: Handle errors when saving with libjpeg #431

Closed smcv closed 4 months ago

smcv commented 4 months ago

SDL3 version of #432. Untested (there's no SDL3 in the scout SDK yet) but the only difference is the name of a constant.

smcv commented 4 months ago

The Windows CI build says:

D:/a/SDL_image/SDL_image/src/IMG_jpg.c:489:18: error: variable 'jpeg_surface' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
sezero commented 4 months ago

Maybe we should separate longjmp handling and the rest like we did for libpng code in https://github.com/libsdl-org/SDL_image/commit/3e42a4fb0138f1f2522c43a9e1cda41d24c2a21c (followed by minor simplification https://github.com/libsdl-org/SDL_image/commit/135bec7c30a00703b3c1a6aab04743ab522262fe) (also applied to SDL2 branch)

smcv commented 4 months ago

Maybe we should separate longjmp handling and the rest like we did for libpng code in 3e42a4f (followed by minor simplification 135bec7) (also applied to SDL2 branch)

Yes, I think that's going to be necessary. I'm trying to work out how to do that in the most reviewable way.

smcv commented 4 months ago

OK, how's this?

sezero commented 4 months ago

Looks good, I think. We should do the same to the load function too.

smcv commented 4 months ago

We should do the same to the load function too.

Ideally yes, but I'd prefer that sort of refactoring not to be a blocker for a crash fix.

sezero commented 4 months ago

@slouken ?

slouken commented 4 months ago

Looks good!

sezero commented 4 months ago

We should adapt to SDL2 too and do the same for loader function as well, later.

smcv commented 4 months ago

We should adapt to SDL2 too

That was #432 (which is actually the version I wrote first)

and do the same for loader function as well, later

435