Repository navigation
Fix the wrong calculation of buffer of AVIF image which cause memory peak - #66
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SDWebImageAVIFCoder/Classes/Conversion.m (1)
176-181: Add overflow guard and reuse rowBytes for consistency.
- Guard against size_t overflow when computing total allocation.
- Reuse the computed rowBytes when initializing resultBuffer.rowBytes to avoid duplication and drift.
Apply this diff:
@@ - size_t const rowBytes = components * sizeof(uint8_t) * avif->width; + size_t const rowBytes = avif->width * components; @@ - resultBufferData = calloc(rowBytes * avif->height, sizeof(uint8_t)); + // Guard against overflow in allocation + if (avif->height != 0 && rowBytes > SIZE_MAX / avif->height) { + err = kvImageMemoryAllocationError; + goto end_all; + } + resultBufferData = calloc(avif->height, rowBytes); @@ - .rowBytes = avif->width * components, + .rowBytes = rowBytes,Add this include once at the top of the file to use SIZE_MAX:
+#include <stdint.h>Also applies to: 189-191, 203-208
🧹 Nitpick comments (2)
SDWebImageAVIFCoder/Classes/Conversion.m (2)
29-31: Avoid recomputing stride inside CreateImageFromBuffer.Rely on the caller-provided vImage stride to reduce mismatch risk if future callers use aligned/packed rows.
Apply this diff:
@@ - size_t bitsPerComponent = usesU16 ? 16 : 8; - size_t bitsPerPixel = components * bitsPerComponent; - size_t rowBytes = result->width * components * (usesU16 ? sizeof(uint16_t) : sizeof(uint8_t)); + size_t bitsPerComponent = usesU16 ? 16 : 8; + size_t bitsPerPixel = components * bitsPerComponent; + size_t rowBytes = result->rowBytes; // trust provided strideAlso applies to: 43-47
171-171: Remove unused variable.scaledAlphaBufferData in the 8-bit path is never written/used. Safe to drop to reduce noise.
Apply this diff:
- uint8_t* scaledAlphaBufferData = NULL; @@ - if (scaledAlphaBufferData) free(scaledAlphaBufferData);Also applies to: 541-541
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SDWebImageAVIFCoder/Classes/Conversion.m(1 hunks)
🔇 Additional comments (1)
SDWebImageAVIFCoder/Classes/Conversion.m (1)
189-189: Correct buffer sizing fix — resolves 4x overallocation.Allocating resultBufferData as rowBytes * height is the correct size for 8-bit paths. This should remove the inflated peak memory observed in #65 for animated frames.
If possible, profile an animated AVIF before/after this change and share the peak RSS/VM numbers to confirm the improvement.
1 participant
This close #65
This close #63
Summary by CodeRabbit
Bug Fixes
Performance