Skip to content

Friendlier error messages when image path does not exist in cv2.seamlessClone() #20167#20617

Closed
snehal2403 wants to merge 3 commits intoopencv:4.xfrom
snehal2403:master
Closed

Friendlier error messages when image path does not exist in cv2.seamlessClone() #20167#20617
snehal2403 wants to merge 3 commits intoopencv:4.xfrom
snehal2403:master

Conversation

@snehal2403
Copy link
Copy Markdown

@snehal2403 snehal2403 commented Aug 27, 2021

This will solve the problem of showing Friendlier error messages when image path does not exist in cv2.seamlessClone().

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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

First imread was not showing error for invalid file path,
Now it will throw an exception if the file path is invalid
@mshabunin
Copy link
Copy Markdown
Contributor

There are several problems with this patch:

  • it changes function behavior - historically this function was not supposed to throw exceptions, it returns false in case of error
  • OpenCV does not throw std::exception by itself, there are CV_Error, CV_Assert and CV_DbgAssert functions which are used to throw cv::Exception
  • there is separate function which checks whether a file exists:
    CV_EXPORTS bool exists(const cv::String& path);
    CV_EXPORTS bool isDirectory(const cv::String& path);
  • this change is not tested

We assume that it is user's responsibility to distinguish between file-not-exists, access-denied and other failures. However, taking in account that many beginners have problems with this function I suggest adding a log message instead of throwing an exception: CV_LOG_DEBUG or CV_LOG_INFO.

@alalek
Copy link
Copy Markdown
Member

alalek commented Aug 27, 2021

As mentioned above, proposed change breaks existed API - replacing returned value by exception.
This would break already existed users code, so it is not acceptable.

Check existed discussion in issues: #14095

snehal2403 and others added 2 commits August 27, 2021 18:36
…essClone() opencv#20167

This code will solve the problem. It will show the error if the file path supplied to seamlessclone() is invalid.
@snehal2403 snehal2403 changed the title Updation in imread function Friendlier error messages when image path does not exist in cv2.seamlessClone() #20167 Aug 27, 2021
Copy link
Copy Markdown
Author

@snehal2403 snehal2403 left a comment

Choose a reason for hiding this comment

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

This code will solve the problem. It will show the error if the file path supplied to seamlessclone() is invalid.

@snehal2403 snehal2403 closed this Aug 27, 2021
@snehal2403 snehal2403 reopened this Aug 27, 2021
int dst_emp = _dst.empty();
int mask_emp = _mask.empty();
if (src_emp == 1)
cout << "Path of Source image is invalid" << endl;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which path is mentioned here if frame data may come from the camera?
There in no "path" on algorithm implementation levels.

Unconditional using of cout is not allowed in cross-platform code.

This should be enough:

  • CV_Assert(!_src.empty());

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okay thanks for your review. But here this code will check if the image read by imread() function is empty or not. Here no path is supplied to seamessclone() instead the parameters type is array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants