Conversation
modules/videoio/src/cap_aravis.cpp
Outdated
| } | ||
|
|
||
| virtual bool open(int); | ||
| virtual bool open(int, const cv::VideoCaptureParameters&); |
There was a problem hiding this comment.
Lets avoid modification of all backends implementation.
Use virtual function of base class instead.
Modify 1-2 backend which are interested and can be used as sample.
There was a problem hiding this comment.
add the open(..., const cv :: VideoCaptureParameters &) function to the base class and implement it only in the required backends (example: gstreamer, msmf)?
There was a problem hiding this comment.
yes.
Also check similar patch for VideoWriter from here: #16766
There was a problem hiding this comment.
Still don't get it. Do you mean that you need to take all the open () to the top in the base class? Can you tell us more about your idea. When implementing the parameters of the video writer, the implementation of the open () function and the extraction of parameters remains at the backend level.
There was a problem hiding this comment.
Add common base class method:
bool CvCapture::open(int idx, const cv::VideoCaptureParameters& params)
{
bool res = open(idx);
if (res && !params.empty())
{
res &= setParameters(params);
if (!res)
close();
}
return res;
}
Derived classes which don't care about new API don't redefine this method.
Only FFmpeg / GStreamer / MSMF backends may have such method as an example (but do not bring large functionality there - it is not in scope of this PR). If you don't understand then don't add this overload except base classes.
Avoid such merge commits in PRs. Prefer to keep 1 commit with proper author/e-mail information instead. Use |
5eeb4ac to
442dfe7
Compare
alalek
left a comment
There was a problem hiding this comment.
Current patch has these common mistakes:
- no tests (especially for new features)
- massive code changes (which should be avoided): 28 changed files
- lack of logic for handling of error cases (like silent ignoring of parameters)
| /****************** Implementation of interface functions ********************/ | ||
|
|
||
| Ptr<IVideoCapture> cv::createAndroidCapture_file(const std::string &filename) { | ||
| Ptr<IVideoCapture> cv::createAndroidCapture_file(const std::string &filename, const cv::VideoCaptureParameters&) { |
There was a problem hiding this comment.
Logic error: We can't just silently ignore parameters.
There was a problem hiding this comment.
I ignore parameters in backends that don't have a open() function or where the base class is CVCapture. Should I add a message about the lack of support for params in this backend? I can also add to the CVCapture the same overload of open functions as in IVideoCapture.
There was a problem hiding this comment.
These changes are dropped.
Fetch latest changes from GitHub with updated handling of such cases.
modules/videoio/src/cap.cpp
Outdated
| bool VideoCapture::open(const String& filename, int apiPreference) | ||
| { | ||
| CV_TRACE_FUNCTION(); | ||
| return open(filename, apiPreference, std::vector<int> {}); |
There was a problem hiding this comment.
std::vector {}
Just: std::vector<int>()
|
|
||
| @overload | ||
|
|
||
| Parameters are same as the constructor VideoCapture(const String& filename, int apiPreference = CAP_ANY, bool const std::vector<int>& params) |
There was a problem hiding this comment.
as the constructor
Where is this constructor?
- add tests - FFmpeg backend sample code - StaticBackend API is done - support through PluginBackend API will be added later
add video capture parameters * add parameters * videoio: revert unnecessary massive changes * videoio: support capture parameters in backends API - add tests - FFmpeg backend sample code - StaticBackend API is done - support through PluginBackend API will be added later Co-authored-by: Milashchenko <maksim.milashchenko@intel.com> Co-authored-by: Alexander Alekhin <alexander.a.alekhin@gmail.com>
relates #16766
@alalek , @mshabunin , @allnes