Skip to content

Add support for CAP_PROP_ORIENTATION_AUTO to AVFoundation backend#22792

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
tailsu:sd/avfoundation-orientation-meta
Nov 30, 2022
Merged

Add support for CAP_PROP_ORIENTATION_AUTO to AVFoundation backend#22792
asmorkalov merged 1 commit intoopencv:4.xfrom
tailsu:sd/avfoundation-orientation-meta

Conversation

@tailsu
Copy link
Copy Markdown
Contributor

@tailsu tailsu commented Nov 10, 2022 •
edited
Loading

This is an extension to #15499. I pulled some of the code up, so now it would be somewhat easier to add support for orientation metadata into the other backends as well.

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
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 14, 2022

Thanks a lot for the contribution. It makes sense to extend existing FFmpeg targeted test for Apple platform too. See https://github.com/opencv/opencv/blob/4.x/modules/videoio/test/test_ffmpeg.cpp#L479.

@asmorkalov asmorkalov added the pr: needs test New functionality requires minimal tests set label Nov 14, 2022
alalek
alalek reviewed Nov 14, 2022
View reviewed changes
@tailsu tailsu force-pushed the sd/avfoundation-orientation-meta branch 2 times, most recently from 09b154b to 0342f9e Compare November 14, 2022 13:34
@tailsu
Copy link
Copy Markdown
Contributor Author

tailsu commented Nov 14, 2022

Added tests. I parametrized the ffmpeg tests by backend API, and moved them to test_orientation.cpp. I also changed an assert to check that both backends support orientation metadata.

@tailsu tailsu force-pushed the sd/avfoundation-orientation-meta branch 2 times, most recently from dbc67f2 to d72e72b Compare November 14, 2022 14:01
@tailsu tailsu requested a review from alalek November 15, 2022 10:07
@asmorkalov asmorkalov removed the pr: needs test New functionality requires minimal tests set label Nov 21, 2022
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 22, 2022

The new test fails on Mac:

2022-11-21T12:50:40.9930830Z [ RUN      ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/0, where GetParam() = FFMPEG
2022-11-21T12:50:41.0010960Z [       OK ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/0 (14 ms)
2022-11-21T12:50:41.0011300Z [ RUN      ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/1, where GetParam() = AVFOUNDATION
2022-11-21T12:50:41.0102350Z /Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/test/test_orientation.cpp:41: Failure
2022-11-21T12:50:41.0102710Z Expected equality of these values:
2022-11-21T12:50:41.0102860Z   384
2022-11-21T12:50:41.0102980Z   frame.cols
2022-11-21T12:50:41.0103110Z     Which is: 0
2022-11-21T12:50:41.0103940Z [  FAILED  ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/1, where GetParam() = AVFOUNDATION (9 ms)

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 22, 2022

CI platform:

2022-11-21T12:33:52.0293350Z OS: macOS Monterey 12.4
2022-11-21T12:33:52.0293600Z CPU:
2022-11-21T12:33:52.0295020Z 	 model: Intel(R) Core(R) i7 CPU @ 2.20GHz
2022-11-21T12:33:52.0295230Z 	 number of CPU: 1
2022-11-21T12:33:52.0295420Z 	 total cores: 6
2022-11-21T12:33:52.0295560Z GPU:
2022-11-21T12:33:52.0295790Z 	 model: Intel(R) UHD Graphics 630
2022-11-21T12:33:52.0295980Z 	 number of GPU: 1
2022-11-21T12:33:52.0296120Z RAM:
2022-11-21T12:33:52.0296270Z 	 size: 32GB
2022-11-21T12:33:52.0296720Z 	 type: DDR4-2667

@tailsu
Copy link
Copy Markdown
Contributor Author

tailsu commented Nov 22, 2022

The tests seem to fail at the very end, when trying to read the actual frame into a mat and getting an empty mat as a result. Until that point all of the orientation metadata expectations pass. So, this issue doesn't seem to be caused by the PR changes themselves.

It looks like AVFoundation simply has trouble decoding big_buck_bunny_rotated.mp4 on that particular configuration. Apparently no other AVFoundation tests use that video. I tried opening the video with QuickTime and got an error saying "The file isn’t compatible with QuickTime Player" (I do my work on Mac Ventura with an M1). So it looks like that the video file is somehow borked.

I can try uploading a new video with rotation metadata. Do you think it's worth a shot?

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 23, 2022

I tried the rotated video and it works well with mplayer (ffmpeg) and vlc without hardware acceleration. VDPAU backend for VLC fails to play the video with rotation too. Not sure if it's related to rotation or very small amount of frames or some other option. Could you share very short clip with the option that could be used as test video?

@tailsu
Copy link
Copy Markdown
Contributor Author

tailsu commented Nov 23, 2022

Pushed a new video in opencv/opencv_extra#1019

@tailsu tailsu force-pushed the sd/avfoundation-orientation-meta branch from 2c3bffe to f0ca8bd Compare November 23, 2022 11:37
asmorkalov
asmorkalov approved these changes Nov 24, 2022
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 in general!

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Nov 24, 2022

@alalek Could you take a look?

alalek
alalek approved these changes Nov 24, 2022
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.

Looks good 👍

@asmorkalov asmorkalov self-assigned this Nov 24, 2022
@tailsu
add support for CAP_PROP_ORIENTATION_AUTO to AVFoundation backend
a462f49 
* extract rotateFrame as free function, rename to applyMetadataRotation
* LegacyCapture::get() always return 0, if cap is null
@tailsu tailsu force-pushed the sd/avfoundation-orientation-meta branch from f0ca8bd to a462f49 Compare November 25, 2022 16:27
@asmorkalov asmorkalov merged commit 1192779 into opencv:4.x Nov 30, 2022
@asmorkalov asmorkalov added this to the 4.7.0 milestone Nov 30, 2022
@alalek
Copy link
Copy Markdown
Member

alalek commented Nov 30, 2022 •
edited
Loading

opencv_extra PR is not provided, not reviewed and not merged. Whole CI is RED.

@tailsu tailsu deleted the sd/avfoundation-orientation-meta branch November 30, 2022 14:33
mshabunin
mshabunin reviewed Nov 30, 2022
View reviewed changes
modules/videoio/src/cap_ffmpeg.cpp

cv::Mat tmp(height, width, CV_MAKETYPE(CV_8U, cn), data, step);
this->rotateFrame(tmp);
applyMetadataRotation(*this, tmp);
Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Nov 30, 2022

Choose a reason for hiding this comment

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

Apparently this PR breaks FFmpeg plugin build (with -DVIDEOIO_PLUGIN_LIST=ffmpeg option). In this mode it does not compile cap.cpp which contains applyMetadataRotation function and can not find it. Perhaps we could move this function to one of headers used by both AVFoundation and FFmpeg backends or to a separate source module and add it to the plugin compilation process.

Copy link
Copy Markdown
Contributor Author

@tailsu tailsu Dec 1, 2022

Choose a reason for hiding this comment

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

Should I take care of this in a separate PR or will you do it internally in the team?

Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Dec 1, 2022

Choose a reason for hiding this comment

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

I'll try to fix it today.

@mshabunin mshabunin mentioned this pull request Dec 1, 2022
Merged
@alalek alalek mentioned this pull request Jan 8, 2023
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

@mshabunin mshabunin mshabunin left review comments

@alalek alalek alalek approved these changes

Assignees

@asmorkalov asmorkalov

Labels

category: videoio feature platform: ios/osx

Projects

None yet

Milestone

4.7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@tailsu @asmorkalov @alalek @mshabunin