Feature/hardware buffer r8 - #2
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request introduces support for decoding 8-bit monochrome (YUV400) AVIF images directly into HardwareBuffer objects using a packed Gray565 format on Android API 29+. It includes comprehensive documentation, test assets, and a new test suite. The review feedback highlights a critical bug in the JNI layer where stride offsets are calculated incorrectly on a byte pointer instead of a pixel pointer, which will cause memory corruption. Additionally, adding a defensive null check for the input ByteBuffer in the Java API is recommended to prevent native crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| uint16_t* dst_row = | ||
| reinterpret_cast<uint16_t*>(dst_bytes + row * desc.stride); |
There was a problem hiding this comment.
The desc.stride field in AHardwareBuffer_Desc represents the row stride in pixels, not bytes. Since dst_bytes is a uint8_t* (byte pointer), adding row * desc.stride only advances the pointer by desc.stride bytes per row instead of desc.stride * 2 bytes (for 16-bit RGB_565 pixels). This will cause rows to overlap and corrupt the decoded image.
To fix this, cast dst_bytes to uint16_t* before applying the row offset.
uint16_t* dst_row =
reinterpret_cast<uint16_t*>(dst_bytes) + row * desc.stride;| public static HardwareBuffer decodeToHardwareBuffer( | ||
| ByteBuffer encoded, | ||
| int length, | ||
| int targetWidth, | ||
| int targetHeight, | ||
| int threads, | ||
| boolean allowGray565) { | ||
| return decodeToHardwareBufferNative( | ||
| encoded, length, targetWidth, targetHeight, threads, allowGray565); | ||
| } |
There was a problem hiding this comment.
To prevent potential JVM/ART crashes or undefined behavior when a null ByteBuffer is passed to the JNI layer, add a defensive null check for encoded before calling the native method.
public static HardwareBuffer decodeToHardwareBuffer(
ByteBuffer encoded,
int length,
int targetWidth,
int targetHeight,
int threads,
boolean allowGray565) {
if (encoded == null) {
throw new IllegalArgumentException("encoded ByteBuffer cannot be null");
}
return decodeToHardwareBufferNative(
encoded, length, targetWidth, targetHeight, threads, allowGray565);
}
No description provided.