[LDM3D] Fix rgblike_to_depthmap truncating 16-bit depth back to 8 bits#14200
Open
chuenchen309 wants to merge 1 commit into
Open
[LDM3D] Fix rgblike_to_depthmap truncating 16-bit depth back to 8 bits#14200chuenchen309 wants to merge 1 commit into
chuenchen309 wants to merge 1 commit into
Conversation
VaeImageProcessorLDM3D.rgblike_to_depthmap combines two 8-bit channels into a 16-bit depth value (green * 256 + blue, range 0-65535). It correctly casts up to a wider integer type for the multiplication, but then cast the result back to the input's dtype (uint8) via `.to(original_dtype)` / `.astype(original_dtype)`, dropping the entire high byte -- so it returned only `depth % 256` (e.g. 1480 came back as 200). numpy_to_depth feeds this into `Image.fromarray(..., mode="I;16")`, which requires 16-bit-wide data, so with a uint8 buffer `VaeImageProcessorLDM3D.postprocess(image, output_type="pil")` -- the normal way an LDM3D pipeline returns a depth image for a 6-channel output -- crashed with `ValueError: buffer is not large enough`. Keep the 16-bit result instead of truncating: return the wider integer type directly (numpy -> uint16 to match the `mode="I;16"` consumer, torch -> the int32 combination). The function's whole purpose is to produce a 16-bit depth map from 8-bit RGB, so casting back to the 8-bit input dtype was never correct. This regression was introduced by huggingface#12546 (which fixed a real NumPy 2.0 `uint8 * 256` overflow but replaced it with this truncation). The existing tests/others/test_image_processor.py had no VaeImageProcessorLDM3D coverage, which is why it shipped; this adds a small regression test class covering the 16-bit combination (numpy + torch) and the pil postprocess path. Verified against the real code before/after: rgblike_to_depthmap(green=5, blue=200) returned 200 (uint8) before and 1480 (uint16 / int32) after; postprocess(output_type="pil") raised ValueError before and returns a valid "I;16" depth image after. Full tests/others/test_image_processor.py: 12 passed. ruff 0.9.10 (repo-pinned) check + format: clean. Note (separate, not addressed here to keep this focused): the output_type="np" branch of postprocess passes float [0, 1] arrays into rgblike_to_depthmap, where the int cast collapses them to 0; that is an independent caller-side issue in the same function's contract and is better handled in its own change.
Contributor
|
Hi @chuenchen309, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
VaeImageProcessorLDM3D.rgblike_to_depthmapcombines two 8-bit channels into a 16-bit depth value (green * 256 + blue, range 0–65535). It correctly casts up to a wider integer type for the multiplication, but then casts the result back to the input's dtype (uint8) via.to(original_dtype)/.astype(original_dtype), dropping the entire high byte — so it returned onlydepth % 256.numpy_to_depthfeeds this intoImage.fromarray(..., mode="I;16"), which requires 16-bit-wide data, so with a uint8 bufferVaeImageProcessorLDM3D.postprocess(image, output_type="pil")— the normal way an LDM3D pipeline returns a depth image for a 6-channel output — crashes withValueError: buffer is not large enough.The fix keeps the 16-bit result instead of truncating: return the wider integer type directly (numpy →
uint16to match themode="I;16"consumer, torch → theint32combination). The function's whole purpose is to produce a 16-bit depth map from 8-bit RGB, so casting back to the 8-bit input dtype was never correct.This regression was introduced by #12546 (which fixed a real NumPy 2.0
uint8 * 256overflow but replaced it with this truncation).tests/others/test_image_processor.pyhad noVaeImageProcessorLDM3Dcoverage, which is why it shipped — this adds a small regression test class.Before submitting
Ldm3dImageProcessorTest: 16-bit combination for numpy + torch, and the pil postprocess path; red before the fix, green after. Fulltest_image_processor.py: 12 passed.ruff0.9.10 check + format clean.)Who can review?
@yiyixuxu @sayakpaul
Found via an AI-assisted (Claude) review pass over
image_processor.py; independently reproduced, fixed, and verified before submitting.