Skip to content

C-API cleanup: highgui, videoio#26025

Merged
asmorkalov merged 1 commit intoopencv:5.xfrom
mshabunin:cpp-videoio-highgui
Sep 9, 2024
Merged

C-API cleanup: highgui, videoio#26025
asmorkalov merged 1 commit intoopencv:5.xfrom
mshabunin:cpp-videoio-highgui

Conversation

@mshabunin
Copy link
Copy Markdown
Contributor

@mshabunin mshabunin commented Aug 13, 2024 •
edited
Loading

❗ Potential conflicts with #25958
❗ Merge with: opencv/opencv_contrib#3780

This PR removes usage of C-API from highgui and videoio modules. Only source code is affected, tests were not using obsolete API.

It should be possible to backport these changes to 4.x branch preserving removed public headers and source files (*_c.h and *_c.cpp).

Checklist

I tried to verify as many backends as possible, though these checks were not as thorough as I'd like them to be. Below is the checklist covering all modified backends with their statuses.

🔹 - small changes
🟢 - consider working
⚪ - considered untested

highgui
Pass Backend Local check CI check
🟢 GTK2 build + test, plugin build build + test ❔
🟢 GTK3 build + test, plugin build build + test
🟢 QT build + test, plugin build
🟢 Wayland 🔹 build + test
🟢 WIN32 🔹 build + test
🟢 Cocoa 🔹 build + test
⚪ WinRT
videoio
Pass Backend Local check CI check
🟢 Android Camera/MediaNDK 🔹 build
🟢 Aravis build
🟢 AVFoundation OSX build + test
🟢 AVFoundation iOS build + test build
🟢 DC1394 build
🟢 DShow 🔹 build
🟢 FFMpeg build, plugin build build + test
🟢 GPhoto 🔹 build
🟢 GStreamer build, plugin build build + test
🟢 Images build build + test
🟢 MSMF 🔹 build + test
🟢 OpenNI build
🟢 PVAPI build
🟢 V4L build + test build
🟢 XIMEA build
🟢 XINE 🔹 build

Notes

  • local linux build checks performed using this framework
  • minor extra changes made in both cap_avfoundation*.mm to make them slightly more synchronized - it would be better to combine them into a single one in the future
  • configurations with plugins have been build but not tested
  • moved unrelated changes to separate PRs two issues have been fixed in separate commits:
    • imgproc: missing cv::hal:: color conversion functions has been used in MediaSDK backend
    • videoio/V4L: wrong color conversion mode caused bad colors for NV12 camera input format (RGB instead of BGR)

It would be nice to check following functionality manually:

  • OSX: camera input
  • iOS: camera and file input
  • WinRT: build, some testing
  • Linux/Wayland: build

opencv-alalek
opencv-alalek reviewed Aug 14, 2024
View reviewed changes
modules/core/include/opencv2/core/cvdef.h
}

//! Macro to construct the fourcc code of the codec. Same as CV_FOURCC()
#define CV_FOURCC_MACRO(c1, c2, c3, c4) (((c1) & 255) + (((c2) & 255) << 8) + (((c3) & 255) << 16) + (((c4) & 255) << 24))
Copy link
Copy Markdown
Contributor

@opencv-alalek opencv-alalek Aug 14, 2024

Choose a reason for hiding this comment

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

May be move it to videoio.hpp as inline function (which could be exported to bindings)

BTW, there is VideoWriter::fourcc (it is confusing because it is used outside of that). So global function is preferable.

Copy link
Copy Markdown
Contributor Author

@mshabunin mshabunin Aug 14, 2024

Choose a reason for hiding this comment

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

There is CV_FOURCC inline function above (used in ~70 places). Do you mean move it too? It is used in imgproc for font rendering params.

Perhaps we can remove CV_FOURCC_MACRO completely, I didn't do it to reduce total changes, because it is used in ~50 places.

VideoWriter::fourcc is used in ~50 places too.

Copy link
Copy Markdown
Contributor

@opencv-alalek opencv-alalek Aug 14, 2024

Choose a reason for hiding this comment

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

That is a mess.
Lets revise fourcc through a separate task then.

@mshabunin mshabunin mentioned this pull request Aug 16, 2024
Merged
asmorkalov
asmorkalov reviewed Aug 19, 2024
View reviewed changes
@asmorkalov asmorkalov added this to the 5.0 milestone Aug 19, 2024
@asmorkalov asmorkalov added cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) category: highgui-gui category: videoio category: videoio(camera) labels Aug 19, 2024
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Aug 20, 2024

Mac specific test failure:

[ RUN      ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/1, where GetParam() = AVFOUNDATION
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/videoio/test/test_orientation.cpp:29: Failure
Value of: cap.set(CAP_PROP_ORIENTATION_AUTO, true)
  Actual: false
Expected: true
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/videoio/test/test_orientation.cpp:34: Failure
Expected equality of these values:
  270
  actual.width
    Which is: 480
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/videoio/test/test_orientation.cpp:35: Failure
Expected equality of these values:
  480
  actual.height
    Which is: 270
/Users/opencv-cn/GHA-OCV-1/_work/opencv/opencv/opencv/modules/videoio/test/test_orientation.cpp:41: Failure
Expected equality of these values:
  270
  frame.cols
    Which is: 480
[  FAILED  ] videoio/VideoCaptureAPITests.mp4_orientation_meta_auto/1, where GetParam() = AVFOUNDATION (15 ms)

@mshabunin
Copy link
Copy Markdown
Contributor Author

mshabunin commented Aug 20, 2024 •
edited
Loading

LegacyCapture wrapper handled orientation for old backends (

opencv/modules/videoio/src/cap_interface.hpp

Lines 298 to 332 in a087a9c

double getProperty(int propId) const CV_OVERRIDE
{
if (!cap)
return 0;
switch(propId)
{
case cv::CAP_PROP_ORIENTATION_AUTO:
return static_cast<double>(autorotate);
case cv::CAP_PROP_FRAME_WIDTH:
return shouldSwapWidthHeight() ? cap->getProperty(cv::CAP_PROP_FRAME_HEIGHT) : cap->getProperty(cv::CAP_PROP_FRAME_WIDTH);
case cv::CAP_PROP_FRAME_HEIGHT:
return shouldSwapWidthHeight() ? cap->getProperty(cv::CAP_PROP_FRAME_WIDTH) : cap->getProperty(cv::CAP_PROP_FRAME_HEIGHT);
default:
return cap->getProperty(propId);
}
}
bool setProperty(int propId, double value) CV_OVERRIDE
{
if (!cap)
return false;
switch(propId)
{
case cv::CAP_PROP_ORIENTATION_AUTO:
autorotate = (value != 0);
return true;
default:
return cvSetCaptureProperty(cap, propId, value) != 0;
}
}
), but now it doesn't and each backend should implement it separately. I'll try to solve it in a more or less compact way.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Aug 21, 2024 •
edited
Loading

Mac build issue:

/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:990:21: error: out-of-line definition of 'retrieveFrame' does not match any declaration in 'CvCaptureFile'; did you mean 'retrieveFrame_'?
bool CvCaptureFile::retrieveFrame(int, cv::OutputArray arr) {
                    ^~~~~~~~~~~~~
                    retrieveFrame_
/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:145:10: note: 'retrieveFrame_' declared here
    bool retrieveFrame_(int, cv::OutputArray) CV_OVERRIDE;
         ^
/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:998:23: error: out-of-line definition of 'getProperty' does not match any declaration in 'CvCaptureFile'; did you mean 'getProperty_'?
double CvCaptureFile::getProperty(int property_id) const{
                      ^~~~~~~~~~~
                      getProperty_
/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:146:12: note: 'getProperty_' declared here
    double getProperty_(int property_id) const CV_OVERRIDE;
           ^
/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:1033:21: error: out-of-line definition of 'setProperty' does not match any declaration in 'CvCaptureFile'; did you mean 'setProperty_'?
bool CvCaptureFile::setProperty(int property_id, double value) {
                    ^~~~~~~~~~~
                    setProperty_
/Users/opencv-cn/GHA-OCV-2/_work/opencv/opencv/opencv/modules/videoio/src/cap_avfoundation_mac.mm:147:10: note: 'setProperty_' declared here
    bool setProperty_(int property_id, double value) CV_OVERRIDE;
         ^
3 errors generated.

@mshabunin mshabunin force-pushed the cpp-videoio-highgui branch from 2c9581f to 2f3f30a Compare August 22, 2024 11:11
@mshabunin mshabunin marked this pull request as ready for review August 22, 2024 11:13
@mshabunin mshabunin force-pushed the cpp-videoio-highgui branch from 2f3f30a to dda16aa Compare August 22, 2024 11:44
vpisarev
vpisarev reviewed Aug 23, 2024
View reviewed changes
@opencv-alalek opencv-alalek mentioned this pull request Aug 23, 2024
Merged
@mshabunin mshabunin force-pushed the cpp-videoio-highgui branch from dda16aa to 2d042d9 Compare August 23, 2024 14:54
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Aug 26, 2024

Wayland build:

/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:608:20: error: ‘InputArray’ has not been declared
  608 |     void set_image(InputArray arr);
      |                    ^~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:743:21: error: ‘InputArray’ has not been declared
  743 |     void show_image(InputArray arr);
      |                     ^~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:1585:6: error: variable or field ‘set_image’ declared void
 1585 | void cv_wl_viewer::set_image(InputArray arr) {
      |      ^~~~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:1585:30: error: ‘InputArray’ was not declared in this scope; did you mean ‘cv::InputArray’?
 1585 | void cv_wl_viewer::set_image(InputArray arr) {
      |                              ^~~~~~~~~~
      |                              cv::InputArray
In file included from /home/vlad/Projects/opencv/modules/core/include/opencv2/core.hpp:58,
                 from /home/vlad/Projects/opencv/modules/highgui/src/precomp.hpp:49,
                 from /home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:5:
/home/vlad/Projects/opencv/modules/core/include/opencv2/core/mat.hpp:446:28: note: ‘cv::InputArray’ declared here
  446 | typedef const _InputArray& InputArray;
      |                            ^~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:1925:6: error: variable or field ‘show_image’ declared void
 1925 | void cv_wl_window::show_image(InputArray arr) {
      |      ^~~~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:1925:31: error: ‘InputArray’ was not declared in this scope; did you mean ‘cv::InputArray’?
 1925 | void cv_wl_window::show_image(InputArray arr) {
      |                               ^~~~~~~~~~
      |                               cv::InputArray
/home/vlad/Projects/opencv/modules/core/include/opencv2/core/mat.hpp:446:28: note: ‘cv::InputArray’ declared here
  446 | typedef const _InputArray& InputArray;
      |                            ^~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:2495:1: error: ‘CV_IMPL’ does not name a type; did you mean ‘CV_CMP’?
 2495 | CV_IMPL int createTrackbar2Impl(const char *trackbar_name, const char *window_name, int *val, int count,
      | ^~~~~~~
      | CV_CMP
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:2540:38: error: ‘InputArray’ has not been declared
 2540 | void showImageImpl(const char *name, InputArray arr) {
      |                                      ^~~~~~~~~~
/home/vlad/Projects/opencv/modules/highgui/src/window_wayland.cpp:2540:6: warning: no previous declaration for ‘void showImageImpl(const char*, int)’ [-Wmissing-declarations]
 2540 | void showImageImpl(const char *name, InputArray arr) {
      |      ^~~~~~~~~~~~~

@mshabunin
Copy link
Copy Markdown
Contributor Author

mshabunin commented Aug 26, 2024

Wayland build has been fixed, but I can not test it.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Aug 26, 2024

Wayland branch works well!

asmorkalov
asmorkalov approved these changes Aug 27, 2024
View reviewed changes
@mshabunin mshabunin force-pushed the cpp-videoio-highgui branch from e6b85e1 to 71c5b8a Compare August 28, 2024 13:54
@mshabunin
Copy link
Copy Markdown
Contributor Author

mshabunin commented Aug 28, 2024

Moved unrelated changes to #26081 and #26082, rebased

asmorkalov
asmorkalov reviewed Aug 30, 2024
View reviewed changes
@mshabunin mshabunin added the backport is needed Label for maintainers. Authors of PR can ignore this label Sep 3, 2024
@VadimLevin
Copy link
Copy Markdown
Contributor

VadimLevin commented Sep 6, 2024

iOS framework build verified. ColorBlobDetection sample works as expected on the device.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 7, 2024

@mshabunin Please fix remaining review notes and I merge the PR.

@mshabunin
Copy link
Copy Markdown
Contributor Author

mshabunin commented Sep 9, 2024

@asmorkalov , done. Please note there is potential conflict with GTK4 PR (#25958), it might be hard to merge 4.x->5.x when both PRs are integrated. We can try to backport GTK changes to 4.x from this PR or merge in the following order: GTK4, 4.x->5.x, this PR.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 9, 2024

Tested manually OpenNI2 with Orbbec Astra camera.

@asmorkalov asmorkalov merged commit 7bdc618 into opencv:5.x Sep 9, 2024
@mshabunin mshabunin deleted the cpp-videoio-highgui branch September 9, 2024 15:56
@asmorkalov asmorkalov mentioned this pull request Sep 12, 2024
Merged
6 tasks
@mshabunin mshabunin mentioned this pull request Oct 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vpisarev vpisarev vpisarev left review comments

@asmorkalov asmorkalov asmorkalov approved these changes

@opencv-alalek opencv-alalek opencv-alalek left review comments

Assignees

No one assigned

Labels

backport is needed Label for maintainers. Authors of PR can ignore this category: highgui-gui category: videoio(camera) category: videoio cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code)

Projects

OpenCV 5.0
Status: Done

Milestone

5.0-alpha

Development

Successfully merging this pull request may close these issues.

5 participants

@mshabunin @asmorkalov @VadimLevin @vpisarev @opencv-alalek