VideoIO classes refactoring #16246

VadimLevin · 2019-12-27T09:44:35Z

Changes:

Added explicit to VideoCapture constructors with 2
arguments, 1 of them has default value
Applied library code style
Introduced 2 debug macros to improve readability of the code

VadimLevin · 2019-12-27T10:40:18Z

This change breaks ABI compatibility. @alalek what do you think about it?

vpisarev · 2020-01-10T14:46:02Z

modules/videoio/include/opencv2/videoio.hpp

@@ -628,7 +628,7 @@ class CV_EXPORTS_W VideoCapture
    implementation if multiple are available: e.g. cv::CAP_FFMPEG or cv::CAP_IMAGES or cv::CAP_DSHOW.
    @sa The list of supported API backends cv::VideoCaptureAPIs
    */
-    CV_WRAP VideoCapture(const String& filename, int apiPreference = CAP_ANY);
+    CV_WRAP explicit VideoCapture(const String& filename, int apiPreference = CAP_ANY);


this is useful change

vpisarev · 2020-01-10T14:46:14Z

modules/videoio/include/opencv2/videoio.hpp

@@ -640,13 +640,13 @@ class CV_EXPORTS_W VideoCapture

    @sa The list of supported API backends cv::VideoCaptureAPIs
    */
-    CV_WRAP VideoCapture(int index, int apiPreference = CAP_ANY);
+    CV_WRAP explicit VideoCapture(int index, int apiPreference = CAP_ANY);


modules/videoio/include/opencv2/videoio.hpp

modules/videoio/src/cap.cpp

vpisarev · 2020-01-10T14:59:34Z

@VadimLevin, if I remember correctly, the goal was to add some profiles to video writer to enhance functionality. Here I just see many changes and absolutely no steps towards the goal that we stated. I have no principal objections against the changes in .cpp, as long as it passes all the tests. But, except for adding "explicit" to the constructors, please, undo all the changes in header files, because the lost compatibility is too high price for a questionably better style.

VadimLevin · 2020-01-13T08:50:46Z

@vpisarev Yes, I separated work into several independent steps and this was one of them.
virtual methods in this class may lead to unexpected behavior for users, those want to inherited their classes from VideoWriter and VideoCapture because open and release are called in constructor and destructor respectively. If inheritance is not supposed to be, so there is no reasons to leave virtual methods in these classes at all (I try to follow the rule C.132: Don’t make a function virtual without reason from Cpp Core Guidelines)

- Added `explicit` to `VideoCapture` constructors with 2 arguments, 1 of them has default value - Applied library code style - Introduced 2 debug macros to improve readability of the code

asmorkalov · 2020-01-23T06:11:30Z

@vpisarev @alalek Please take a look again.
@VadimLevin Please update PR description after scope change.

asmorkalov · 2020-01-29T10:56:57Z

@vpisarev friendly reminder.

…cleanup

VadimLevin added the category: videoio label Dec 27, 2019

asmorkalov requested a review from alalek January 10, 2020 05:43