From 3fde8cd04a9d7e4b75f7c860c2f466a688ff0906 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Thu, 5 Mar 2026 20:34:28 +0100 Subject: [PATCH 1/3] Unify and fix jpeg resource cleanup --- lib/JpegToBmpConverter/JpegToBmpConverter.cpp | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp index 4e674fbd..1ded3d14 100644 --- a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp +++ b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp @@ -278,8 +278,26 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm bytesPerRow = (outWidth * 2 + 31) / 32 * 4; } + uint8_t* rowBuffer = nullptr; + uint8_t* mcuRowBuffer = nullptr; + AtkinsonDitherer* atkinsonDitherer = nullptr; + FloydSteinbergDitherer* fsDitherer = nullptr; + Atkinson1BitDitherer* atkinson1BitDitherer = nullptr; + uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) + uint16_t* rowCount = nullptr; // Count of source pixels accumulated per output X + + auto cleanupResources = [&]() { + delete[] rowAccum; + delete[] rowCount; + delete atkinsonDitherer; + delete fsDitherer; + delete atkinson1BitDitherer; + free(mcuRowBuffer); + free(rowBuffer); + }; + // Allocate row buffer - auto* rowBuffer = static_cast(malloc(bytesPerRow)); + rowBuffer = static_cast(malloc(bytesPerRow)); if (!rowBuffer) { LOG_ERR("JPG", "Failed to allocate row buffer"); return false; @@ -293,23 +311,19 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm // Validate MCU row buffer size before allocation if (mcuRowPixels > MAX_MCU_ROW_BYTES) { LOG_DBG("JPG", "MCU row buffer too large (%d bytes), max: %d", mcuRowPixels, MAX_MCU_ROW_BYTES); - free(rowBuffer); + cleanupResources(); return false; } - auto* mcuRowBuffer = static_cast(malloc(mcuRowPixels)); + mcuRowBuffer = static_cast(malloc(mcuRowPixels)); if (!mcuRowBuffer) { LOG_ERR("JPG", "Failed to allocate MCU row buffer (%d bytes)", mcuRowPixels); - free(rowBuffer); + cleanupResources(); return false; } // Create ditherer if enabled // Use OUTPUT dimensions for dithering (after prescaling) - AtkinsonDitherer* atkinsonDitherer = nullptr; - FloydSteinbergDitherer* fsDitherer = nullptr; - Atkinson1BitDitherer* atkinson1BitDitherer = nullptr; - if (oneBit) { // For 1-bit output, use Atkinson dithering for better quality atkinson1BitDitherer = new Atkinson1BitDitherer(outWidth); @@ -324,8 +338,6 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm // For scaling: accumulate source rows into scaled output rows // We need to track which source Y maps to which output Y // Using fixed-point: srcY_fp = outY * scaleY_fp (gives source Y in 16.16 format) - uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) - uint16_t* rowCount = nullptr; // Count of source pixels accumulated per output X int currentOutY = 0; // Current output row being accumulated uint32_t nextOutY_srcStart = 0; // Source Y where next output row starts (16.16 fixed point) @@ -351,8 +363,7 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } else { LOG_ERR("JPG", "JPEG decode MCU failed at (%d, %d) with error code: %d", mcuX, mcuY, mcuStatus); } - free(mcuRowBuffer); - free(rowBuffer); + cleanupResources(); return false; } @@ -534,24 +545,7 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } } - // Clean up - if (rowAccum) { - delete[] rowAccum; - } - if (rowCount) { - delete[] rowCount; - } - if (atkinsonDitherer) { - delete atkinsonDitherer; - } - if (fsDitherer) { - delete fsDitherer; - } - if (atkinson1BitDitherer) { - delete atkinson1BitDitherer; - } - free(mcuRowBuffer); - free(rowBuffer); + cleanupResources(); LOG_DBG("JPG", "Successfully converted JPEG to BMP"); return true; From 36a91a53a3b0a8682b8b74ebae1cb3c994285898 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Thu, 5 Mar 2026 20:51:46 +0100 Subject: [PATCH 2/3] Change type --- lib/JpegToBmpConverter/JpegToBmpConverter.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp index 1ded3d14..f896d518 100644 --- a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp +++ b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp @@ -284,7 +284,7 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm FloydSteinbergDitherer* fsDitherer = nullptr; Atkinson1BitDitherer* atkinson1BitDitherer = nullptr; uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) - uint16_t* rowCount = nullptr; // Count of source pixels accumulated per output X + uint32_t* rowCount = nullptr; // Count of source pixels accumulated per output X auto cleanupResources = [&]() { delete[] rowAccum; @@ -343,7 +343,7 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm if (needsScaling) { rowAccum = new uint32_t[outWidth](); - rowCount = new uint16_t[outWidth](); + rowCount = new uint32_t[outWidth](); nextOutY_srcStart = scaleY_fp; // First boundary is at scaleY_fp (source Y for outY=1) } @@ -539,7 +539,7 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } // Moving to next source row - reset accumulators memset(rowAccum, 0, outWidth * sizeof(uint32_t)); - memset(rowCount, 0, outWidth * sizeof(uint16_t)); + memset(rowCount, 0, outWidth * sizeof(uint32_t)); } } } From ae19e975fa51a8dd637fc26216c8357d37b44264 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 7 Mar 2026 10:11:54 +0100 Subject: [PATCH 3/3] Implement znelson recommendation --- lib/JpegToBmpConverter/JpegToBmpConverter.cpp | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp index f896d518..bdc368ab 100644 --- a/lib/JpegToBmpConverter/JpegToBmpConverter.cpp +++ b/lib/JpegToBmpConverter/JpegToBmpConverter.cpp @@ -286,15 +286,26 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm uint32_t* rowAccum = nullptr; // Accumulator for each output X (32-bit for larger sums) uint32_t* rowCount = nullptr; // Count of source pixels accumulated per output X - auto cleanupResources = [&]() { - delete[] rowAccum; - delete[] rowCount; - delete atkinsonDitherer; - delete fsDitherer; - delete atkinson1BitDitherer; - free(mcuRowBuffer); - free(rowBuffer); - }; + // RAII guard: frees all heap resources on any return path, including early exits. + // Holds references so it always sees the latest pointer values assigned below. + struct Cleanup { + uint8_t*& rowBuffer; + uint8_t*& mcuRowBuffer; + AtkinsonDitherer*& atkinsonDitherer; + FloydSteinbergDitherer*& fsDitherer; + Atkinson1BitDitherer*& atkinson1BitDitherer; + uint32_t*& rowAccum; + uint32_t*& rowCount; + ~Cleanup() { + delete[] rowAccum; + delete[] rowCount; + delete atkinsonDitherer; + delete fsDitherer; + delete atkinson1BitDitherer; + free(mcuRowBuffer); + free(rowBuffer); + } + } cleanup{rowBuffer, mcuRowBuffer, atkinsonDitherer, fsDitherer, atkinson1BitDitherer, rowAccum, rowCount}; // Allocate row buffer rowBuffer = static_cast(malloc(bytesPerRow)); @@ -311,14 +322,12 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm // Validate MCU row buffer size before allocation if (mcuRowPixels > MAX_MCU_ROW_BYTES) { LOG_DBG("JPG", "MCU row buffer too large (%d bytes), max: %d", mcuRowPixels, MAX_MCU_ROW_BYTES); - cleanupResources(); return false; } mcuRowBuffer = static_cast(malloc(mcuRowPixels)); if (!mcuRowBuffer) { LOG_ERR("JPG", "Failed to allocate MCU row buffer (%d bytes)", mcuRowPixels); - cleanupResources(); return false; } @@ -363,7 +372,6 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } else { LOG_ERR("JPG", "JPEG decode MCU failed at (%d, %d) with error code: %d", mcuX, mcuY, mcuStatus); } - cleanupResources(); return false; } @@ -545,8 +553,6 @@ bool JpegToBmpConverter::jpegFileToBmpStreamInternal(FsFile& jpegFile, Print& bm } } - cleanupResources(); - LOG_DBG("JPG", "Successfully converted JPEG to BMP"); return true; }