Fix the wrong calculation of buffer of AVIF image which cause memory peak

[dreampiggy]

This close #65
This close #63

Summary by CodeRabbit

• Bug Fixes

  • Corrected buffer sizing in AVIF image processing to match actual per-row data, preventing over-allocation that could cause elevated memory usage or occasional crashes with certain images.

• Performance

  • Reduced memory footprint during AVIF decoding, improving efficiency and stability, especially on memory-constrained devices.
  • More consistent memory behavior when handling large or high-resolution AVIF images.

[coderabbitai]

Walkthrough

Adjusted memory allocation in SDWebImageAVIFCoder’s CreateCGImage8 by sizing the result buffer to rowBytes multiplied by image height instead of components times rowBytes times height. No other logic or control flow was changed.

Changes

Cohort / File(s)	Summary
AVIF conversion buffer sizing SDWebImageAVIFCoder/Classes/Conversion.m	Updated allocation of resultBufferData in CreateCGImage8 from components × rowBytes × height to rowBytes × height to align with vImage_Buffer rowBytes usage and avoid over-allocation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled bytes with careful cheer,
Trimmed the buffer, kept it near—
Row by row, the lines align,
No extra hay in coder’s twine.
AVIF hops lighter now, you see—
A tidier warren of memory. 🐇💾

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and concisely describes the primary fix — correcting a buffer size calculation that caused a memory peak — and directly reflects the change made in Conversion.m.
Linked Issues Check	✅ Passed	The change removes an erroneous multiplication by "components" when allocating resultBufferData (changing components * rowBytes * height to rowBytes * height), which eliminates the ~components factor of over-allocation and therefore directly addresses the memory-peak behavior reported in issue #65.
Out of Scope Changes Check	✅ Passed	The diff only adjusts the buffer allocation line in SDWebImageAVIFCoder/Classes/Conversion.m and contains no other logic, API, or unrelated file changes, so there are no detectable out-of-scope modifications.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch bugfix/calculation_buffer_size

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 stride

Also 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

📥 Commits

Reviewing files that changed from the base of the PR and between f784fcf and da93d8f.

📒 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.