Allocation follow-up: consuming output finalization and HDR-to-SDR intermediates
- Dominant language
- Rust
- Stars
- 1
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
Inventory of every allocating public API in both crates. The pattern: **borrow-in / own-out signatures force an allocation that no short-circuit can remove.** The fix is `_into` / `&mut self` siblings, not early returns.
> Correction worth stating up front: an "identity ⇒ move instead of copy" fast path **cannot work** for these. `convert_to(&self, ..) -> PixelBuffer`, `convert_buffer(src: &[u8], ..) -> Vec` and `finalize_for_output_with(buffer: &PixelBuffer, ..) -> EncodeReady` all take a borrow and return an owned value — you cannot move out of a `&self`. The allocation is the signature, not an oversight.
## Allocates on a path where the output is byte-identical to the input
| # | Site | Detail |
|---|---|---|
| 1 | `ext.rs:199-218` | `convert_to` with `src_desc == target`: allocates a full buffer and memcpys every row; context cloned (`:214-216`). Output byte-identical. |
| 2 | `output.rs:457-465` **and** `:311-319` | `finalize_for_output(_with)` fast path: `contiguous_bytes()` returns `Cow::Borrowed`, then `.into_owned()` **throws the borrow away** — on the branch where `descriptors_match` (`output.rs:520`) has just proven format/transfer/primaries/signal_range all equal. |
| 3 | `adapt.rs:334-336` | `convert_buffer` with `from == to`: `src.to_vec()`. |
| 4 | `ext.rs:474-481` | `convert_to_typed` (`to_rgba8`/`to_rgb8`/`to_gray8`/`to_bgra8`) has **no identity check at all** — `to_rgba8()` on an RGBA8_SRGB buffer builds an identity plan and memcpys. Also `.expect()`s (panics) at `:477`, `:492`. |
| 5 | `orient/mod.rs:157-161` | `apply_orientation` with `Orientation::Identity` allocates **and** copies. Documented as the caller's job (`:112`), but EXIF orientation 1 is the overwhelmingly common case. |
| 6 | `adapt.rs:797-802` | `adapt_for_encode*` on strided-but-matching input: repacked, not converted. See the `Adapted`-stride issue. |
**#2 is the sharpest.** `finalize_for_output_with` is the blessed encode path — `lib.rs:392`'s codec checklist tells every encoder to use it — and it copies every image needlessly. An overload taking `PixelBuffer` **by value** could move the allocation through and re-tag the descriptor; or `EncodeReady<'a>` could hold borrowed pixels.
## Missing `_into` siblings
| Item | Sibling? |
|---|---|
| `PixelBufferConvertExt::convert_to` (`ext.rs:196`) | **NONE.** Highest leverage: `try_add_alpha` (`:245`), `try_widen_to_u16` (`:263`), `try_narrow_to_u8` (`:275`), `linearize` (`:287`), `delinearize` (`:300`) **all delegate to it**, so one `convert_into(&self, dst: PixelSliceMut)` collapses the whole family. |
| `to_rgba8` / `to_rgb8` / … (`ext.rs:455-467`) | NONE — and this is the top decode idiom downstream (~93 fully-qualified imports). |
| `finalize_for_output_with` (`output.rs:417`) | NONE |
| `PixelBufferHdrConvertExt::convert_to_sdr` (`ext.rs:362`) | NONE — and it allocates **two** full images: an F32-linear intermediate (`:396`, **4× an RGBA8 source**) purely to measure MaxCLL, then the output. `CllMeasure` takes a `PixelSlice`, so it could measure row-at-a-time through a reused scratch row. |
## Already written, correct, and held back — promote when a consumer lands
- `hdr::quantize_into` (`hdr/mod.rs:410`) — stride-correct both ends, tested (`hdr/mod.rs:789`), `pub(crate)`. `lib.rs:563` states it stays private "until a concrete consumer … lands." That is YAGNI applied correctly, not a defect.
- `adapt::convert_into_with_anchor` (`adapt.rs:420`) — fully both-ended stride-correct, `pub(crate)`. This **is** the no-alloc `convert_buffer`.
## Silent trap (a convenient footgun)
`PixelBuffer::into_contiguous_pixels
` (`buffer.rs:2016`) **always copies for any multi-byte `P`**. The backing is `Vec` (align 1), so `bytemuck::try_cast_vec` (`:2031`) can never satisfy `align_of::>()` / `Rgba` and silently falls to `.to_vec()` (`:2035-2037`). Zero-copy is reachable **only** for the `u8` pixel types. The doc (`:2014`) is accurate but the trap is easy to miss — the name promises a move.
## Verified NON-offenders (do not "fix" these)
- `synthesize_icc_for_cicp` / `synthesize_gray_icc_for_cicp` (`icc_profiles.rs:340`/`443`) — `Cow::Borrowed` on **every** path; the only allocation is a one-time-per-transfer-group LZ4 decode cached in a `OnceLock`.
- `PixelSlice::contiguous_bytes` (`buffer.rs:698`) — borrows when packed, allocates only when strided. Good citizen; it is two **callers** (`output.rs:312`, `:458`) that defeat it with `.into_owned()`.
- `try_reduce_to_load_bearing_format` (`load_bearing.rs:326`) — returns `None` **without allocating** when nothing reduces (`:340-342`).
- The `ConvertPlan` step machinery (`convert.rs:2018-2130`) — row-scoped ping-pong scratch, grown once and reused (`:1983-1988`), last step writes straight to `dst`. Per-image cost is O(row), amortized to zero. Not an offender.
- `gamut::apply_matrix*` — in-place `&mut` kernels. `negotiate`/`best_match`, `icc::{extract_cicp, identify_common}` — allocate nothing.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.