refactor: remove OpenCvSharp dependency and replace with native implementations#1642
refactor: remove OpenCvSharp dependency and replace with native implementations#1642
Conversation
…tive implementations Replace OpenCV-based blur filters (Blur, GaussianBlur, MedianBlur) with SkiaSharp shader implementations, replace OpenCV YUV conversion in MFReader with custom YuvConversion utility, and remove CubeFile's OpenCV dependency by using direct pixel manipulation. Remove OpenCvSharp packages from Directory.Packages.props, CI workflows, and third-party notices.
OpenCv namespace under FilterEffects (Blur, GaussianBlur, MedianBlur) is no longer needed after the OpenCvSharp dependency removal. Remove the effect files and their node/library registrations.
There was a problem hiding this comment.
Pull request overview
This PR removes the OpenCvSharp dependency across the project, replacing prior OpenCV-based image processing and caching paths with SkiaSharp and native pixel conversion utilities.
Changes:
- Removed OpenCvSharp NuGet packages, CI native dependency download steps, and OpenCV third-party notices.
- Replaced OpenCV-based frame read/conversion and frame-cache storage with a new native
YuvConversionutility and byte[]-backed caching. - Removed OpenCV filter effect implementations and their library/node registrations.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| THIRD_PARTY_NOTICES.md | Removes OpenCvSharp/OpenCV license blocks from notices. |
| src/Beutl/Services/LibraryRegistrar.cs | Removes OpenCV filter effect group registration. |
| src/Beutl/Models/FrameCacheManager.CacheEntry.cs | Replaces Mat storage with byte[] cache data + Skia resize + YUV conversion. |
| src/Beutl.NodeGraph/Nodes/NodesRegistrar.cs | Removes OpenCV nodes from node registry. |
| src/Beutl.Extensions.MediaFoundation/Decoding/MFReader.cs | Replaces OpenCV color conversion with native YUY2→BGRA conversion via YuvConversion. |
| src/Beutl.Engine/Media/YuvConversion.cs | Adds native BGRA↔I420 and YUY2→BGRA conversion routines. |
| src/Beutl.Engine/Graphics/Image.cs | Removes Image.ToMat() extension and OpenCvSharp usage. |
| src/Beutl.Engine/Graphics/FilterEffects/OpenCv/MedianBlur.cs | Deletes OpenCV-based effect implementation. |
| src/Beutl.Engine/Graphics/FilterEffects/OpenCv/GaussianBlur.cs | Deletes OpenCV-based effect implementation. |
| src/Beutl.Engine/Graphics/FilterEffects/OpenCv/Blur.cs | Deletes OpenCV-based effect implementation. |
| src/Beutl.Engine/Graphics/CubeFile.cs | Removes OpenCvSharp Vec3b from LUT interpolation API. |
| src/Beutl.Engine/Beutl.Engine.csproj | Drops OpenCvSharp package references. |
| Directory.Packages.props | Removes OpenCvSharp package versions. |
| .github/workflows/daily-build.yml | Removes Linux libOpenCvSharpExtern.so download step. |
| .github/workflows/build-executable.yml | Removes Linux libOpenCvSharpExtern.so download step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use ceiling division for UV plane dimensions so that odd-sized inputs are converted correctly instead of silently dropping the last row/column.
Switch cache buffers from new byte[] to ArrayPool to reduce GC pressure. Use SKFilterMode.Linear for resize instead of Default for better quality.
…me cache YuvConversion and FrameCacheManager assumed tightly packed pixel data (RowBytes == width * bytesPerPixel), which could corrupt images if SKBitmap has row padding. Add explicit stride parameters to BgraToI420, I420ToBgra, and Yuy2ToBgra, and copy row-by-row when strides differ.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| Buffer.MemoryCopy( | ||
| srcBase + (long)y * srcStride, | ||
| dstBase + (long)y * dstStride, |
Check failure
Code scanning / CodeQL
Unvalidated local pointer arithmetic Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 3 hours ago
General approach: after renting the buffer from ArrayPool<byte>.Shared.Rent(bgraSize), validate that bgraData.Length is at least bgraSize before using it in unsafe code and pointer arithmetic. If the length is too small, either throw a descriptive exception or fall back to a safe allocation path. This ensures that regardless of the concrete ArrayPool<byte> implementation, pointer arithmetic on dstBase remains within the bounds of the underlying array.
Best specific fix for this snippet: immediately after byte[] bgraData = ArrayPool<byte>.Shared.Rent(bgraSize); on line 101, add a check:
if (bgraData.Length < bgraSize)
{
throw new InvalidOperationException("Rented buffer is smaller than requested size.");
}This is minimal, preserves existing behavior under normal conditions (where the check always passes), and satisfies the requirement of validating the result of the virtual Rent call before using it in unsafe pointer arithmetic in the else branch. No other logic needs to change, because the code already restricts each per-row copy to dstStride bytes, and the total requested size bgraSize is used consistently.
File/lines to change: src/Beutl/Models/FrameCacheManager.CacheEntry.cs, in the method shown, right after line 101 where bgraData is rented. Required additions: a using System; is already implicit in most C# projects; if not already present in this file or via global usings, throwing InvalidOperationException may require using System;, but we will not modify imports unless needed. Since InvalidOperationException is in System, and the file already uses other System.* namespaces, it is safe to assume System is available or can be referenced without additional using; we therefore just add the check.
| @@ -99,6 +99,10 @@ | ||
| int dstStride = w * 4; | ||
| int bgraSize = dstStride * h; | ||
| byte[] bgraData = ArrayPool<byte>.Shared.Rent(bgraSize); | ||
| if (bgraData.Length < bgraSize) | ||
| { | ||
| throw new InvalidOperationException("Rented buffer is smaller than requested size."); | ||
| } | ||
| int srcStride = current.RowBytes; | ||
|
|
||
| if (srcStride == dstStride) |
|
No TODO comments were found. |
Minimum allowed line rate is |
Description
Remove the OpenCvSharp NuGet dependency entirely from the project, replacing all usages with SkiaSharp-based or native implementations:
Mat/Cv2.CvtColorinMFReaderwith a newYuvConversionutility class that performs YUY2→BGRA conversion using direct pixel manipulation.Image.ToMat()dependency; LUT application now uses directSKBitmappixel access.SKBitmapencode/decode for memory-efficient frame caching.OpenCvfilter effects namespace (Blur,GaussianBlur,MedianBlur) and their node/library registrations, as these are superseded by existing SkiaSharp-based effects.libOpenCvSharpExtern.sodownload step frombuild-executable.ymlanddaily-build.yml.OpenCvSharp4,OpenCvSharp4.runtime.osx.10.15-x64, andOpenCvSharp4.runtime.winfromDirectory.Packages.props.THIRD_PARTY_NOTICES.md.Breaking changes
Beutl.Graphics.FilterEffects.OpenCv.Blur,GaussianBlur, andMedianBlurfilter effects have been removed. Users of these effects in saved projects will need to switch to the built-in SkiaSharp blur effects.Image.ToMat()extension method has been removed.Fixed issues
N/A