Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing cap to VideoCapture constructor #157

Merged
merged 7 commits into from
Sep 13, 2024

Conversation

la-we
Copy link

@la-we la-we commented Aug 27, 2024

Currently working on a project where i need functionality of passing explicit CAPs to VideoCapture, figured i contribute a little.

@UrielCh
Copy link
Owner

UrielCh commented Aug 27, 2024

Please try to match parameters names from openCV:

VideoCapture(const String &filename, int apiPreference=[CAP_ANY])

Opens a video file or a capturing device or an IP video stream for video capturing with API Preference.

the parametere is named apiPreference, not cap

@@ -27,11 +27,19 @@ NAN_METHOD(VideoCapture::New) {
FF::TryCatch tryCatch("VideoCapture::New");
FF_ASSERT_CONSTRUCT_CALL();
VideoCapture* self = new VideoCapture();

uint32_t apiPreference = cv::CAP_ANY;
if (info.Length() == 2) {
Copy link
Owner

Choose a reason for hiding this comment

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

the latest vertion allow a 3th parameter, so the test should be >=2

and is you want a more cleaner code throw an not implemented yet if more than 2 parameters are provided.

the 3th parameter is only available in openCV ~~ 4.5+

Copy link
Author

Choose a reason for hiding this comment

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

New changes should cover all constructor variant a user might try to use and throw if it is not yet implemented

test/pnpm-lock.yaml Outdated Show resolved Hide resolved
@UrielCh
Copy link
Owner

UrielCh commented Aug 28, 2024

@la-we
Copy link
Author

la-we commented Aug 28, 2024

Tests should pass now on all OSes

@UrielCh
Copy link
Owner

UrielCh commented Aug 31, 2024

the test still failed on macos.

@UrielCh
Copy link
Owner

UrielCh commented Sep 13, 2024

LGTM

@UrielCh UrielCh merged commit da02bd9 into UrielCh:master Sep 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants