Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imquery() and overloaded imread() functions #17753

Open
wants to merge 2 commits into
base: 4.x
Choose a base branch
from

Conversation

sturkmen72
Copy link
Contributor

@sturkmen72 sturkmen72 commented Jul 5, 2020

proposed solution to #6574 #17862

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 OpenCV (BSD) 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

@asmorkalov asmorkalov changed the title proposed solution to issue 6574 imreadmulti extention for issue #6574 Jul 13, 2020
@asmorkalov asmorkalov added category: imgcodecs RFC pr: needs test New functionality requires minimal tests set labels Jul 13, 2020
@vpisarev vpisarev self-assigned this Jul 13, 2020
@vpisarev
Copy link
Contributor

vpisarev commented Jul 13, 2020

@sturkmen72, thank you for the contribution! I think, we should discuss the API. Can you provide a little code snippet how people would use this new functionality?

In C++ one of the best approaches would be to create an image iterator. For Python it could be possible to use a similar approach, even though it would not look as idiomatic. Something like that:

class ImageCollection
{
public:
     ...
     CV_WRAP size_t nimages() const;
     CV_WRAP ImageIterator begin() const;
     ImageIterator end() const;
     ...

     CV_WRAP static ImageCollection fromMultiPageImage(const std::string& img);
     CV_WRAP static ImageCollection fromDirectory(const std::string& dir);
}; 

class ImageIterator
{
public:
   Mat operator *() const;
   CV_WRAP Mat read() const; // operator * for Python/Java
   CV_WRAP size_t currentIndex() const;
   CV_WRAP bool next(); // operator ++() for Python/Java
   ImageIterator& operator ++();
   ImageIterator operator ++(int);
protected:
   Ptr<ImageIteratorImpl> pimpl;
}

// C+++
for(cv::Mat img: ImageCollection::fromMultiPageImage("big_tiff_file.tif")) {
    ...
}

# Python
image_col = cv2.ImageCollection.fromMultiPageImage("big_tiff_file.tif")
nimg = image_col.nimages()
it = image_col.begin()
i  = 0
while i < nimg:
   img = it.read()
   # process img ...
   it.next()
   i += 1

of course, it would also be possible to create more Pythonic wrapper on top of it, like suggested here, for example:
https://treyhunner.com/2018/06/how-to-make-an-iterator-in-python/

@sturkmen72
Copy link
Contributor Author

@vpisarev thank you for your feedback. let me work on the PR a bit more

@asmorkalov
Copy link
Contributor

@sturkmen72 Do you have a chance to finish the PR in meantime?

@sturkmen72
Copy link
Contributor Author

@asmorkalov , @vpisarev i am still thinking of an effective usage way considering #8511

@sturkmen72 sturkmen72 force-pushed the patch-1 branch 5 times, most recently from f993d08 to 8613543 Compare August 6, 2020 14:34
@sturkmen72
Copy link
Contributor Author

sturkmen72 commented Aug 6, 2020

A simple review is required before continuing. please forgive my sloppiness i did not look codes of #8511 before. now i am aware both PR has basely similar changes.

@brian-armstrong-discord do you have time to look my changes

@sturkmen72
Copy link
Contributor Author

sturkmen72 commented Oct 6, 2020

@vpisarev, @alalek please take a look at this PR. I know it still need more work but i need to know if i am on the right path.

sample usage

C++

    ImageLoader imageloader;

    if( imageloader.open(filename, flags) )
        for (int i = 0; i < imageloader.getNumPages(); i++)
        {
            Mat page;
            if( imageloader.load(page) )
            {
                 // do something 
            }
            imageloader.nextPage();
        }

Python

import cv2

il = cv2.ImageLoader()
il.open('D:/Github/opencv_extra/testdata/highgui/readwrite/multipage.tif', ,cv2.IMREAD_REDUCED_GRAYSCALE_2)

for i in range(il.getNumPages()):
    ret,img = il.load()
    cv2.imshow("pages", img)
    cv2.waitKey()
    il.nextPage()

cv2.destroyAllWindows()

@vpisarev
Copy link
Contributor

@sturkmen72, thank you for keep working on it. Could you, please, provide some sample code and the test for the functionality?

@sturkmen72
Copy link
Contributor Author

@vpisarev, @alalek i created a repo for testing this PR. still i have some issues. is it possible to make a short online meeting about this PR.

note : indentations ignored for now to facilitate review. also there is many lines can be deleted if this PR will be mature.

@vpisarev
Copy link
Contributor

@sturkmen72, yes, let's discuss it. I sent you e-mail

@sturkmen72 sturkmen72 force-pushed the patch-1 branch 2 times, most recently from 12a9896 to 854c1c4 Compare May 30, 2021 08:23
row = int((page_info1.height - page_info3.height)/2)

roi = img1[row:row+page_info3.height, column:column+page_info3.width]
#ret,roi = cv.imread("images.tif", None, cv.IMREAD_REDUCED_COLOR_2, i)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can try the following:

ret,roi = cv.imread("images.tif", None, cv.IMREAD_REDUCED_COLOR_2, i)
img1[row:row+page_info3.height, column:column+page_info3.width] = roi

modules/imgcodecs/src/loadsave.cpp Outdated Show resolved Hide resolved
}
return !mats.empty();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mats shoud not be empty before calling this function

}
return !mats.empty();
return counter == count;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we asked for more images than multipage page count and we successfully read existing images should this be false ?

/**
* try to go to next page
*/
bool GdalDecoder::nextPage(){
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think GDAL driver loads only first page of multipage images. i will try to implement GdalDecoder::nextPage() after actual changes seems good.

@sturkmen72
Copy link
Contributor Author

@vpisarev I think I've made significant progress after our meeting. could you review the actual code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: imgcodecs pr: needs test New functionality requires minimal tests set RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants