Skip to content

VideoCapture timeout set/get#20541

Merged
alalek merged 4 commits into3.4from
unknown repository
Aug 12, 2021
Merged

VideoCapture timeout set/get#20541
alalek merged 4 commits into3.4from
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Aug 11, 2021 •
edited by ghost
Loading

The original PR is amazing, a fixed timeout is not that convenient however, so this small patch adds get and set option for timeout
#6053

@ghost ghost marked this pull request as draft August 12, 2021 00:41
asmorkalov
asmorkalov reviewed Aug 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the patch! Looks good in general.

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

Thanks a lot for the patch! Looks good in general.

You are welcome 🙏
Just needs some work I think for some reason build fails on the cloud server

@ghost ghost marked this pull request as ready for review August 12, 2021 12:13
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

hmm not sure why the checks fail

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 12, 2021 •
edited
Loading

AFAIK, cap_ffmpeg_impl.hpp doesn't have access to enums from videoio.hpp

Please use enum values aligned with the "master" branch (=53 is free there), so we the same values between OpenCV versions. (all is done here)

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

AFAIK, cap_ffmpeg_impl.hpp doesn't have access to enums from videoio.hpp

Please use enum values aligned with the "master" branch (=53 is free there), so we the same values between OpenCV versions. (all is done here)

ah ok

@ghost ghost marked this pull request as draft August 12, 2021 12:29
@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 12, 2021

If you don't need this feature for OpenCV 3.x release series, then it makes sense to retarget this PR to "master" (4.x) branch. There is no such problem.

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

If you don't need this feature for OpenCV 3.x release series, then it makes sense to retarget this PR to "master" (4.x) branch. There is no such problem.

ah no worries, I need it for both, will do another pull after fixing the small issue

@ghost ghost marked this pull request as ready for review August 12, 2021 13:21
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

Ok this should fix it, was working on master, 3.4 had FFMPEG enums in different place

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

Checks passed

@ghost ghost requested a review from asmorkalov August 12, 2021 15:09
alalek
alalek reviewed Aug 12, 2021
View reviewed changes
modules/videoio/src/cap_ffmpeg_api.hpp Outdated
CV_FFMPEG_CAP_PROP_ORIENTATION_AUTO=49
CV_FFMPEG_CAP_PROP_ORIENTATION_AUTO=49,
CV_FFMPEG_CAP_PROP_OPEN_TIMEOUT_MSEC=50,
CV_FFMPEG_CAP_PROP_READ_TIMEOUT_MSEC=51
Copy link
Copy Markdown
Member

@alalek alalek Aug 12, 2021

Choose a reason for hiding this comment

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

Need to restore defined enum values in videoio.hpp. It is a public API header, from which users know how to use this feature.

Also please use 53, 54 as values to align with master (for both "enums").

Copy link
Copy Markdown
Author

@ghost ghost Aug 12, 2021

Choose a reason for hiding this comment

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

ok will do now

Copy link
Copy Markdown
Author

@ghost ghost Aug 12, 2021

Choose a reason for hiding this comment

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

Done

alalek
alalek approved these changes Aug 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Well done! Thank you for contribution 👍

@alalek alalek merged commit 4300bb2 into opencv:3.4 Aug 12, 2021
@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

Btw I should make another PR for master right?

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 12, 2021

It would be nice, because there are several differences.
On master branch we have VideoCaptureParameters which can pass "open timeout" properly. On 3.4 it doesn't work as expected ("set" property is always called after "open").

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 12, 2021

It would be nice, because there are several differences.

On master branch we have VideoCaptureParameters which can pass "open timeout" properly. On 3.4 it doesn't work as expected ("set" property is always called after "open").

I see, will do a PR asap

This was referenced Aug 13, 2021
Merged
Merged
@alalek alalek mentioned this pull request Oct 15, 2021
Merged
@ribeirompl-soldersmith ribeirompl-soldersmith mentioned this pull request Feb 2, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov Awaiting requested review from asmorkalov

Assignees

@alalek alalek

Labels

category: videoio feature

Projects

None yet

Milestone

3.4.16

Development

Successfully merging this pull request may close these issues.

2 participants

@alalek @asmorkalov