Repository navigation
Sync upstream master (fd21ecf): server cancel, Z-Image reference crash fix, ggml Vulkan small-BAR placement, one placeholder per image - #35
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3b773a306
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (key != "inpaint_width" && key != "inpaint_height" && key != "steps") { | ||
| continue; |
There was a problem hiding this comment.
When sd-server is started with --ad-model, native and OpenAI requests can supply extra_ad_args, but this validation only bounds inpaint dimensions and steps. A request such as extra_ad_args: "input_size=1000000" therefore reaches make_letterbox_input, which attempts to allocate and process a 1,000,000×1,000,000×3 float tensor, allowing a single request to exhaust memory or terminate the server. Bound input_size and the other resource-sensitive native ADetailer parameters before generation, as is already done for the SDAPI translation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This code comes from upstream (leejet/stable-diffusion.cpp master-995-fd21ecf); this PR only merges it and does not touch this file. Leaving it to upstream.
| for (int h = min_size; h < 2LL * area / w; h += factor) { | ||
| int64_t pixels = int64_t(w) * h; |
There was a problem hiding this comment.
With vae_size_buckets=true, vae_input_max_pixels is accepted as any positive int and is used directly as area here. A server request can set it near INT_MAX, causing this nested search to examine millions of widths and hundreds of millions of width/height pairs before inference while the generation mutex is held; Fibo enables bucket sizing by default. Cap the requested area to a practical image limit or compute the candidate dimensions without iterating up to a user-controlled area.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This code comes from upstream (leejet/stable-diffusion.cpp master-995-fd21ecf); this PR only merges it and does not touch this file. Leaving it to upstream.
|
|
||
| sd_image_t* detailed_images = nullptr; | ||
| int detailed_count = 0; | ||
| if (!adetail_image(ad_ctx.get(), |
There was a problem hiding this comment.
If the YOLO runner fails and returns an empty tensor, ADetailerGGML::predict logs the failure but returns the same empty detection vector used for a legitimate no-detection result. adetail_image then copies the original image and returns success, so this new server call produces HTTP success instead of the documented request failure. Preserve a distinct detector-error state and make this call fail while continuing to accept a successful inference with zero detections.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This code comes from upstream (leejet/stable-diffusion.cpp master-995-fd21ecf); this PR only merges it and does not touch this file. Leaving it to upstream.
|
@codex review |
danielhanchen
changed the title
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 344a3b663b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static int show_eta = -1; // -1 - estimate duration, 0 - hide, 1 - show | ||
| static float passed_time = 0.f; |
There was a problem hiding this comment.
When two sd_ctx_t instances generate concurrently without a custom progress callback, both threads reset and update these file-scope variables without synchronization. This creates a C++ data race and mixes elapsed times between generations, producing nondeterministic ETAs; keep the accounting per generation/context or otherwise synchronize it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This comes from upstream (leejet/stable-diffusion.cpp master-995-fd21ecf): src/core/util.cpp is identical to upstream at that commit, and this PR only merges it. Leaving it to upstream.
|
@codex review |
Syncs the fork with upstream
master-995-fd21ecf(2026-10-22). This replaces themaster-987-a77d362sync: upstream moved 8 commits since, including a ggml bump. Fork master ismaster-982-0518b34(#34), so this brings in 13 upstream commits in total.Merge this with "Create a merge commit", not squash or rebase. The prebuilt workflow names a release after the highest upstream
master-*tag reachable from the tree, sofd21ecfhas to stay reachable, and the tagmaster-995-fd21ecfhas to be in this repository (git push origin refs/tags/master-995-fd21ecf).Upstream commits brought in
From the earlier
a77d362sync:--list-devices(fix: include RPC devices in --list-devices leejet/stable-diffusion.cpp#2182), memory capacity rechecked when the eviction candidates did not change (fix: recheck memory capacity when eviction candidates are unchanged leejet/stable-diffusion.cpp#2183).New since
a77d362:--taesd-preview-onlyin the shared context params (feat: add --taesd-preview-only to shared context params, so that it can be used also in sd-server leejet/stable-diffusion.cpp#2184).backend/params_backend(feat(server): allow recipes to override backend/params_backend in context leejet/stable-diffusion.cpp#2188).dc99877toc1f80e5(fix: update ggml to avoid small BAR heaps on Vulkan leejet/stable-diffusion.cpp#2181): one commit, "ggml-vulkan: avoid small BAR heaps for device buffers".Conflicts
None.
git merge --no-ff upstream/masterapplied cleanly onf3b773a(thea77d362sync). All 13 patches inscripts/unsloth/ggml-patchesapply in order on ggmlc1f80e5.The CPU speed branch stacked on this one (#36) conflicts in
src/core/util.cpp, because leejet#2186 fixes the same sysfs typo #36 fixes. #36 keeps its version: upstream's loop now finds the files but still counts every CPU of the machine, so a 4-CPU cpuset on a 192-CPU host would still get 96 threads, while #36 counts the cores in the affinity mask.Verification
sd-clibuilt for thea77d362sync (f3b773a, ggmldc99877+ 13 patches) and for this branch (ggmlc1f80e5+ 13 patches), same GCC, CPU backend, 16 threads, seed 42, 256x256, 4 steps:f3b773avs this branch--ref-imageThe ggml change is Vulkan-only (memory heap selection for device buffers), so the CPU renders cannot show it.
Not covered