Add ImageProcessor library to ExecuTorch (#19967)

[metascroy]

Summary:

Introduces an ImageProcessor library to ExecuTorch: a small, well-tested
core for turning camera/decoded image buffers into normalized model-input
tensors. This initial implementation is fully portable; performant
platform-specific backends (e.g. Apple Accelerate/Core Image) are added in
follow-up diffs behind the same API.

Given source pixels (BGRA/RGBA bytes or semi-planar YUV planes), the
processor crops/resizes to the configured target and emits a
[1, 3, target_height, target_width] float tensor, normalized per the
configured scheme.

Configuration (ImageProcessorConfig):

• target_width / target_height: output spatial size.
• resize_mode: how the source is fit to the target.
• letterbox_anchor: placement of content when letterboxing.
• pad_value: fill value for letterbox padding.
• normalization: how uint8 [0,255] pixels map to floats.
• gpu_min_input_pixels: minimum source pixel count (width * height) at which
  the GPU path is used; smaller inputs run on CPU. Sentinels: 0 = always GPU,
  INT64_MAX = always CPU (defaults to ~1080p, i.e. 1920*1080 + 1). Platform
  backends consult this to pick CPU vs GPU. (Note GPU is always platform optional; but GPU will never be routed when below gpu_min_input_pixels.)

Enum options:
Enum options:

• ColorFormat (BGRA, RGBA): interleaved 8-bit input layout.
• YUVFormat (NV12, NV21): semi-planar YUV input layout.
• ResizeMode (STRETCH, LETTERBOX): STRETCH fills the target, ignoring aspect ratio; LETTERBOX preserves aspect ratio and pads.
• LetterboxAnchor (CENTER, TOP_LEFT): where padded content is anchored.
• Orientation (UP): source orientation (upright).
• Normalization (zeroToOne, imagenet): zeroToOne scales to [0, 1]; imagenet additionally applies per-channel mean/std.

Example:

  #include <executorch/extension/image/image_processor.h>

  using namespace executorch::extension::image;

  // Resize to 224x224, letterboxed, with ImageNet normalization.
  ImageProcessorConfig config;
  config.target_width = 224;
  config.target_height = 224;
  config.resize_mode = ResizeMode::LETTERBOX;
  config.normalization = Normalization::imagenet();

  ImageProcessor processor(config);

  // RGBA/BGRA input -> normalized [1, 3, 224, 224] float tensor.
  auto result = processor.process(
      pixels, width, height, /*stride_bytes=*/width * 4, ColorFormat::RGBA);
  if (result.ok()) {
    TensorPtr tensor = std::move(result.get());
    // feed `tensor` to the model...
  }

  // Semi-planar YUV (e.g. NV12 camera frame):
  auto yuv = processor.process_yuv(
      y_plane, y_stride, uv_plane, uv_stride, width, height, YUVFormat::NV12);

Reviewed By: psiddh

Differential Revision: D106898421

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19967

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[meta-codesync]

@metascroy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D106898421.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[metascroy]

@claude review this code

[claude]

Claude finished @metascroy's task in 1m 35s —— View job

---

Code Review: Add ImageProcessor library to ExecuTorch (#19967)

• Read and analyze all changed files
• Review header files (API design, config)
• Review implementation files
• Review test file
• Review build system files
• Post final review

---

Overall this is a well-structured, well-tested library addition. The API is clean, the pImpl pattern is appropriate for the planned platform backends, and the test coverage is thorough. Below are my findings:

---

Issues

1. Redundant memory allocation in process_yuv_into (medium severity)

extension/image/image_processor.cpp:366-384 — The YUV path allocates two buffers (rgb and rgba), converts YUV→RGB, then copies RGB→RGBA pixel-by-pixel just to feed process_into which immediately swizzles RGBA→RGB again. This is 3 full-frame passes and 2x the necessary memory for the intermediate step.

The YUV→RGB conversion already produces RGB. You could either:

• Convert directly to RGBA in yuv_to_rgb_semi_planar (avoiding the rgb intermediate), or
• Refactor process_into to accept RGB directly (since the swizzle step already produces RGB internally).

The current approach works correctly but doubles memory and adds an unnecessary O(n) copy for every YUV frame.

Fix this →

2. process() discards the orientation parameter (low severity)

extension/image/image_processor.cpp:398-405 — The process() wrapper takes an Orientation orientation parameter but names it /*orientation*/ (unused), then hardcodes Orientation::UP when calling process_into. If orientation is always UP for the portable backend, this is fine for now, but the parameter should probably be forwarded rather than discarded — a caller passing a non-UP orientation would silently get wrong results.

// Line 424: hardcodes UP instead of forwarding the caller's value
    auto err = process_into(
        data, width, height, stride_bytes, input_format, *out,
        Orientation::UP,  // should be `orientation`
        roi);

3. Integer overflow potential in yuv_to_rgb_semi_planar stride arithmetic (low severity)

extension/image/image_processor.cpp:50-51 — y * y_stride and y * rgb_stride use int32_t multiplication. For large frames (e.g., 4K with padded strides), height * stride can approach INT32_MAX. The process_yuv_into validates dimensions but doesn't bound the product. Consider using static_cast<size_t>(y) * y_stride for pointer offset arithmetic, as is done elsewhere in the file.

---

Suggestions (non-blocking)

4. Normalization has 4-element arrays but only 3 channels are used

extension/image/image_processor_config.h:63-78 — mean[4] and std_dev[4] have a 4th element that's never read by the processing pipeline (which always uses kChannels = 3). The 4th slot in imagenet() is set to {0.0f} / {1.0f} which is a reasonable sentinel, but documenting why it's 4 (or shrinking to 3) would clarify intent. If it's for future alpha-channel normalization, a brief note would help.

5. compute_resize_dims missing default case

extension/image/image_processor_config.h:143-157 — The switch on config.resize_mode has no default case. If a new ResizeMode variant is added later, the function would fall through without initializing resize_w / resize_h, leading to UB. Consider adding a default: that sets them to tw/th or triggers an assertion.

6. Consider reserving the stb dependency behind a build flag or vendoring the specific file

extension/image/CMakeLists.txt:11-17 — FetchContent pulls the entire stb repo at build time (including all ~20 stb headers). Since only stb_image_resize.h is needed, vendoring just that file or using a shallow clone would reduce fetch time and avoid network dependencies during builds.

---

Positive notes

• Excellent test coverage: parameterized over GPU/CPU paths, error cases, stride handling, ROI, YUV range, and cross-stage integration.
• Good defensive validation at entry points (process_into, process_yuv_into) — checking null pointers, dimension validity, stride bounds, ROI bounds, and output tensor properties.
• The pImpl pattern is well-suited for swapping platform backends without recompiling dependents.
• The compute_letterbox_padding API is a thoughtful addition — callers need this to map detections back to source coordinates.
• Build system integration is clean: optional flag, proper dependency declaration, and the check_required_options_on for EXTENSION_TENSOR.

---

[metascroy]

Claude finished @metascroy's task in 1m 35s —— View job

Code Review: Add ImageProcessor library to ExecuTorch (#19967)

• Read and analyze all changed files
• Review header files (API design, config)
• Review implementation files
• Review test file
• Review build system files
• Post final review

Overall this is a well-structured, well-tested library addition. The API is clean, the pImpl pattern is appropriate for the planned platform backends, and the test coverage is thorough. Below are my findings:

Issues

1. Redundant memory allocation in process_yuv_into (medium severity)

extension/image/image_processor.cpp:366-384 — The YUV path allocates two buffers (rgb and rgba), converts YUV→RGB, then copies RGB→RGBA pixel-by-pixel just to feed process_into which immediately swizzles RGBA→RGB again. This is 3 full-frame passes and 2x the necessary memory for the intermediate step.

The YUV→RGB conversion already produces RGB. You could either:

• Convert directly to RGBA in yuv_to_rgb_semi_planar (avoiding the rgb intermediate), or
• Refactor process_into to accept RGB directly (since the swizzle step already produces RGB internally).

The current approach works correctly but doubles memory and adds an unnecessary O(n) copy for every YUV frame.

Fix this →

2. process() discards the orientation parameter (low severity)

extension/image/image_processor.cpp:398-405 — The process() wrapper takes an Orientation orientation parameter but names it /*orientation*/ (unused), then hardcodes Orientation::UP when calling process_into. If orientation is always UP for the portable backend, this is fine for now, but the parameter should probably be forwarded rather than discarded — a caller passing a non-UP orientation would silently get wrong results.

// Line 424: hardcodes UP instead of forwarding the caller's value
    auto err = process_into(
        data, width, height, stride_bytes, input_format, *out,
        Orientation::UP,  // should be `orientation`
        roi);

3. Integer overflow potential in yuv_to_rgb_semi_planar stride arithmetic (low severity)

extension/image/image_processor.cpp:50-51 — y * y_stride and y * rgb_stride use int32_t multiplication. For large frames (e.g., 4K with padded strides), height * stride can approach INT32_MAX. The process_yuv_into validates dimensions but doesn't bound the product. Consider using static_cast<size_t>(y) * y_stride for pointer offset arithmetic, as is done elsewhere in the file.

Suggestions (non-blocking)

4. Normalization has 4-element arrays but only 3 channels are used

extension/image/image_processor_config.h:63-78 — mean[4] and std_dev[4] have a 4th element that's never read by the processing pipeline (which always uses kChannels = 3). The 4th slot in imagenet() is set to {0.0f} / {1.0f} which is a reasonable sentinel, but documenting why it's 4 (or shrinking to 3) would clarify intent. If it's for future alpha-channel normalization, a brief note would help.

5. compute_resize_dims missing default case

extension/image/image_processor_config.h:143-157 — The switch on config.resize_mode has no default case. If a new ResizeMode variant is added later, the function would fall through without initializing resize_w / resize_h, leading to UB. Consider adding a default: that sets them to tw/th or triggers an assertion.

6. Consider reserving the stb dependency behind a build flag or vendoring the specific file

extension/image/CMakeLists.txt:11-17 — FetchContent pulls the entire stb repo at build time (including all ~20 stb headers). Since only stb_image_resize.h is needed, vendoring just that file or using a shallow clone would reduce fetch time and avoid network dependencies during builds.

Positive notes

• Excellent test coverage: parameterized over GPU/CPU paths, error cases, stride handling, ROI, YUV range, and cross-stage integration.
• Good defensive validation at entry points (process_into, process_yuv_into) — checking null pointers, dimension validity, stride bounds, ROI bounds, and output tensor properties.
• The pImpl pattern is well-suited for swapping platform backends without recompiling dependents.
• The compute_letterbox_padding API is a thoughtful addition — callers need this to map detections back to source coordinates.
• Build system integration is clean: optional flag, proper dependency declaration, and the check_required_options_on for EXTENSION_TENSOR.

Addressed 1, 2, 4 (with comment), and 5.

3 is incorrect. Elsewhere size_t is used for allocation, not indexing. height * stride does not come close to approaching INT32_MAX for images.

[metascroy]

@claude review this code

[claude]

Claude finished @metascroy's task in 3m 14s —— View job

---

Code Review: Add ImageProcessor library to ExecuTorch (#19967)

• Read and analyze all changed files
• Review updated implementation (post-fix)
• Review header files and config
• Review tests
• Post final review

---

Re-reviewed after the fixes for issues 1, 2, 4, and 5 from the previous review. The code is in good shape — the YUV path now writes RGBA directly (eliminating the double-buffer), orientation is forwarded correctly, the Normalization arrays have a clear comment, and compute_resize_dims pre-initializes out-params to avoid UB while preserving -Wswitch coverage. Nice fixes.

I have one remaining issue and a few minor suggestions:

---

Issues

1. process_into silently ignores non-UP orientation (low severity)

extension/image/image_processor.cpp:195 — The parameter forwarding fix is correct (callers now pass the real value), but process_into still discards it via /*orientation*/. A caller passing e.g. a hypothetical Orientation::RIGHT would get a correctly-oriented result from a platform backend but silently wrong output from the portable backend. Consider either:

• Returning Error::InvalidArgument if orientation != Orientation::UP in the portable backend (fail-fast), or
• Documenting at the API level that non-UP orientations require a platform backend.

Since Orientation currently only has UP, this is purely forward-looking and non-blocking.

---

Suggestions (non-blocking)

2. stb fetch pulls the full repo

extension/image/CMakeLists.txt:12-17 — FetchContent pulls the entire stb repository (~20 headers, history). Since only stb_image_resize.h (from deprecated/) is used, a GIT_SHALLOW TRUE would reduce clone time on CI. Alternatively, vendoring the single file avoids the network dependency entirely. (I see this is pinned to a specific commit, which is good.)

3. make_tensor_ptr with std::vector for the owned buffer

extension/image/image_processor.cpp:422-424 — The allocating process() creates a std::vector<float> and hands it to make_tensor_ptr. This is fine for the portable backend, but worth noting that on hot paths (video frames), callers should prefer process_into with a pre-allocated tensor, which is already documented in the header — good.

---

Positive notes

• The yuv_to_rgba_semi_planar rewrite is cleaner and avoids the extra allocation + O(n) copy. Well done.
• The compute_resize_dims fix (pre-initialize + no default:) is elegant — gets both safety and compile-time exhaustiveness checking.
• The Normalization comment on the reserved 4th slot is clear and sufficient.
• Test coverage remains excellent: parameterized across GPU/CPU, error paths, stride handling, ROI, YUV range, cross-stage integration, and thread safety.
• The overall API design is clean and production-ready.

LGTM with the caveat on orientation behavior for future backends.

---

[metascroy]

Claude finished @metascroy's task in 3m 14s —— View job

Code Review: Add ImageProcessor library to ExecuTorch (#19967)

• Read and analyze all changed files
• Review updated implementation (post-fix)
• Review header files and config
• Review tests
• Post final review

Re-reviewed after the fixes for issues 1, 2, 4, and 5 from the previous review. The code is in good shape — the YUV path now writes RGBA directly (eliminating the double-buffer), orientation is forwarded correctly, the Normalization arrays have a clear comment, and compute_resize_dims pre-initializes out-params to avoid UB while preserving -Wswitch coverage. Nice fixes.

I have one remaining issue and a few minor suggestions:

Issues

1. process_into silently ignores non-UP orientation (low severity)

extension/image/image_processor.cpp:195 — The parameter forwarding fix is correct (callers now pass the real value), but process_into still discards it via /*orientation*/. A caller passing e.g. a hypothetical Orientation::RIGHT would get a correctly-oriented result from a platform backend but silently wrong output from the portable backend. Consider either:

• Returning Error::InvalidArgument if orientation != Orientation::UP in the portable backend (fail-fast), or
• Documenting at the API level that non-UP orientations require a platform backend.

Since Orientation currently only has UP, this is purely forward-looking and non-blocking.

Suggestions (non-blocking)

2. stb fetch pulls the full repo

extension/image/CMakeLists.txt:12-17 — FetchContent pulls the entire stb repository (~20 headers, history). Since only stb_image_resize.h (from deprecated/) is used, a GIT_SHALLOW TRUE would reduce clone time on CI. Alternatively, vendoring the single file avoids the network dependency entirely. (I see this is pinned to a specific commit, which is good.)

3. make_tensor_ptr with std::vector for the owned buffer

extension/image/image_processor.cpp:422-424 — The allocating process() creates a std::vector<float> and hands it to make_tensor_ptr. This is fine for the portable backend, but worth noting that on hot paths (video frames), callers should prefer process_into with a pre-allocated tensor, which is already documented in the header — good.

Positive notes

• The yuv_to_rgba_semi_planar rewrite is cleaner and avoids the extra allocation + O(n) copy. Well done.
• The compute_resize_dims fix (pre-initialize + no default:) is elegant — gets both safety and compile-time exhaustiveness checking.
• The Normalization comment on the reserved 4th slot is clear and sufficient.
• Test coverage remains excellent: parameterized across GPU/CPU, error paths, stride handling, ROI, YUV range, cross-stage integration, and thread safety.
• The overall API design is clean and production-ready.

LGTM with the caveat on orientation behavior for future backends.

Orientation is an enum and UP is only valid value, so will not address. Future diff in stack extends it.

[metascroy]

Note follow-up diffs will provide platform-accelerated implementations. Here is a preview of coming perf improvements for Apple:

Test case A: 4K video
ImageProcessor::process_into benchmark
  input:   3840x2160 RGBA (31.6 MB/frame)
  target:  224x224 (STRETCH)
  iters:   200 (+20 warmup)
  
Compiled with portable C++ implementation (diff 1):
results:
  avg:    71.213 ms/frame  (14.0 FPS)
  min:    52.511 ms/frame
  max:   116.094 ms/frame
  total:14242.592 ms over 200 frames
Compiled with Apple optimized implementation (diff 2):
results:
  avg:     1.252 ms/frame  (799.0 FPS)
  min:     0.962 ms/frame
  max:     3.759 ms/frame
  total: 250.311 ms over 200 frames
  
  
Accelerated implementation is 57x faster. 
--------------------------------------------------------------
Test case B: 1080p video
ImageProcessor::process_into benchmark
  input:   1920x1080 RGBA (7.9 MB/frame)
  target:  224x224 (STRETCH)
  iters:   200 (+20 warmup)
Compiled with portable C++ implementation (diff 1):
results:
  avg:    21.372 ms/frame  (46.8 FPS)
  min:    19.383 ms/frame
  max:    88.639 ms/frame
  total:4274.457 ms over 200 frames
Compiled with Apple optimized implementation (diff 2):
  
results:
  avg:     0.790 ms/frame  (1265.7 FPS)
  min:     0.685 ms/frame
  max:     3.762 ms/frame
  total: 158.012 ms over 200 frames
  
Accelerated implementation is 27x faster. 

[psiddh]

cast to size_t before the multiply ? Potential Integer overflow ?

[metascroy]

Changed

[psiddh]

Same potential issue with overflow ?

[psiddh]

nits: not sure if std_dev can be 0, maybe a defensive check for std_dev !=0

or aomething like
ET_CHECK_OR_RETURN_ERROR(norm.std_dev[c] != 0.0f, InvalidArgument,
"std_dev must be nonzero");

[metascroy]

Added check at process_into.

[psiddh]

Nice to have: Looks like process_into allocates two scratch buffers on every call (lines 261, 279 — ~6 MB at 1080p). Since the API's purpose is to avoid per-call allocation, and the threading model is already one-instance-per-thread, these could be cached in Impl and reused across calls, unless I am missing something.
Same I guess for the rgba buffer in process_yuv_into (line 380).

[metascroy]

Caching in impl is an optimization in the Apple version. I don't want to spend too much optimizing these for-loop based implementations, though.

[psiddh]

looks great to me! Thanks for doing this including perf testings