Skip to content

Set of build fixes for ## 3580, 3574.#3584

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
asmorkalov:as/build_fixes
Oct 27, 2023
Merged

Set of build fixes for ## 3580, 3574.#3584
asmorkalov merged 1 commit intoopencv:4.xfrom
asmorkalov:as/build_fixes

Conversation

@asmorkalov
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov commented Oct 26, 2023 •
edited
Loading

Fixes #3580
Fixes #3574.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@asmorkalov asmorkalov requested a review from mshabunin October 26, 2023 10:26
@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Oct 26, 2023

Windows build on buildbot has failed:

C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1216): error C2131: expression did not evaluate to a constant [C:\build\precommit-contrib_windows64\build\modules\face\opencv_face.vcxproj]
  C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1216): note: failure was caused by non-constant arguments or reference to a non-constant symbol
  C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1216): note: see usage of 'l'
C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1217): error C3863: array type 'double [l]' is not assignable [C:\build\precommit-contrib_windows64\build\modules\face\opencv_face.vcxproj]
C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1218): error C3863: array type 'double [l]' is not assignable [C:\build\precommit-contrib_windows64\build\modules\face\opencv_face.vcxproj]
C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1314): error C2665: 'cv::max': none of the 6 overloads could convert all the argument types [C:\build\precommit-contrib_windows64\build\modules\face\opencv_face.vcxproj]
  C:\build\precommit-contrib_windows64\4.x\opencv\modules\core\include\opencv2/core/mat.hpp(3766): note: could be 'cv::MatExpr cv::max(double,const cv::Mat &)' (compiling source file C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp)
  C:\build\precommit-contrib_windows64\4.x\opencv\modules\core\include\opencv2/core/mat.hpp(3765): note: or       'cv::MatExpr cv::max(const cv::Mat &,double)' (compiling source file C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp)
  C:\build\precommit-contrib_windows64\4.x\opencv\modules\core\include\opencv2/core/mat.hpp(3764): note: or       'cv::MatExpr cv::max(const cv::Mat &,const cv::Mat &)' (compiling source file C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp)
  C:\build\precommit-contrib_windows64\4.x\opencv_contrib\modules\face\src\facemarkLBF.cpp(1314): note: while trying to match the argument list '(double, double [l])'

mshabunin
mshabunin reviewed Oct 26, 2023
View reviewed changes
modules/face/src/facemarkLBF.cpp Outdated
std::vector<double> beta(l);
std::vector<double> QD(l);
std::vector<double> lambda(l);
std::vector<double> upper_bound(l);
Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Oct 26, 2023

Choose a reason for hiding this comment

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

Mentioned commit in scikit-learn also changes #define GETI(i) (i) and adds initialization of lambda and upper_bound arrays: https://github.com/scikit-learn/scikit-learn/blob/dcebbc4c5c97f886a57b0684a5e943f94db1bd58/sklearn/svm/src/liblinear/linear.cpp#L1073-L1089
And assuming we have L2R_L1LOSS_SVR_DUAL case here, single-element arrays make sense, but GETI macro is wrong.

Alternatively there is another source of original code where GETI(i) is 0 and arrays of size 1 works well: https://github.com/cjlin1/liblinear/blob/8dc206b782e07676dc0d00678bedd295ce85acf3/linear.cpp#L1127

@asmorkalov asmorkalov changed the title Set of build fixes for ## 3581, 3580, 3574. Set of build fixes for ## 3580, 3574. Oct 26, 2023
@asmorkalov
Copy link
Copy Markdown
Contributor Author

asmorkalov commented Oct 26, 2023

@mshabunin I reverted changes in facemarkLBF.cpp. Let's fix it with the next PR.

@asmorkalov asmorkalov merged commit d51add3 into opencv:4.x Oct 27, 2023
@asmorkalov asmorkalov mentioned this pull request Nov 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshabunin mshabunin mshabunin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

compile failure due to <Type> syntax Polymorphic type reference error during build

2 participants

@asmorkalov @mshabunin