Skip to content

Add live timestamps to MSMF Video API#18966

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
Staticity:add_live_timestamps_to_msmf
Dec 3, 2020
Merged

Add live timestamps to MSMF Video API#18966
opencv-pushbot merged 1 commit intoopencv:3.4from
Staticity:add_live_timestamps_to_msmf

Conversation

@Staticity
Copy link
Copy Markdown

@Staticity Staticity commented Nov 30, 2020 •
edited
Loading

When using cv::VideoCapture by index on Windows with MS Media Foundation (cv::CAP_MSMF), the field cv::CAP_PROP_POS_MSEC always returns 0.

This PR makes an unused timestamp parameter from the IMFSourceReaderCallback::OnReadSample(...) interface available through cv::VideoCapture::get(cv::CAP_PROP_POS_MSEC).

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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 November 30, 2020 06:58
@asmorkalov asmorkalov requested review from asmorkalov and removed request for mshabunin November 30, 2020 07:35
@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Nov 30, 2020
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 30, 2020

@Staticity Thanks for the contribution! It looks like we do not have test for cv::CAP_PROR_FRAME_MSEC option at al. I added general test for all back-ends in https://github.com/opencv/opencv/pull/18968. Let's merge the test first and the we cane check your solution and merge it too.

@asmorkalov asmorkalov mentioned this pull request Nov 30, 2020
Merged
6 tasks
asmorkalov
asmorkalov requested changes Dec 1, 2020
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.

@Staticity My patch with CAP_PROP_POS_MSEC has been merged. Please rebase your brunch on top of current 3.4 and remove Media Foundation check in test code to active test for MSMF back-end too.

@Staticity Staticity force-pushed the add_live_timestamps_to_msmf branch from 008b1d0 to 9aab7c8 Compare December 1, 2020 09:24
@Staticity
Copy link
Copy Markdown
Author

Staticity commented Dec 1, 2020

@asmorkalov Ignore the faulty latest push(es). Sounds good, I'm rebased on 3.4 and building the tests now.

I left a comment on your merged PR for the additional test. Does it also test VideoCapture by index? It seems that the VideoCapture is opened with a filepath instead of device index -- so I'm unsure if that test will actually test this code.

@alalek
Copy link
Copy Markdown
Member

alalek commented Dec 1, 2020

Does it also test VideoCapture by index

We usually don't have such tests in regular OpenCV testing scope (due to specific requirements).

However it would be nice to add some "manual" test here: https://github.com/opencv/opencv/blob/3.4.12/modules/videoio/test/test_camera.cpp

(check usage note in mentioned file)

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Dec 1, 2020

The patch looks good to me. Please squeeze commits to merge the code. Also it'll be great if you add disabled test for cameras in the way as alalek proposed.

asmorkalov
asmorkalov requested changes Dec 2, 2020
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.

Please add the same check to TEST(DISABLED_VideoIO_Camera, validate_V4L2_MJPEG).

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Dec 2, 2020

@Staticity Please squash commits after fix.

@Staticity
Add Timestamps to MSMF Video Capture by index
2fa624a 
Enable frame timestamp tests for MSMF

Add functional test for camera live timestamps

Remove trailing whitespace

Add timestamp test to all functional tests. Protect div by 0

Add Timestamps to MSMF Video Capture by index
@Staticity Staticity force-pushed the add_live_timestamps_to_msmf branch from 079bc86 to 2fa624a Compare December 2, 2020 21:38
asmorkalov
asmorkalov approved these changes Dec 3, 2020
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.

Looks good to me 👍

@Staticity
Copy link
Copy Markdown
Author

Staticity commented Dec 3, 2020

Wonderful, thanks for the help!

alalek
alalek approved these changes Dec 3, 2020
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.

Thank you for contribution!

@opencv-pushbot opencv-pushbot merged commit b98dd72 into opencv:3.4 Dec 3, 2020
@alalek alalek mentioned this pull request Dec 4, 2020
Merged
@alalek alalek mentioned this pull request Apr 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

@alalek alalek alalek approved these changes

Assignees

@asmorkalov asmorkalov

Labels

category: videoio feature platform: win32 platform: winrt/uwp

Projects

None yet

Milestone

3.4.13

Development

Successfully merging this pull request may close these issues.

4 participants

@Staticity @asmorkalov @alalek @opencv-pushbot