fix(opencv): Handle delayed cv.ready and add timeout #1489
Conversation
MarkRedeman
requested review from
ActiveChooN,
dwesolow and
romanowska
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR addresses sporadic OpenCV loading failures by implementing a robust loading mechanism with polling and timeout handling. The fix targets cases where cv.ready doesn't resolve immediately, particularly when smart tools are used outside the Geti environment.
Key Changes:
- Adds polling mechanism to wait for
cv.readyavailability with 100ms intervals - Implements 30-second timeout for OpenCV loading operations
- Prevents concurrent loading attempts by caching the loading promise
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| loadingPromise = Promise.race([ | ||
| (async () => { | ||
| try { | ||
| const cv: OpenCVTypes = await import('../opencv/4.9.0/opencv.js'); | ||
|
|
||
| if ('ready' in cv) await cv.ready; | ||
| // Wait for cv.ready with polling and timeout | ||
| await waitForOpenCVReady(cv); | ||
|
|
||
| opencv = cv; | ||
| if (!cv.Mat) { | ||
| throw new Error('OpenCV missing essential methods'); | ||
| } | ||
| opencv = cv; | ||
| return opencv; | ||
| } catch (error) { | ||
| loadingPromise = null; | ||
| throw error; | ||
| } | ||
| })(), | ||
| new Promise<never>((_, reject) => | ||
| setTimeout( | ||
| () => reject(new Error(`OpenCV loading timeout (${OPENCV_LOAD_TIMEOUT_MS}ms)`)), | ||
| OPENCV_LOAD_TIMEOUT_MS | ||
| ) | ||
| ), | ||
| ]); |
There was a problem hiding this comment.
The timeout Promise in the race is redundant since waitForOpenCVReady already implements the same timeout. This creates two separate 30-second timers that could reject with different error messages, making debugging confusing. Remove the outer Promise.race wrapper and rely on the timeout in waitForOpenCVReady.
| // Check if cv.ready is already resolved (some builds may have it pre-resolved) | ||
| if (cv && cv.onload && typeof cv.onload === 'function') { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The check for cv.onload is ineffective for determining if OpenCV is ready. The presence of an onload function doesn't guarantee initialization is complete. This could lead to returning before OpenCV is actually ready to use, causing subsequent operations to fail.
| // Check if cv.ready is already resolved (some builds may have it pre-resolved) | |
| if (cv && cv.onload && typeof cv.onload === 'function') { | |
| return; | |
| } | |
| // (Removed ineffective check for cv.onload) |
This fixes an issue where opencv was not being able to load. This likely was because `cv.ready` would not resolve.
MarkRedeman
force-pushed
the
mark/improve-loading-behaviour-of-opencv
branch
from
4d9756e to
28c3a2d
Compare
1 participant
📝 Description
This fixes an issue where opencv was not being able to load. This likely was because
cv.readywould not resolve.The bug seems to be very sporadic and not happen inside of Geti, likely due to state updates retriggering loading of opencv. I noticed this error more when using the smart tools outside of Geti.
✨ Type of Change
Select the type of change your PR introduces:
🧪 Testing Scenarios
Describe how the changes were tested and how reviewers can test them too: