wasm: enable JPEG/PNG/TIFF codecs#2044
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe wasm workflow refreshes the OpenCV cache, enables JPEG/TIFF/PNG codecs, and aligns exception handling flags across wasm builds. The smoke test validates encode/decode round trips for all three formats. ChangesWasm codec support
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant WasmWorkflow
participant OpenCVBuild
participant OpenCvSharpExternBuild
participant WasmSmokeTest
WasmWorkflow->>OpenCVBuild: Build OpenCV wasm with codecs and matching exception flags
WasmWorkflow->>OpenCvSharpExternBuild: Build extern wasm with matching exception flags
OpenCVBuild-->>WasmSmokeTest: Provide codec-enabled runtime
WasmSmokeTest->>OpenCVBuild: Encode and decode test image
OpenCVBuild-->>WasmSmokeTest: Return encoded data and decoded Mat
WasmSmokeTest->>WasmSmokeTest: Validate non-empty output and matching dimensions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/wasm.yml:
- Around line 128-130: Add an OpenCV codec validation step in the workflow
before actions/cache/save, asserting JPEG, PNG, and TIFF support using the
existing BuildConfigurationTest coverage or an equivalent decode smoke test.
Ensure the validation also runs on cache-hit paths before cached artifacts are
reused, preventing partial codec builds from passing silently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2301aee0-e7f1-4bb6-84f5-f265ad0a75b1
📒 Files selected for processing (1)
.github/workflows/wasm.yml
| -DWITH_JPEG=ON \ | ||
| -DWITH_TIFF=ON \ | ||
| -DWITH_PNG=ON \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
nl -ba .github/workflows/wasm.yml | sed -n '90,180p'
echo
echo "== search for codec/build-info checks in workflows =="
rg -n "getBuildInformation|BUILD_INFORMATION|imread|decode|JPEG|PNG|TIFF|WITH_JPEG|WITH_PNG|WITH_TIFF|opencv_version" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
echo
echo "== locate OpenCV version pin =="
rg -n "OPENCV_VERSION|opencv/opencv|opencv" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: shimat/opencvsharp
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/wasm.yml (120-170) =="
nl -ba .github/workflows/wasm.yml | sed -n '120,170p'
echo
echo "== references to the cache step in wasm workflow =="
rg -n "cache|actions/cache|save cache|restore cache|ccache|sccache" .github/workflows/wasm.yml
echo
echo "== references to OpenCV build info / smoke tests in repo =="
rg -n "getBuildInformation|imread|imdecode|decode.*smoke|jpeg|png|tiff" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: shimat/opencvsharp
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/wasm.yml (120-170) =="
sed -n '120,170p' .github/workflows/wasm.yml | cat -n
echo
echo "== cache / build / smoke-test references in wasm workflow =="
rg -n "cache|actions/cache|ccache|sccache|getBuildInformation|BUILD_INFORMATION|imread|imdecode|decode|JPEG|PNG|TIFF|WITH_JPEG|WITH_PNG|WITH_TIFF|opencv_version" .github/workflows/wasm.yml
echo
echo "== any codec/build-info checks elsewhere in repo =="
rg -n "getBuildInformation|BUILD_INFORMATION|imread|imdecode|decode.*smoke|WITH_JPEG|WITH_PNG|WITH_TIFF|JPEG|PNG|TIFF" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: shimat/opencvsharp
Length of output: 27231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/wasm.yml (1-120) =="
sed -n '1,120p' .github/workflows/wasm.yml | cat -n
echo
echo "== BuildConfigurationTest.cs relevant part =="
sed -n '1,220p' test/OpenCvSharp.Tests/core/BuildConfigurationTest.cs | cat -nRepository: shimat/opencvsharp
Length of output: 17013
Add a codec smoke test before caching OpenCV
test/OpenCvSharp.Tests/core/BuildConfigurationTest.cs already checks JPEG/PNG/TIFF via Cv2.GetBuildInformation(), but this workflow never runs it. Add a build-info assertion or JPEG/PNG/TIFF decode smoke test before actions/cache/save, and on cache-hit paths, so a partial OpenCV build can’t be reused silently.
🤖 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 @.github/workflows/wasm.yml around lines 128 - 130, Add an OpenCV codec
validation step in the workflow before actions/cache/save, asserting JPEG, PNG,
and TIFF support using the existing BuildConfigurationTest coverage or an
equivalent decode smoke test. Ensure the validation also runs on cache-hit paths
before cached artifacts are reused, preventing partial codec builds from passing
silently.
These were disabled since the workflow's original commit, copied from opencv.js's own build recipe. That recipe assumes image decoding happens via the browser's Canvas/Image API, bypassing imgcodecs entirely, but OpenCvSharp's wasm target has no such path: Cv2.ImDecode / Mat.FromImageData are the only decode entry points, so disabling these codecs made them silently return an empty Mat instead of throwing. Bump the OpenCV build cache key so CI doesn't reuse a stale cache built without these codecs. Fixes #2043
shimat
force-pushed
the
feature/2043-wasm-jpeg-png-tiff
branch
from
5a8818e to
04fcbe8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/OpenCvSharp.Tests.Wasm/App.razor (1)
29-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid round-trip validation.
The empty-buffer and wrong-size checks correctly guard against codecs that build but silently no-op under wasm, matching the PR objectives. The
usingondecodedproperly disposes the decodedMat.One optional note:
ImEncodereturns aboolsuccess flag (per the upstream contract inCv2_imgcodecs.cs:486-498) that is currently ignored. The downstreamImDecodesize check provides a safety net, so this is not a functional gap, but checking the return value directly would give a clearer failure message on encode failure.♻️ Optional: check ImEncode return value for clearer diagnostics
- Cv2.ImEncode(ext, mat, out var buf); - if (buf.Length == 0) - throw new InvalidOperationException($"ImEncode({ext}) produced an empty buffer"); + if (!Cv2.ImEncode(ext, mat, out var buf) || buf.Length == 0) + throw new InvalidOperationException($"ImEncode({ext}) produced an empty buffer");🤖 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 `@test/OpenCvSharp.Tests.Wasm/App.razor` around lines 29 - 38, Optionally update the round-trip loop in App.razor to capture and validate the boolean result returned by Cv2.ImEncode before inspecting buf. Preserve the existing empty-buffer and decode validation, and throw a clear extension-specific error when encoding reports failure.
🤖 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.
Nitpick comments:
In `@test/OpenCvSharp.Tests.Wasm/App.razor`:
- Around line 29-38: Optionally update the round-trip loop in App.razor to
capture and validate the boolean result returned by Cv2.ImEncode before
inspecting buf. Preserve the existing empty-buffer and decode validation, and
throw a clear extension-specific error when encoding reports failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e52ef63-4efc-485b-86a9-21307c650dd2
📒 Files selected for processing (1)
test/OpenCvSharp.Tests.Wasm/App.razor
Addresses CodeRabbit feedback on PR #2044.
…default libjpeg-turbo (and libpng) use setjmp/longjmp for error handling. Building OpenCV/OpenCvSharpExtern with plain emcc left those calls lowered with the legacy JS-based invoke_ wrapper mechanism, while the .NET wasm SDK links its own runtime using the newer wasm exception-handling proposal by default (WasmEnableExceptionHandling=true). Mixing the two in the final link fails with "invoke_ functions exported but exceptions and longjmp are both disabled". Compile both OpenCV and OpenCvSharpExtern with -fwasm-exceptions -sSUPPORT_LONGJMP=wasm to match.
1 participant
Summary
Enables
-DWITH_JPEG=ON -DWITH_TIFF=ON -DWITH_PNG=ONfor the wasm build (previously allOFF), and bumps the OpenCV build cache key so CI rebuilds with the new flags instead of reusing a stale cache.Background
These codecs were disabled in
.github/workflows/wasm.ymlsince the workflow's very first commit, apparently copied from OpenCV's ownopencv.jsbuild recipe (opencv/platforms/js/build_js.py), which disables the same set of codecs. That makes sense foropencv.js, since it decodes images via the browser's Canvas/Image API and never touchesimgcodecsat all.OpenCvSharp's wasm target has no equivalent JS-side decode path, though:
Cv2.ImDecode/Mat.FromImageDataare the only decode entry points exposed to .NET code. With the codecs disabled, these silently returned an emptyMatinstead of throwing, which was hard to diagnose (confirmed in #2043).find_package(JPEG/PNG/TIFF)fails to find a system library under the Emscripten cross-compile toolchain, soWITH_*=ONalone is enough to fall back to the bundled3rdparty/libjpeg-turbo/libpng/libtiffsources (opencv/cmake/OpenCVFindLibsGrfmt.cmake). libjpeg-turbo's SIMD detection also degrades gracefully to a non-SIMD build for unrecognized CPU types rather than failing.Fixes #2043
Test plan
Cv2.GetBuildInformation()on the resulting wasm build shows JPEG/PNG/TIFF as available under Media I/OCv2.ImDecode/Mat.FromImageDatasuccessfully decode a JPEG/PNG (not TIFF, not empty) byte array on wasmSummary by CodeRabbit