Skip to content

Migrate WASM backend from Rust to Emscripten/embind libultrahdr#15

Merged
adamsilverstein merged 4 commits into
mainfrom
14-emscripten-migration
May 8, 2026
Merged

Migrate WASM backend from Rust to Emscripten/embind libultrahdr#15
adamsilverstein merged 4 commits into
mainfrom
14-emscripten-migration

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented May 8, 2026

Copy link
Copy Markdown
Owner

Closes #14.

Replaces the in-house Rust UltraHDR reimplementation with a thin Emscripten/embind C++ wrapper around upstream google/libultrahdr, now that upstream is dual-licensed Apache-2.0 OR MIT (google/libultrahdr#386) and can be redistributed freely.

Why

The Rust crate existed because libultrahdr was Apache-2.0-only and we needed GPLv2 compatibility. With upstream now permissively dual-licensed, building on the canonical implementation gives us:

  • spec-conformant ISO 21496-1 and UltraHDR v1 behavior maintained by Google,
  • a much smaller surface to own (one C++ binding file instead of a full reimplementation),
  • a more permissive Apache-2.0 OR MIT license for downstream consumers (notably WordPress/gutenberg#74873).

What changed

Before After
WASM source Rust crate (~3000 LoC) wasm/src/bindings.cpp (~470 LoC) + jpeg_meta.cpp (~50 LoC)
Toolchain rustc + wasm-pack emsdk + cmake + emcc
Upstream none (reimplementation) google/libultrahdr @ 82b4f6da (git submodule)
JPEG codec jpeg-decoder + jpeg-encoder crates Emscripten libjpeg port (--use-port=libjpeg)
License GPL-2.0-or-later Apache-2.0 OR MIT
Versions wasm 0.1.2, js 0.1.1 both 0.2.0
WASM size 341 KB 577 KB (+1.7×, within the <2× budget)

Public JS API surface is unchanged — all exports keep the same names, signatures, and return shapes. Downstream consumers just bump the dep.

Verification

  • npm run build:wasm produces wasm/pkg/open_ultrahdr.{js,wasm,d.ts} from a clean checkout (with emsdk activated)
  • npm test (JS) — 47/47 tests pass locally, exercising probe/decode/encode/extract/metadata/round-trips
  • npm run typecheck clean (tsc --noEmit)
  • npm run lint:js clean
  • npm run format:check clean

Test fixture change

The legacy 2×2 JPEG fixture is replaced with a 16×16 fixture. libultrahdr enforces an 8×8 minimum dimension for spec-conformant gain map generation; the prior Rust crate's lax handling of tiny images was not spec-compliant. This is a test-only change — public API behavior is identical.

Migration phases (per #14)

  • Vendor libultrahdr submodule, remove Rust crate
  • CMake/Emscripten build with embind bindings (probe/decode/encode/extract/metadata)
  • Hand-authored pkg/open_ultrahdr.d.ts
  • Rewire js/src/index.ts to Emscripten factory + locateFile
  • Relicense to Apache-2.0 OR MIT, bump to 0.2.0
  • CI uses mymindstorm/setup-emsdk@v14
  • Downstream: bump dep in Media: Add UltraHDR (ISO 21496-1) gain map support WordPress/gutenberg#74873 (separate)

Risks / notes

  1. Size: 577 KB wasm vs 341 KB before. Acceptable per the budget called out in Migrate WASM backend from Rust reimplementation to Emscripten/embind build of libultrahdr #14. We can revisit -Oz if it becomes an issue.
  2. Threading: kept single-threaded (no pthreads, no crossOriginIsolated requirement). libultrahdr supports multi-threading; we accept the perf hit for deployment simplicity, revisit later.
  3. Floating-point HDR input: the JS-side API still takes Float32Array (RGB, 3 floats/px). The C++ binding packs to UHDR_IMG_FMT_64bppRGBAHalfFloat (RGBA F16) before handing to libultrahdr.
  4. Metadata scale: kept in log2 stops at the JS boundary (matching the Rust crate); the binding converts to/from libultrahdr's linear scale internally so no consumer-visible change.

Commits

  1. Migrate WASM backend from Rust to Emscripten/embind libultrahdr
  2. Relicense from GPL-2.0-or-later to Apache-2.0 OR MIT
  3. Update CI workflows for Emscripten toolchain

Summary by CodeRabbit

  • New Features
    • Added public API to override the WASM module URL
    • Probe API now returns a deterministic “invalid” result instead of throwing on WASM errors
  • Chores
    • Changed project license to dual Apache-2.0 / MIT
    • Switched WASM build to Emscripten/CMake and updated CI/CD accordingly
    • Bumped package versions to 0.2.0
  • Documentation
    • README and package docs updated to reflect licensing and build changes
  • Tests
    • Test fixtures enlarged for more robust integration coverage

Replace the in-house Rust UltraHDR reimplementation with a thin
Emscripten/embind C++ wrapper around upstream google/libultrahdr,
vendored as a git submodule pinned to commit 82b4f6da (the dual-license
merge from google/libultrahdr#386).

The Rust crate existed because libultrahdr was Apache-2.0-only and could
not be redistributed under GPLv2-compatible terms. With upstream now
dual-licensed Apache-2.0 OR MIT, building on the canonical implementation
gives us spec-conformant behavior, ongoing security/perf maintenance, and
a much smaller surface to own.

Public JS API of the open-ultrahdr package is unchanged (same exports,
signatures, and return shapes); only the implementation backend differs.
Bumps both packages to 0.2.0 since the WASM binary identity changes.

Build chain: emcmake + cmake + emcc, --use-port=libjpeg, MODULARIZE=1 +
EXPORT_ES6=1, single-threaded, FILESYSTEM=0. WASM size 577 KB vs prior
Rust 341 KB (+1.7x, well within the <2x budget noted in the issue).

Test fixture sizes raised from 2x2 to 16x16 to satisfy libultrahdr's
documented 8x8 minimum dimensions; the prior Rust impl's lax handling
of tiny images was not spec-compliant.

Refs #14
Mirror upstream libultrahdr's dual licensing now that google/libultrahdr#386
has shipped. Apache-2.0 OR MIT is far more permissive than GPL-2.0 and
unlocks downstream consumers (notably WordPress/gutenberg) that need a
permissive license for their own redistribution.

LICENSE becomes a short preamble pointing at LICENSE-APACHE and LICENSE-MIT
(the same layout libultrahdr uses). README's GPLv2 framing is replaced
with the dual-license note. Per-package license fields were already updated
to "Apache-2.0 OR MIT" in the migration commit.

Refs #14
Replace the Rust + wasm-pack build steps with an emsdk + cmake build via
mymindstorm/setup-emsdk@v14. Submodules are checked out recursively so
libultrahdr is available to the build. Ts-lint, ts-typecheck, and the
e2e matrix continue to consume the wasm artifact unchanged.

Refs #14
@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Migrates WASM backend from a Rust/wasm-pack build to an Emscripten/embind C++ build of upstream libultrahdr (vendored as a git submodule), rewrites JS WASM loader, updates CI/publish workflows for emsdk, converts license to Apache-2.0 OR MIT, and bumps package versions to 0.2.0. Tests and fixtures updated for larger synthetic images.

Changes

WASM Backend and Licensing Migration

Layer / File(s) Summary
Licensing & Manifests
LICENSE, LICENSE-APACHE, LICENSE-MIT, README.md, js/README.md, package.json, js/package.json, wasm/package.json
Replace GPLv2 with dual Apache-2.0 OR MIT; bump versions to 0.2.0; update README license text and package metadata and descriptions.
VCS / Ignores / Submodules
.gitmodules, .gitignore, wasm/.npmignore, wasm/third_party/libultrahdr
Add libultrahdr submodule; adjust .gitignore to include wasm build artifacts; remove wasm/.npmignore Rust exclusions.
Build System: CMake / Emscripten
wasm/CMakeLists.txt, wasm/package.json, wasm/README.md, wasm/Cargo.toml, wasm/rustfmt.toml
Introduce Emscripten/CMake build target open_ultrahdr (MODULARIZE+EXPORT_ES6), switch wasm package scripts from wasm-pack to emcmake/cmake, remove old Rust Cargo manifest and rustfmt config.
C++ Embind Surface & Helpers
wasm/src/bindings.cpp, wasm/src/jpeg_meta.h, wasm/src/jpeg_meta.cpp
Add embind bindings exposing probe/decode/encode/metadata APIs and helpers; add JPEG-dimension parsing C++ helpers used by bindings.
WASM Type Declarations
wasm/pkg/open_ultrahdr.d.ts
Add hand-authored TypeScript declarations for the embind-built module and its options/results.
Rust Modules Reorganized/Removed
wasm/src/* (removed/changed exports: gainmap, jpeg, ultrahdr, wasm/Cargo.toml)
Rust module re-exports and some crate files removed or replaced as part of migration to C++ embind implementation.
Gainmap/JPEG Core (Rust)
wasm/src/gainmap/*, wasm/src/jpeg/*, wasm/src/ultrahdr/*, wasm/src/error.rs, wasm/src/lib.rs, wasm/src/types.rs
Large Rust implementation files present in the diff (new/old content shown); many modules restructured or re-scoped for the new embind entrypoints and rework of public exports.
JS Loader / API Layer
js/src/index.ts, js/src/types.ts
Add setWasmUrl, rework getWasm to dynamically import embind module and resolve .wasm via locateFile or Node fs, cache instances and prevent stale in-flight instances; make probeUltraHdr return deterministic invalid result on WASM errors; pass metadata/options objects directly into WASM functions.
Tests & Fixtures
js/__tests__/fixtures/test-data.ts, js/__tests__/*.test.ts
Update fixtures to use 16×16 synthetic JPEG/HDR data; update tests assertions and HDR data generation accordingly.
CI / Publish Workflows
.github/workflows/ci.yml, .github/workflows/npm-publish.yml
Replace Rust/wasm-pack WASM job with emsdk + emcmake/cmake build, set up Emscripten SDK steps, adjust artifact assumptions; remove rust-lint/rust-test from final success gate; npm-publish workflow simplified to emsdk setup and JS test invocation.
JS Packaging / Scripts
root package.json, js/package.json
Top-level and workspace scripts simplified to JS-only lint/test/format flows; build still invokes both wasm and js; dependency pins updated for open-ultrahdr-wasm^0.2.0.

Sequence Diagram

sequenceDiagram
    participant JS as JS App Layer
    participant Loader as getWasm()
    participant Emscripten as Emscripten Module
    participant Lib as libultrahdr (C++)
    participant FS as Filesystem/Network

    rect rgba(100, 200, 150, 0.5)
    Note over JS,Loader: WASM Module Initialization
    JS->>Loader: getWasm()
    Loader->>Emscripten: dynamic import open-ultrahdr-wasm
    alt explicitWasmUrl set
        Emscripten->>FS: fetch(customUrl)
    else Node with setLocation
        Emscripten->>FS: fs.readFileSync(.wasm)
    else browser default
        Emscripten->>FS: locateFile resolver
    end
    FS-->>Emscripten: .wasm binary
    Emscripten->>Emscripten: instantiate WASM, bind functions
    Emscripten-->>Loader: OpenUltraHdrModule (cached)
    Loader-->>JS: module instance
    end

    rect rgba(150, 150, 200, 0.5)
    Note over JS,Emscripten: Encode Flow
    JS->>Emscripten: encodeUltraHdr(sdrBytes, hdrFloats, options)
    Emscripten->>Lib: parse JPEG, pack F32→F16, encode gainmap
    Lib->>Lib: compute gainmap, create XMP/MPF, assemble JPEG
    Lib-->>Emscripten: encoded JPEG bytes
    Emscripten-->>JS: Uint8Array result
    end

    rect rgba(200, 150, 150, 0.5)
    Note over JS,Emscripten: Decode Flow
    JS->>Emscripten: decodeUltraHdr(jpegBuffer)
    Emscripten->>Lib: parse XMP/MPF, extract gainmap & SDR
    Lib->>Lib: decode gainmap, apply gains, return metadata
    Lib-->>Emscripten: {sdr, gainmap, metadata, dims}
    Emscripten-->>JS: UltraHdrDecodeResult
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped into the build with vim,
C++ and embind — a brand new whim.
libultrahdr now snug and tight,
dual-license banner waving bright.
Same JS smile, but backend primed — let’s ship!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 14-emscripten-migration

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
js/__tests__/fixtures/test-data.ts (1)

74-78: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Stale comment: createMinimalJpegBuffer still says "2x2 JPEG".

The function comment on Line 75 was not updated to reflect the fixture change to 16×16. Additionally, the function accepts _width and _height parameters but unconditionally ignores them — any caller passing different dimensions will silently get the pre-encoded 16×16 JPEG back. Consider either removing the parameters (since REGULAR_JPEG_BASE64 is public and callers can call base64ToArrayBuffer directly) or updating the comment to clarify the fixed-size behavior.

🔧 Proposed fix
-export function createMinimalJpegBuffer(_width: number, _height: number): ArrayBuffer {
-	// Return the pre-encoded 2x2 JPEG for simplicity
-	// In a real scenario, we'd use a library like sharp or canvas
+export function createMinimalJpegBuffer(): ArrayBuffer {
+	// Return the pre-encoded 16x16 JPEG fixture.
+	// For dynamic sizes, use a library like sharp or canvas.
 	return base64ToArrayBuffer(REGULAR_JPEG_BASE64);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/__tests__/fixtures/test-data.ts` around lines 74 - 78, The comment for
createMinimalJpegBuffer is stale and the function silently ignores its _width
and _height parameters; update the comment to state it returns a pre-encoded
16×16 JPEG (from REGULAR_JPEG_BASE64) and explicitly document that the _width
and _height args are ignored (or, if you can change callers, remove the unused
parameters and let callers call base64ToArrayBuffer(REGULAR_JPEG_BASE64)
directly); adjust the JSDoc/text in createMinimalJpegBuffer and keep
REGULAR_JPEG_BASE64 and base64ToArrayBuffer references to make the intended
fixed-size behavior clear.
🧹 Nitpick comments (1)
wasm/package.json (1)

48-48: ⚡ Quick win

Make wasm package test run real tests instead of a no-op.

The current script always succeeds, which can mask regressions when contributors run npm test from wasm/.

Suggested patch
-		"test": "echo 'WASM tests run via the JS package (cd ../js && npm test)'"
+		"test": "cd ../js && npm test"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wasm/package.json` at line 48, Replace the no-op "test" script in
wasm/package.json so it runs the real JS tests: update the "test" script value
to invoke the JS package test runner (for example use "npm --prefix ../js test"
or "npm --prefix ../js run test") so running npm test in wasm/ actually executes
and fails on test failures; edit the "test" entry in package.json accordingly
(look for the existing "test" script key).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 23-37: The workflow's "Build WASM" step runs npm without a pinned
Node, causing runner drift; add a step before "Build WASM" that uses
actions/setup-node (e.g., actions/setup-node@v4) to set node-version to a fixed
value or an env var (e.g., NODE_VERSION) so npm runs on a deterministic Node;
ensure the new setup-node step appears before the "Build WASM" step so the "npm
run build" in that step uses the pinned Node runtime.

In `@js/src/index.ts`:
- Around line 67-88: The cache reset is vulnerable to races because resetCache()
nulled initPromise but in-flight getWasm() calls can still resolve and overwrite
wasmInstance; add an init generation token (e.g., increment a module-level
initGeneration whenever setLocation or setWasmUrl is called or resetCache runs)
and have getWasm capture the current generation when it starts and compare it
before assigning to wasmInstance or clearing initPromise so only the latest
generation can publish its result; update resetCache(), setLocation(),
setWasmUrl(), and getWasm() to use this token check and avoid publishing stale
in-flight initializations.
- Around line 149-166: The try/catch only wraps the call to wasm.probeUltraHdr
so failures from getWasm() still throw; move the await getWasm() call inside the
try in probeUltraHdr so both WASM initialization and probe execution are caught,
and on any error return the same default UltraHdrProbeResult; locate function
probeUltraHdr and the getWasm() invocation and wrap both in the single try block
that returns the invalid/default result on failure.

In `@wasm/src/bindings.cpp`:
- Around line 57-64: UltraHdrEncodeOptions contains includeIsoMetadata and
includeUltrahdrV1 but encodeUltraHdr() never uses them; update encodeUltraHdr()
to read these fields from UltraHdrEncodeOptions and either (preferred) pass them
into the underlying encoder/configuration API (e.g., set equivalent flags on the
encoder instance or encoder options used by encodeUltraHdr()) so metadata
emission respects includeIsoMetadata and includeUltrahdrV1, or if the encoder
cannot support them, validate and return/throw an explicit error when those
flags are set; locate UltraHdrEncodeOptions and the encodeUltraHdr()
implementation and wire these two boolean flags into the encoder configuration
or add validation logic to reject unsupported combinations.

---

Outside diff comments:
In `@js/__tests__/fixtures/test-data.ts`:
- Around line 74-78: The comment for createMinimalJpegBuffer is stale and the
function silently ignores its _width and _height parameters; update the comment
to state it returns a pre-encoded 16×16 JPEG (from REGULAR_JPEG_BASE64) and
explicitly document that the _width and _height args are ignored (or, if you can
change callers, remove the unused parameters and let callers call
base64ToArrayBuffer(REGULAR_JPEG_BASE64) directly); adjust the JSDoc/text in
createMinimalJpegBuffer and keep REGULAR_JPEG_BASE64 and base64ToArrayBuffer
references to make the intended fixed-size behavior clear.

---

Nitpick comments:
In `@wasm/package.json`:
- Line 48: Replace the no-op "test" script in wasm/package.json so it runs the
real JS tests: update the "test" script value to invoke the JS package test
runner (for example use "npm --prefix ../js test" or "npm --prefix ../js run
test") so running npm test in wasm/ actually executes and fails on test
failures; edit the "test" entry in package.json accordingly (look for the
existing "test" script key).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73c344cb-3051-4990-bb6f-46e69040a488

📥 Commits

Reviewing files that changed from the base of the PR and between 66535a0 and 2d03a03.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (42)
  • .github/workflows/ci.yml
  • .github/workflows/npm-publish.yml
  • .gitignore
  • .gitmodules
  • LICENSE
  • LICENSE-APACHE
  • LICENSE-MIT
  • README.md
  • js/README.md
  • js/__tests__/encode.test.ts
  • js/__tests__/fixtures/test-data.ts
  • js/__tests__/integration.test.ts
  • js/__tests__/probe.test.ts
  • js/package.json
  • js/src/index.ts
  • package.json
  • wasm/.npmignore
  • wasm/CMakeLists.txt
  • wasm/Cargo.toml
  • wasm/README.md
  • wasm/package.json
  • wasm/pkg/open_ultrahdr.d.ts
  • wasm/rustfmt.toml
  • wasm/src/bindings.cpp
  • wasm/src/error.rs
  • wasm/src/gainmap/decode.rs
  • wasm/src/gainmap/encode.rs
  • wasm/src/gainmap/math.rs
  • wasm/src/gainmap/metadata.rs
  • wasm/src/gainmap/mod.rs
  • wasm/src/jpeg/mod.rs
  • wasm/src/jpeg/parser.rs
  • wasm/src/jpeg/writer.rs
  • wasm/src/jpeg/xmp.rs
  • wasm/src/jpeg_meta.cpp
  • wasm/src/jpeg_meta.h
  • wasm/src/lib.rs
  • wasm/src/types.rs
  • wasm/src/ultrahdr/decoder.rs
  • wasm/src/ultrahdr/encoder.rs
  • wasm/src/ultrahdr/mod.rs
  • wasm/third_party/libultrahdr
💤 Files with no reviewable changes (18)
  • wasm/src/jpeg/parser.rs
  • wasm/rustfmt.toml
  • wasm/src/gainmap/math.rs
  • wasm/src/types.rs
  • wasm/src/ultrahdr/mod.rs
  • wasm/src/ultrahdr/encoder.rs
  • wasm/src/jpeg/writer.rs
  • wasm/.npmignore
  • wasm/src/error.rs
  • wasm/Cargo.toml
  • wasm/src/gainmap/encode.rs
  • wasm/src/gainmap/metadata.rs
  • wasm/src/gainmap/mod.rs
  • wasm/src/jpeg/mod.rs
  • wasm/src/gainmap/decode.rs
  • wasm/src/lib.rs
  • wasm/src/ultrahdr/decoder.rs
  • wasm/src/jpeg/xmp.rs

Comment thread .github/workflows/ci.yml
Comment thread js/src/index.ts
Comment thread js/src/index.ts
Comment thread wasm/src/bindings.cpp
Comment on lines +57 to +64
struct UltraHdrEncodeOptions {
int baseQuality = kDefaultBaseQuality;
int gainMapQuality = kDefaultGainMapQuality;
float targetHdrCapacity = kDefaultTargetHdrCapacity;
bool includeIsoMetadata = true;
bool includeUltrahdrV1 = true;
int gainMapScale = 1;
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ultrahdr_api.h =="
fd -i 'ultrahdr_api.h' -x sed -n '1,260p' {}

echo
echo "== encoder-related symbols =="
rg -n -C2 'uhdr_enc_.*(metadata|format|gainmap|iso|v1)' --glob '*.{h,hpp,cc,cpp}'

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 867


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the full context of UltraHdrEncodeOptions struct (lines 57-64)
echo "== UltraHdrEncodeOptions struct (lines 57-64) =="
sed -n '50,70p' wasm/src/bindings.cpp

echo
echo "== encodeUltraHdr function signature and initial logic =="
sed -n '451,475p' wasm/src/bindings.cpp

echo
echo "== Full search for includeIsoMetadata references =="
rg 'includeIsoMetadata' -n --context 2

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 5112


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Full search for includeUltrahdrV1 references =="
rg 'includeUltrahdrV1' -n --context 2

echo
echo "== List all header files potentially containing ultrahdr API =="
fd '\.h$' | grep -i ultrahdr | head -20

echo
echo "== Search for files with ultrahdr_api or encoder API =="
fd '\.h$|\.hpp$' | xargs grep -l 'uhdr_enc' | head -10

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 2610


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find ultrahdr API definitions
echo "== Searching for all uhdr_enc_ function declarations =="
rg 'uhdr_enc_' -A 2 --glob '*.h' --glob '*.hpp' | head -100

echo
echo "== Searching for ISO or metadata related encoder functions =="
rg 'uhdr_enc.*\(iso\|metadata\|format\|v1\)' -i --glob '*.h' --glob '*.hpp'

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 133


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find libultrahdr or ultrahdr headers/sources =="
fd -i 'ultrahdr' --type f | head -30

echo
echo "== Check for vendored dependencies =="
find . -maxdepth 3 -name 'CMakeLists.txt' -o -name 'package.json' -o -name 'BUILD' | head -10

echo
echo "== Full encodeUltraHdr function (lines 451-529) =="
sed -n '451,529p' wasm/src/bindings.cpp

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 3868


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== All uhdr_enc_ calls in bindings.cpp =="
rg 'uhdr_enc_' wasm/src/bindings.cpp -n

echo
echo "== Check what happens with the options struct in encodeUltraHdr =="
sed -n '451,530p' wasm/src/bindings.cpp | grep -i 'option\|include\|metadata\|iso\|v1' || echo "No matches"

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1289


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Look for external library or submodule =="
cat .gitmodules 2>/dev/null || echo "No .gitmodules"

echo
echo "== Check if libultrahdr is referenced in build files =="
find . -maxdepth 2 -type f \( -name '*.cmake' -o -name 'CMakeLists.txt' -o -name '*.bazel' \) -exec grep -l 'ultrahdr' {} \;

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 330


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check what encoder setters exist in vendored libultrahdr =="
find wasm/third_party/libultrahdr -name '*.h' -o -name '*.hpp' | head -20

echo
echo "== Search for metadata/ISO/format setters in libultrahdr =="
rg 'uhdr_enc_set.*\(metadata\|iso\|format\|v1\)' -i wasm/third_party/libultrahdr/ --glob '*.h' || echo "No metadata/ISO/format setters found"

echo
echo "== List all uhdr_enc_set functions in libultrahdr headers =="
rg 'uhdr_enc_set' -o wasm/third_party/libultrahdr/ --glob '*.h' | sort -u

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 300


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check if libultrahdr submodule directory exists =="
ls -la wasm/third_party/libultrahdr/ 2>&1 | head -20

echo
echo "== Check for libultrahdr include paths in CMakeLists.txt =="
grep -n 'libultrahdr\|ultrahdr' wasm/CMakeLists.txt

echo
echo "== Search for header includes in bindings.cpp =="
head -50 wasm/src/bindings.cpp | grep '#include'

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1206


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find where uhdr headers come from =="
rg '#include.*uhdr' wasm/src/bindings.cpp -A 1 -B 1

echo
echo "== Check if libultrahdr is available as external dependency =="
grep -r 'libultrahdr\|ultrahdr' wasm/CMakeLists.txt -A 3 -B 3 | head -50

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 117


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find ultrahdr_api.h =="
find . -name 'ultrahdr_api.h' -type f

echo
echo "== Check if it's in third_party =="
find wasm/third_party -type f \( -name '*.h' -o -name '*.hpp' \) 2>/dev/null | head -20

echo
echo "== List all files in third_party =="
find wasm/third_party -type f 2>/dev/null | head -30

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 176


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for ultrahdr_api in entire repo =="
rg 'ultrahdr_api' . -l

echo
echo "== Check CMakeLists.txt more carefully for ultrahdr source =="
cat wasm/CMakeLists.txt

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1855


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check if there are any other references to includeIsoMetadata or includeUltrahdrV1 in C++ code =="
rg 'includeIsoMetadata|includeUltrahdrV1' wasm/src/ -n

echo
echo "== Verify the struct fields are never consumed in encodeUltraHdr =="
sed -n '451,529p' wasm/src/bindings.cpp | grep -i 'include' || echo "No 'include' references in encodeUltraHdr"

echo
echo "== Confirm all used options fields =="
sed -n '451,529p' wasm/src/bindings.cpp | grep 'options\.' || echo "Done"

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1017


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check if maybe the flags are used elsewhere in the codebase =="
rg 'includeIsoMetadata|includeUltrahdrV1' --glob '*.cpp' --glob '*.h' --glob '*.hpp' wasm/src/

echo
echo "== Verify defaultEncodeOptions implementation =="
sed -n '530,545p' wasm/src/bindings.cpp

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1108


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search libultrahdr API documentation online or check if setters exist =="
echo "Looking for what the actual libultrahdr encoder API supports..."

# Check if there are any comments or docs in bindings.cpp about why these fields exist
echo
echo "== Check if there are TODO or FIXME comments related to these fields =="
rg 'TODO|FIXME|XXX' wasm/src/bindings.cpp -B 2 -A 2 | grep -i 'metadata\|iso\|v1' || echo "No TODOs found"

echo
echo "== Check git history or comments for context =="
head -100 wasm/src/bindings.cpp | tail -50

Repository: adamsilverstein/lib-open-ultrahdr

Length of output: 1824


includeIsoMetadata and includeUltrahdrV1 are silently ignored

These options are part of the public API (exposed in embind bindings and documented in TypeScript), but encodeUltraHdr() never reads them. Only baseQuality, gainMapQuality, gainMapScale, and targetHdrCapacity are used. Callers expecting these flags to control metadata emission will find them have no effect. Either wire them through to the underlying encoder or reject invalid configurations explicitly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wasm/src/bindings.cpp` around lines 57 - 64, UltraHdrEncodeOptions contains
includeIsoMetadata and includeUltrahdrV1 but encodeUltraHdr() never uses them;
update encodeUltraHdr() to read these fields from UltraHdrEncodeOptions and
either (preferred) pass them into the underlying encoder/configuration API
(e.g., set equivalent flags on the encoder instance or encoder options used by
encodeUltraHdr()) so metadata emission respects includeIsoMetadata and
includeUltrahdrV1, or if the encoder cannot support them, validate and
return/throw an explicit error when those flags are set; locate
UltraHdrEncodeOptions and the encodeUltraHdr() implementation and wire these two
boolean flags into the encoder configuration or add validation logic to reject
unsupported combinations.

- ci.yml: pin Node via actions/setup-node@v4 in the wasm-build job. Without
  it the npm scripts ran on whatever Node ubuntu-latest happens to ship,
  which can drift silently.

- js/src/index.ts: guard the WASM cache against stale in-flight inits.
  Previously, calling setLocation()/setWasmUrl() while a getWasm() was
  pending could let the older init publish to wasmInstance under the new
  config. Track an init generation; only the latest generation is allowed
  to set wasmInstance or clear initPromise.

- js/src/index.ts: move getWasm() inside probeUltraHdr's try/catch so
  module-load failures degrade to the documented invalid-result fallback
  rather than escaping.

- wasm/src/bindings.cpp + js/src/types.ts: document includeIsoMetadata
  and includeUltrahdrV1 as accepted-but-no-op for now. libultrahdr always
  emits both metadata blocks and exposes no public toggle; the fields stay
  in the API for stability with the prior Rust crate.
@adamsilverstein

Copy link
Copy Markdown
Owner Author

Thanks for the review. Pushed 3293169 addressing all four findings:

  • CI: Pin Node in wasm-build — added actions/setup-node@v4 step with the existing NODE_VERSION env var, before the build step.
  • resetCache() race — added an initGeneration counter; getWasm() captures it on entry and only writes to wasmInstance / clears initPromise if its generation is still current. A late-resolving init under a stale config now drops its instance and re-enters getWasm() for a fresh attempt.
  • probeUltraHdr() non-throwing — moved getWasm() inside the try so module-load failures also degrade to the documented invalid-result fallback.
  • includeIsoMetadata / includeUltrahdrV1 no-ops — kept in the API for stability with the prior Rust crate, but documented as accepted-but-currently-no-op in both the C++ struct and the TypeScript type, since libultrahdr's encoder unconditionally emits both metadata blocks and exposes no public toggle. Will revisit if/when upstream adds granular controls.

Re-ran locally: all 47 JS tests pass, lint + typecheck clean.

@adamsilverstein adamsilverstein merged commit b1016c3 into main May 8, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate WASM backend from Rust reimplementation to Emscripten/embind build of libultrahdr

1 participant