Addedexplicit toVideoCapture 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 added the category: videoio label

Dec 27, 2019

Copy link

ContributorAuthor

VadimLevin commentedDec 27, 2019

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

asmorkalov requested a review fromalalek

January 10, 2020 05:43

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp

		@sa The list of supported API backends cv::VideoCaptureAPIs
		*/
		CV_WRAPVideoCapture(const String& filename,int apiPreference = CAP_ANY);
		CV_WRAPexplicitVideoCapture(const String& filename,int apiPreference = CAP_ANY);

Copy link

Contributor

Choose a reason for hiding this comment

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

this is useful change

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp

		@sa The list of supported API backends cv::VideoCaptureAPIs
		*/
		CV_WRAPVideoCapture(int index,int apiPreference = CAP_ANY);
		CV_WRAPexplicitVideoCapture(int index,int apiPreference = CAP_ANY);

Copy link

Contributor

Choose a reason for hiding this comment

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

useful

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/include/opencv2/videoio.hpp OutdatedShow resolvedHide resolved

vpisarev reviewed

Jan 10, 2020

View reviewed changes

modules/videoio/src/cap.cppShow resolvedHide resolved

Copy link

Contributor

vpisarev commentedJan 10, 2020•
edited
Loading

@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.

Copy link

ContributorAuthor

VadimLevin commentedJan 13, 2020•
edited
Loading

@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 fromVideoWriter andVideoCapture becauseopen andrelease 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 ruleC.132: Don’t make a function virtual without reason from Cpp Core Guidelines)

VadimLevin force-pushed thedev/vlevin/videoio_classes_cleanup branch from5c5ce9d toe2f3446Compare

January 22, 2020 08:01

VadimLevin changed the base branch frommaster to3.4

January 22, 2020 08:01

VadimLevin changed the base branch from3.4 tomaster

January 22, 2020 08:02

VideoIO classes refactoring

3fe9dfa

  - 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 force-pushed thedev/vlevin/videoio_classes_cleanup branch frome2f3446 to3fe9dfaCompare

January 22, 2020 09:05