Issues when reading from memory

[jtorresfabra]

Hey @richgel999 ,

Just wanted to try your new jpeg decoder, subsituting the calls I have to stbi with jpegd ones. I'm getting errors. UBSAN is warning me about left shift on negative values:

After that later in my pipe I get a crash when trying to destroy the associated image memory, and of course I never get to see the image. For more data these are my calls in the code:

 // uint8_t* raw = stbi_load_from_memory(imgBuffer, imgBufferSize, &tileData.width, &tileData.height, &channels, 0);
    uint8_t* raw = jpgd::decompress_jpeg_image_from_memory(imgBuffer, imgBufferSize, &tileData.width, &tileData.height, &channels,3);

What worked for stbi crash for jpegd. Any clue? Thanks!

[richgel999]

I'll fix the left shift issue tonight. Not sure about the crash - I'll try to reproduce. Thanks a bunch!

[richgel999]

The left shifts should be totally harmless: const int dcval = (pSrc[0] << PASS1_BITS);

Are you using free() to free the memory returned by decompress_jpeg_image_from_memory(), etc.?

[richgel999]

I've fixed the left shift issues, will check it in shortly. I've ran the decompressor through valgrind (both before release and just now), and it reports no issues. I'm not experiencing any crashes on any platform, so I need more reproduction information:

richg@richgX-PC:~/jpeg-compressor/jpeg-compressor/bin$ valgrind ./jpge -d /mnt/j/dev/test_images/hot15_gray.jpg 1.tga ==18683== Memcheck, a memory error detector ==18683== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==18683== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info ==18683== Command: ./jpge -d /mnt/j/dev/test_images/hot15_gray.jpg 1.tga ==18683== ==18683== error calling PR_SET_PTRACER, vgdb might block jpge/jpgd example app Source JPEG file: "/mnt/j/dev/test_images/hot15_gray.jpg", image resolution: 480x800, actual comps: 1 Decompression time: 268.556ms Wrote decompressed image to TGA file "1.tga" Success. ==18683== ==18683== HEAP SUMMARY: ==18683== in use at exit: 0 bytes in 0 blocks ==18683== total heap usage: 8 allocs, 8 frees, 1,263,472 bytes allocated ==18683== ==18683== All heap blocks were freed -- no leaks are possible ==18683== ==18683== For counts of detected and suppressed errors, rerun with: -v ==18683== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[richgel999]

Just checked in a fix for the left shifts in jpge. Ran valgrind on the encoder - no problems detected:

richg@richgX-PC:~/jpeg-compressor/jpeg-compressor/bin$ valgrind ./jpge 1.tga 1.jpg 100 ==20114== Memcheck, a memory error detector ==20114== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==20114== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info ==20114== Command: ./jpge 1.tga 1.jpg 100 ==20114== ==20114== error calling PR_SET_PTRACER, vgdb might block jpge/jpgd example app Source file: "1.tga", image resolution: 480x800, actual comps: 3 Writing JPEG image to file: 1.jpg Compressed file size: 107198, bits/pixel: 2.233 Compression time: 359.889ms, Decompression time: 397.702ms Error Max: 4.000000, Mean: 0.063854, Mean^2: 0.079937, RMSE: 0.282732, PSNR: 59.103298 Success. ==20114== ==20114== HEAP SUMMARY: ==20114== in use at exit: 2,304,000 bytes in 2 blocks ==20114== total heap usage: 16 allocs, 14 frees, 2,488,168 bytes allocated ==20114== ==20114== LEAK SUMMARY: ==20114== definitely lost: 2,304,000 bytes in 2 blocks ==20114== indirectly lost: 0 bytes in 0 blocks ==20114== possibly lost: 0 bytes in 0 blocks ==20114== still reachable: 0 bytes in 0 blocks ==20114== suppressed: 0 bytes in 0 blocks ==20114== Rerun with --leak-check=full to see details of leaked memory ==20114== ==20114== For counts of detected and suppressed errors, rerun with: -v ==20114== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[richgel999]

All writing to the output buffer holding the destination image is completely confined to the small decompress_jpeg_image_from_stream() function. I reviewed it again to be sure, and it looks OK.

This particular function is pretty old code, and it's been used in a bunch of projects over the years. So I definitely need more reproduction information to help you. Thanks!

[jtorresfabra]

Good news!

Your fixes for undefined behaviour fixed my crashes (both linux native and webassmebly). Or at least your last commits made it.

That was fast :). Thanks a lot! Gonna do some benching now!

[richgel999]

Okay, that's a relief. We're slower than stb_image, especially on H2V2 chroma subsampling. We need SSE2 chroma upsample and colorspace conversion. I'll eventually put this in, but for now I'm going to focus on some other fires.

However, we're fuzzed & stb_image's reader isn't yet, so it's a tradeoff.