fix: face_detector_yn needs to accept NO_OBJDETECT#1830
fix: face_detector_yn needs to accept NO_OBJDETECT#1830ctacke wants to merge 1 commit intoshimat:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds preprocessor guards around FaceDetectorYN API implementations and declarations, providing no-op fallback implementations when OBJDETECT support is unavailable in OpenCV, replacing previous runtime/compile-time error behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 the current code and only fix it if needed.
Inline comments:
In `@src/OpenCvSharpExtern/face_detector_yn.cpp`:
- Around line 43-67: The stub implementations under the `#else` branch incorrectly
reference OpenCV types that only exist when HAVE_OPENCV_OBJDETECT is defined;
either remove these stub definitions so the .cpp matches the header guards, or
replace their signatures to use opaque types (e.g., void* or
conditionally-compiled typedefs) to avoid referring to cv::FaceDetectorYN and
cv::Ptr<cv::FaceDetectorYN> when HAVE_OPENCV_OBJDETECT is not set. Update the
implementations for cveFaceDetectorYNCreate, cveFaceDetectorYNRelease, and
cveFaceDetectorYNDetect accordingly and ensure the `#ifdef/HAVE_OPENCV_OBJDETECT`
guards in the .cpp mirror the header declarations exactly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/OpenCvSharpExtern/face_detector_yn.cppsrc/OpenCvSharpExtern/face_detector_yn.h
| #else | ||
|
|
||
| cv::FaceDetectorYN* cveFaceDetectorYNCreate( | ||
| cv::String* model, | ||
| cv::String* config, | ||
| CvSize* inputSize, | ||
| float scoreThreshold, | ||
| float nmsThreshold, | ||
| int topK, | ||
| int backendId, | ||
| int targetId, | ||
| cv::Ptr<cv::FaceDetectorYN>** sharedPtr) | ||
| { | ||
| return nullptr; | ||
| } | ||
|
|
||
| void cveFaceDetectorYNRelease(cv::Ptr<cv::FaceDetectorYN>** faceDetector) | ||
| { | ||
| } | ||
|
|
||
| int cveFaceDetectorYNDetect(cv::FaceDetectorYN* faceDetector, cv::_InputArray* image, cv::_OutputArray* faces) | ||
| { | ||
| return 0; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Stub implementations use undefined types when HAVE_OPENCV_OBJDETECT is not defined.
The #else branch (stub implementations) references types like cv::FaceDetectorYN*, cv::Ptr<cv::FaceDetectorYN>** which are only defined when opencv2/objdetect/face.hpp is included. This branch executes precisely when that header is NOT included, causing compilation failures.
Additionally, the header file (lines 17-32) only declares these functions when HAVE_OPENCV_OBJDETECT is defined, creating an inconsistency where the .cpp defines functions that aren't declared in the header.
🔧 Proposed fix: Remove stub implementations and align with header guards
The header doesn't declare these functions when HAVE_OPENCV_OBJDETECT is not defined, so the .cpp shouldn't define them either:
`#endif`
-
-#else
-
-cv::FaceDetectorYN* cveFaceDetectorYNCreate(
- cv::String* model,
- cv::String* config,
- CvSize* inputSize,
- float scoreThreshold,
- float nmsThreshold,
- int topK,
- int backendId,
- int targetId,
- cv::Ptr<cv::FaceDetectorYN>** sharedPtr)
-{
- return nullptr;
-}
-
-void cveFaceDetectorYNRelease(cv::Ptr<cv::FaceDetectorYN>** faceDetector)
-{
-}
-
-int cveFaceDetectorYNDetect(cv::FaceDetectorYN* faceDetector, cv::_InputArray* image, cv::_OutputArray* faces)
-{
- return 0;
-}
-#endif
`#endif`Alternatively, if stub implementations are required for linkage, use opaque void* pointers instead of the concrete OpenCV types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/OpenCvSharpExtern/face_detector_yn.cpp` around lines 43 - 67, The stub
implementations under the `#else` branch incorrectly reference OpenCV types that
only exist when HAVE_OPENCV_OBJDETECT is defined; either remove these stub
definitions so the .cpp matches the header guards, or replace their signatures
to use opaque types (e.g., void* or conditionally-compiled typedefs) to avoid
referring to cv::FaceDetectorYN and cv::Ptr<cv::FaceDetectorYN> when
HAVE_OPENCV_OBJDETECT is not set. Update the implementations for
cveFaceDetectorYNCreate, cveFaceDetectorYNRelease, and cveFaceDetectorYNDetect
accordingly and ensure the `#ifdef/HAVE_OPENCV_OBJDETECT` guards in the .cpp
mirror the header declarations exactly.
|
Thank you for identifying this and submitting a fix! I've opened a separate PR that addresses the same root cause while aligning the implementation with the conventions used in the rest of the codebase. Closing this in favor of that PR. |
2 participants
If you're trying to build a slimmed-down version and have NO_OBJDETECT, the new face_detector_yn regressed the build
Summary by CodeRabbit