io_image read_image bugfix & add feature read_image_16 (bit)#681
io_image read_image bugfix & add feature read_image_16 (bit)#681mratsim merged 3 commits intomratsim:masterfrom
Conversation
Fixed a bug in the read_image proc that swapped height and width
Replaced int with apposite stb_image enum & improved proc doc
Added read_image_16 proc for reading 16-bit images from a file or buffer
📝 WalkthroughWalkthroughIntroduces 16-bit image reading capabilities through two new procedures ( Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FileIO as File/Buffer
participant Decoder as Image Decoder
participant Reshape as Reshape Ops
participant Tensor as Tensor Output
rect rgba(100, 150, 200, 0.2)
Note over Caller,Tensor: 16-bit Path (New)
Caller->>FileIO: read_image_16(input)
FileIO->>Decoder: load16FromMemory(buffer)
Decoder->>Reshape: raw_data [width, height, channels]
Reshape->>Reshape: reshape to [height, width, channels]
Reshape->>Reshape: hwc_to_chw() → [channels, height, width]
Reshape->>Tensor: Tensor[uint16]
end
rect rgba(150, 150, 150, 0.15)
Note over Caller,Tensor: 8-bit Path (Updated)
Caller->>FileIO: read_image(input)
FileIO->>Decoder: decode with Default channels
Decoder->>Reshape: raw_data [width, height, channels]
Reshape->>Reshape: reshape to [height, width, channels]
Reshape->>Reshape: hwc_to_chw() → [channels, height, width]
Reshape->>Tensor: Tensor[uint8]
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/arraymancer/io/io_image.nim (1)
51-54: Consistent with filepath version—same verification needed.The changes here mirror those in the filepath version: correct reshape ordering and the
Defaultenum for channel autodetection. The same verification and error-handling considerations apply.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/arraymancer/io/io_image.nim
🔇 Additional comments (1)
src/arraymancer/io/io_image.nim (1)
68-83:load16FromMemoryis correctly integrated—note that it's marked as untested in the nim-stb_image wrapper.The function exists in nim-stb_image and maps to stb_image's
stbi_load_16_from_memory. The implementation mirrors the file-based version and correctly uses the wrapper's conversion ofseq[byte]to pointer + length internally. However, the function is listed as an untested binding in the wrapper, so validate with known 16-bit test images before deployment.
| let desired_channels = Default # Channel autodetection | ||
|
|
||
| let raw_pixels = load(filepath, width, height, channels, desired_channels) | ||
| result = raw_pixels.toTensor.reshape(height, width, channels).hwc_to_chw |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.nim" | head -20Length of output: 13
🏁 Script executed:
#!/bin/bash
# Find Default enum definition in stb_image related files
rg -n --type nim 'Default' -g '*/stb*' | head -20Repository: arraymancer
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Search for load function signature to understand desired_channels parameter
rg -n --type nim 'proc load\*?\(' -g '*/stb*' -A5 | head -40Repository: arraymancer
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Read the actual file being reviewed to see the context
head -90 src/arraymancer/io/io_image.nim | tail -70Repository: arraymancer
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check if there's error handling for nil returns elsewhere in the file
rg -n 'if.*nil\|isNil' src/arraymancer/io/io_image.nim -B2 -A2Repository: arraymancer
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Look for the stb_image read module imports and usage
head -30 src/arraymancer/io/io_image.nimRepository: arraymancer
Length of output: 166
🌐 Web query:
stb_image nim wrapper Default enum channel autodetection
💡 Result:
Short answer: The Nim stb_image wrapper uses the same enum as stb_image: Default corresponds to STBI_default (0) which tells the loader to autodetect the number of channels. Other enum values map to STBI_grey (1), STBI_grey_alpha (2), STBI_rgb (3) and STBI_rgb_alpha (4). Use Default (or 0) for autodetection, or pick a specific enum to force channel count. [1][2][3]
Sources:
- stb_image-Nim README (usage & Default example). [1]
- stb_image.h enum definitions (STBI_default = 0, STBI_grey = 1, …). [2]
- StbImage docs showing channels: 0 = auto-detection. [3]
🌐 Web query:
stb_image library desired_channels parameter documentation
💡 Result:
The desired_channels (sometimes called req_comp) argument to stbi_load / stbi_load_from_memory (and the 16-bit/float variants) tells stb_image how many components per pixel you want in the returned buffer:
- If desired_channels == 0: the loader returns the image with its native number of components and writes that native count into *channels_in_file.
- If desired_channels != 0: stb_image will force the output to have exactly desired_channels components per pixel (padding or truncating channels as needed); *channels_in_file is still filled with the original number of components in the file. Typical values are 1 (gray), 2 (gray+alpha), 3 (RGB), 4 (RGBA). [1][2]
Sources: stb_image.h function comments and prototypes. [1] [2]
References
[1] stb_image.h header (function doc/comments).
[2] API reference / header listings for stbi_load variants.
The Default enum and reshape logic are correct, but add error handling for nil returns.
The change from 0 to Default for channel autodetection is semantically correct—stb_image's Default enum value equals STBI_default (0) and tells the loader to autodetect the native channel count. The reshape with (height, width, channels) is also correct, as stb_image returns pixels in row-major order matching this layout.
However, the load() function can return nil on failure. Add a nil-check before calling .toTensor() to handle cases where image loading fails.
🤖 Prompt for AI Agents
In src/arraymancer/io/io_image.nim around lines 34 to 37, the call to load(...)
can return nil on failure but the code immediately calls .toTensor() which will
crash; add a nil-check after raw_pixels is assigned and before calling
toTensor(), and if nil handle the error by returning a clear failure (e.g.,
raise an IO/ValueError or return nil/error result) that includes the filepath
and loader error context so callers can handle image-load failures gracefully.
| proc read_image_16*(filepath: string): Tensor[uint16] = | ||
| ## Read a 16-bit image file and loads it into a Tensor[uint16] of shape | ||
| ## Channel x Height x Width. Channel is 1 for greyscale, 3 for RGB. | ||
| ## | ||
| ## Supports JPEG, PNG, TGA, BMP, PSD, GIF, HDR, PIC, PNM | ||
| ## See stb_image https://github.com/nothings/stb/blob/master/stb_image.h | ||
| var width, height, channels: int | ||
| let desired_channels = Default # Channel autodetection | ||
|
|
||
| let raw_pixels = load16(filepath, width, height, channels, desired_channels) | ||
| result = raw_pixels.toTensor.reshape(height, width, channels).hwc_to_chw |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for load16 function definition
echo "=== Searching for load16 function ==="
rg -n 'load16' --type nim
echo ""
echo "=== Searching for write functions related to 16-bit ==="
rg -n 'write.*16|proc write' --type nim | grep -i "image\|write"
echo ""
echo "=== Checking io_image.nim file structure ==="
wc -l src/arraymancer/io/io_image.nim
echo ""
echo "=== Reading the entire io_image.nim file to understand context ==="
head -100 src/arraymancer/io/io_image.nimRepository: arraymancer
Length of output: 166
🌐 Web query:
stb_image load16 16-bit image support formats
💡 Result:
stb_image provides a 16‑bit-per‑channel load API (stbi_load_16 / stbi_load_16_from_memory). Supported formats that can yield 16‑bit/channel data are:
- PNG — supports 16‑bit-per‑channel PNGs. [1][2]
- PSD — PSD can be decoded with 8/16‑bit-per‑channel. [1][2]
Notes/limitations: JPEG via stb_image is treated as 8‑bit (no 12‑bit JPEG/arithmetic coding support), HDR is a float (f32) interface, and many formats (TGA, BMP, GIF, PIC, PNM) are primarily 8‑bit in stb_image. Use stbi_is_16_bit / stbi_is_16_bit_from_memory to detect 16‑bit images before calling stbi_load_16. [1][2][3]
Sources: [1] stb_image function list/header, [2] stb bindings/docs summary, [3] stb-related documentation.
🏁 Script executed:
#!/bin/bash
# Get file size first
wc -l src/arraymancer/io/io_image.nim
# Read first 100 lines to see imports and context
head -100 src/arraymancer/io/io_image.nim
# Search for load16 definition
rg -n 'proc load16' --type nim
# Search for write_image functions
rg -n 'proc write_image' --type nimRepository: arraymancer
Length of output: 166
Update documentation to accurately reflect 16-bit format support.
The listed formats in the documentation are misleading. According to stb_image specifications, only PNG and PSD actually support 16-bit reading via stbi_load_16. The other listed formats—JPEG, TGA, BMP, GIF, PIC, PNM—are 8-bit only in stb_image, and HDR is float (f32), not uint16. Update the docstring to reflect only the formats that truly support 16-bit depth.
Also verify that load16() exists in the stb_image/read module and consider whether the absence of corresponding write_image_16 functions is intentional.
🤖 Prompt for AI Agents
In src/arraymancer/io/io_image.nim around lines 56 to 66, the docstring wrongly
claims many formats support 16-bit images; update it to state that only PNG and
PSD are supported for 16-bit reads via stbi_load_16 (HDR is float32 and the
others are 8-bit only). Change the listed supported formats accordingly, confirm
that load16() is present in the stb_image/read binding and adjust the comment to
reference stbi_load_16, and add a note or TODO about the lack of corresponding
write_image_16 functions if that is intentional so callers are aware.
|
Thank you! |
2 participants
read_imagefrom buffer proc, which swapped height and widthread_image_16proc to be able to read 16-bit images (useful in order to work with medical images)Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.