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

export img decoders/encoders #8511

Open
wants to merge 2 commits into
base: master
from

Conversation

5 participants
@brian-armstrong-discord
Contributor

brian-armstrong-discord commented Apr 3, 2017

This pull request changes the image codecs API so that findDecoder and findEncoder are available to users. This allows users to inspect the header of an image before decoding it so that they can decide whether they want to decode the image before they do so. For example, the image might describe itself as being 1000000 x 1000000 pixels, in which case openCV will go ahead and allocate an extremely large buffer to hold this image. This new API gives the user the ability to decide they don't want to do that.

In addition, this patch attempts to comment the API a little and make it safer to use by doing bounds checking in readData so that a too-small buffer given to readData will not yield some kind of bad write outside array bounds.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 3, 2017

Contributor
  • added string description to each decoder so that the caller can decide if they don't want to touch the header once the magic bytes have been read. for example, tiffs are quite prone to CVEs, so we might not want to touch them at all.
Contributor

brian-armstrong-discord commented Apr 3, 2017

  • added string description to each decoder so that the caller can decide if they don't want to touch the header once the magic bytes have been read. for example, tiffs are quite prone to CVEs, so we might not want to touch them at all.

brian-armstrong-discord added a commit to brian-armstrong-discord/opencv that referenced this pull request Apr 7, 2017

export exif orientation
this builds on opencv#8492 and
opencv#8511 to add a new method,
.orientation(), to ImageDecoder. The JPEG subclass now overrides this to
do jpeg-specific exif tag reading

we also now export a method, OrientationTransform, which uses this
orientation value to do the actual matrix operations
@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 19, 2017

Contributor

This patch resolves #8605 and #8606

Contributor

brian-armstrong-discord commented Apr 19, 2017

This patch resolves #8605 and #8606

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Apr 19, 2017

Contributor

New OpenCV public API should not contain complex classes with "virtual" methods. This is needed to avoid ABI (application binary interface) violations and producing hacks like this - just want to add new method.

Please follow "pImpl design pattern" instead. Example: ocl::Device
You would still have these "virtual classes", but they will go into the private scope (into some .hpp file from src folder: without exposing them public) - probably you should just reuse existed BaseImageEncoder as "internal" implementation.

Please avoid(or minimize) using of STL objects in interfaces (std::vector/std::map/std::string/etc) - this may raise ABI problems which is very hard to debug (especially for newbie C++ developers): #6198 #5227 #6165

Contributor

alalek commented Apr 19, 2017

New OpenCV public API should not contain complex classes with "virtual" methods. This is needed to avoid ABI (application binary interface) violations and producing hacks like this - just want to add new method.

Please follow "pImpl design pattern" instead. Example: ocl::Device
You would still have these "virtual classes", but they will go into the private scope (into some .hpp file from src folder: without exposing them public) - probably you should just reuse existed BaseImageEncoder as "internal" implementation.

Please avoid(or minimize) using of STL objects in interfaces (std::vector/std::map/std::string/etc) - this may raise ABI problems which is very hard to debug (especially for newbie C++ developers): #6198 #5227 #6165

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 19, 2017

Contributor

@alalek 👍 Good stuff. I think I thought I was accomplishing the pimpl pattern by making the virtual base class and returning Ptr but this makes a lot more sense. Will make those changes soon.

Contributor

brian-armstrong-discord commented Apr 19, 2017

@alalek 👍 Good stuff. I think I thought I was accomplishing the pimpl pattern by making the virtual base class and returning Ptr but this makes a lot more sense. Will make those changes soon.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 20, 2017

Contributor

@alalek What would you think about leaving ImageEncoder's write params arg as const std::vector<int>& params? imencode already takes this and it seems like it would make the most sense to have parity with that function, since the argument types are the same.

Actually, for that matter, imencode also uses std::vector<uchar>& buf for its output. So to do that with ImageEncoder here would match imencode as well.

As far as I can tell those are the only places in this patch where STL types are exposed in the ABI.

Contributor

brian-armstrong-discord commented Apr 20, 2017

@alalek What would you think about leaving ImageEncoder's write params arg as const std::vector<int>& params? imencode already takes this and it seems like it would make the most sense to have parity with that function, since the argument types are the same.

Actually, for that matter, imencode also uses std::vector<uchar>& buf for its output. So to do that with ImageEncoder here would match imencode as well.

As far as I can tell those are the only places in this patch where STL types are exposed in the ABI.

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Apr 20, 2017

Contributor

Probably we can try to use InputArray wrapper instead of "std::vector& params" (the latest changes support C++ std::array<> argument type).

void setDestination( std::vector& buf )

I believe Mat& (of 8UC1) should work in this case too.

Contributor

alalek commented Apr 20, 2017

Probably we can try to use InputArray wrapper instead of "std::vector& params" (the latest changes support C++ std::array<> argument type).

void setDestination( std::vector& buf )

I believe Mat& (of 8UC1) should work in this case too.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 21, 2017

Contributor

@alalek is it possible to run the tests locally? i tried pulling down the sample data but i wasn't really sure where it needs to go or how to invoke it, so i'm dependent on the tester here to check for now :(

Contributor

brian-armstrong-discord commented Apr 21, 2017

@alalek is it possible to run the tests locally? i tried pulling down the sample data but i wasn't really sure where it needs to go or how to invoke it, so i'm dependent on the tester here to check for now :(

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 21, 2017

Contributor

@alalek Ok, nevermind, I put together a small test program.

Contributor

brian-armstrong-discord commented Apr 21, 2017

@alalek Ok, nevermind, I put together a small test program.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 21, 2017

Contributor

@alalek So this passes the tests, but I'm not so good at opencv-fu, and I needed to allocate a new Mat and then copy that Mat back to the destination vector in imencode once the encode finished. This is the only way I know of to maintain the backwards compat here with std::vector.

Is there a way around that that we can avoid? I have briefly tried using OutputArray but I'm not really sure how to do the operations I need to on it, including when it's of type std::vector

Contributor

brian-armstrong-discord commented Apr 21, 2017

@alalek So this passes the tests, but I'm not so good at opencv-fu, and I needed to allocate a new Mat and then copy that Mat back to the destination vector in imencode once the encode finished. This is the only way I know of to maintain the backwards compat here with std::vector.

Is there a way around that that we can avoid? I have briefly tried using OutputArray but I'm not really sure how to do the operations I need to on it, including when it's of type std::vector

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 25, 2017

Contributor

@alalek I folded #8547 into this patch - it wasn't a huge change, just exported the Exif stuff on top of Decoder/Encoder. It's now part of the JPEGDecoder

Contributor

brian-armstrong-discord commented Apr 25, 2017

@alalek I folded #8547 into this patch - it wasn't a huge change, just exported the Exif stuff on top of Decoder/Encoder. It's now part of the JPEGDecoder

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Apr 25, 2017

Contributor

@brian-armstrong-discord There is some conflicts with master branch. Please rebase this patch to actual master branch.

Something like these:

BTW, We prefer to have patches with single commit to eliminate intermediate "fixup" commits from git history. Sometimes it is better to squash your commits before rebase to upstream code (git rebase -i HEAD~11 (11 is number of commits); "pick" the first commit, "f" or "s" - others)

Rebased branch for reference:

Contributor

alalek commented Apr 25, 2017

@brian-armstrong-discord There is some conflicts with master branch. Please rebase this patch to actual master branch.

Something like these:

BTW, We prefer to have patches with single commit to eliminate intermediate "fixup" commits from git history. Sometimes it is better to squash your commits before rebase to upstream code (git rebase -i HEAD~11 (11 is number of commits); "pick" the first commit, "f" or "s" - others)

Rebased branch for reference:

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord May 2, 2017

Contributor

@alalek Looks like the conflicts are gone now

Contributor

brian-armstrong-discord commented May 2, 2017

@alalek Looks like the conflicts are gone now

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord May 24, 2017

Contributor

Hi @alalek

Have you had a chance to look at this recently?

Contributor

brian-armstrong-discord commented May 24, 2017

Hi @alalek

Have you had a chance to look at this recently?

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek May 24, 2017

Contributor

Hi @brian-armstrong-discord !
I'm sorry for huge delay. I need more time to review this (to test some suggestions on public interfaces).

Contributor

alalek commented May 24, 2017

Hi @brian-armstrong-discord !
I'm sorry for huge delay. I need more time to review this (to test some suggestions on public interfaces).

@Turim

This comment has been minimized.

Show comment
Hide comment
@Turim

Turim Jun 9, 2017

Contributor

@alalek @brian-armstrong-discord I also found the idea -to get the header data- very promising. If one might ask, what's the state so far?

P.S. It doesn't seem like you need some help, but if you are - feel free to text me :)
P.P.S.

  • The stated patch intention is to provide, say, lazy evaluation for image processing. But to do so the export whole bunch of ImageEncoders/Decoders is seen as little redundand (4 me ofc) - e.g. we could have added just +1 method imreadheader() or like so. (as C-like interface extension)
  • I wonder shall or shall not Encoders be visible with only simmetric (AFAIK) reasoning
  • I have some concerns about imageDescription(). As for me, the downside is the case -what if I want to know, what the exact format of the image?-. Should I compare the strings? Or shall we introduce an enum instead?
  • Also, it seems, ctors like Decoder(const Decoder& oth, PtrDecoder::Impl) could be avoided.
Contributor

Turim commented Jun 9, 2017

@alalek @brian-armstrong-discord I also found the idea -to get the header data- very promising. If one might ask, what's the state so far?

P.S. It doesn't seem like you need some help, but if you are - feel free to text me :)
P.P.S.

  • The stated patch intention is to provide, say, lazy evaluation for image processing. But to do so the export whole bunch of ImageEncoders/Decoders is seen as little redundand (4 me ofc) - e.g. we could have added just +1 method imreadheader() or like so. (as C-like interface extension)
  • I wonder shall or shall not Encoders be visible with only simmetric (AFAIK) reasoning
  • I have some concerns about imageDescription(). As for me, the downside is the case -what if I want to know, what the exact format of the image?-. Should I compare the strings? Or shall we introduce an enum instead?
  • Also, it seems, ctors like Decoder(const Decoder& oth, PtrDecoder::Impl) could be avoided.
@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Jun 21, 2017

Contributor

Hi @alalek

Any updates in this PR? I would love to put this work behind me finally :)

Anecdotally, this patch is working well in production. I know you will likely have some more requests on the interface of it, though.

Contributor

brian-armstrong-discord commented Jun 21, 2017

Hi @alalek

Any updates in this PR? I would love to put this work behind me finally :)

Anecdotally, this patch is working well in production. I know you will likely have some more requests on the interface of it, though.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Jun 21, 2017

Contributor

@Turim an imreadheader would, among other issues, force us to re-read the header bytes when we go to decode. it also opens the possibility of imread() and imreadheader() doing different things. this would be a brittle approach to the solution.

i'm open to using enums, but internally opencv uses strings, and it made sense to me to make this change as lightweight as possible.

Contributor

brian-armstrong-discord commented Jun 21, 2017

@Turim an imreadheader would, among other issues, force us to re-read the header bytes when we go to decode. it also opens the possibility of imread() and imreadheader() doing different things. this would be a brittle approach to the solution.

i'm open to using enums, but internally opencv uses strings, and it made sense to me to make this change as lightweight as possible.

@Turim

This comment has been minimized.

Show comment
Hide comment
@Turim

Turim Jun 23, 2017

Contributor

@brian-armstrong-discord ,
Frankly speaking, I didn't see here strong reasoning against.

  • Yup, re-read() is possible. Ok, we could live even with that or provide within imreadheader() an out param, which could be used to read the image but header.
  • About possibilities. A strange argument, as for me, coz we could have had tests for the case. Besides, we could implement imread() via imreadheader() and that's it, isn't it?

i'm open to using enums, but internally opencv uses strings, and it made sense to me to make this change as lightweight as possible.

Can I ask you to be more specific? In competition 'enum vs strings' where's the pros of the string usage?
(Say, at present to the question -Which format does the image have?- I see the overhead in terms of string comparisons and have no clue -how to avoid it-).

Contributor

Turim commented Jun 23, 2017

@brian-armstrong-discord ,
Frankly speaking, I didn't see here strong reasoning against.

  • Yup, re-read() is possible. Ok, we could live even with that or provide within imreadheader() an out param, which could be used to read the image but header.
  • About possibilities. A strange argument, as for me, coz we could have had tests for the case. Besides, we could implement imread() via imreadheader() and that's it, isn't it?

i'm open to using enums, but internally opencv uses strings, and it made sense to me to make this change as lightweight as possible.

Can I ask you to be more specific? In competition 'enum vs strings' where's the pros of the string usage?
(Say, at present to the question -Which format does the image have?- I see the overhead in terms of string comparisons and have no clue -how to avoid it-).

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Jun 23, 2017

Contributor

@Turim You're more than welcome to try whatever you think will work well. I'd like to stick with the approach that's in this PR because the Decoder and Encoder classes do the same work that was being done before and offer the user a relatively clean interface.

Contributor

brian-armstrong-discord commented Jun 23, 2017

@Turim You're more than welcome to try whatever you think will work well. I'd like to stick with the approach that's in this PR because the Decoder and Encoder classes do the same work that was being done before and offer the user a relatively clean interface.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Sep 20, 2017

Contributor

@alalek Do you think there's any hope for this PR? I'm wondering where I should set my expectations.

Contributor

brian-armstrong-discord commented Sep 20, 2017

@alalek Do you think there's any hope for this PR? I'm wondering where I should set my expectations.

@sturkmen72

This comment has been minimized.

Show comment
Hide comment
@sturkmen72

sturkmen72 Nov 14, 2017

Contributor

i want to share my humble thoughts about this PR
i completely agree that there should be a function or class giving image information only reading file header.
when adding this functionality we also should keep in mind some related improvements
1.catching internal errors while loading images (see #3314)
2.when saving an image keeping EXIF information or adding new EXIF tags

personally i think a new class using existing decoders and encoders internally can be a good candidate to the solution

Contributor

sturkmen72 commented Nov 14, 2017

i want to share my humble thoughts about this PR
i completely agree that there should be a function or class giving image information only reading file header.
when adding this functionality we also should keep in mind some related improvements
1.catching internal errors while loading images (see #3314)
2.when saving an image keeping EXIF information or adding new EXIF tags

personally i think a new class using existing decoders and encoders internally can be a good candidate to the solution

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Nov 14, 2017

Contributor

Sorry for huge delay!

Current interface is very questionable (for example, setScale(), orientation() and nextPage() methods). Such interfaces should be polished accurately:

  • minimal "required" set of parameters.
  • easily extendable interface
  • usability (construct image decoder, call readHeader(), check result and then maybe get image size via width and height - this long story we should to hide from users)

Perhaps, we could start with simple set of functions, for example:

/** Query image parameters

@sa cv::imread

Throws exception if image is not supported.

@param filename Name of file to be loaded
@param flags Flag that can take values of cv::ImreadModes
@param[out] type (optional) type of decoded cv::Mat returned by cv::imtead
@param[out] decoderName (optional) name of used image decoder for cv::imread
@returns size of image
*/
Size imquery(const cv::String& filename, int flags = IMREAD_COLOR,
                CV_OUT int* type = NULL, CV_OUT cv::String* decoderName = NULL);

I believe it is able to handle declared set of problems from the PR description.

Contributor

alalek commented Nov 14, 2017

Sorry for huge delay!

Current interface is very questionable (for example, setScale(), orientation() and nextPage() methods). Such interfaces should be polished accurately:

  • minimal "required" set of parameters.
  • easily extendable interface
  • usability (construct image decoder, call readHeader(), check result and then maybe get image size via width and height - this long story we should to hide from users)

Perhaps, we could start with simple set of functions, for example:

/** Query image parameters

@sa cv::imread

Throws exception if image is not supported.

@param filename Name of file to be loaded
@param flags Flag that can take values of cv::ImreadModes
@param[out] type (optional) type of decoded cv::Mat returned by cv::imtead
@param[out] decoderName (optional) name of used image decoder for cv::imread
@returns size of image
*/
Size imquery(const cv::String& filename, int flags = IMREAD_COLOR,
                CV_OUT int* type = NULL, CV_OUT cv::String* decoderName = NULL);

I believe it is able to handle declared set of problems from the PR description.

@Turim

This comment has been minimized.

Show comment
Hide comment
@Turim

Turim Nov 15, 2017

Contributor

@alalek , IMHO

  • nextPage() can't be ignored as just redundand for nowadays we're able to handle multipage TIFF images;
  • I hope imageHeader/width and imageHeader/height still can be a subject for a discussion;

I think the case could be mantained for the imgcodecs to be more clean. Nowadays the only what a imgcodecs' user can is try to load the image via, say, imread and check the retval. But in the prod code it's just sort of gambling with user's memory (i.e. working set) - every try to load image, say, 10000x10000 or even bigger leads to a memory consumption peak with empty retval.

Am I right that you prefer not to add struct/classes?
The another way to mantain the case could be code like that:

/**
@param filename Name of file to be loaded
@param flags Flag that can take values of cv::ImreadModes
@param[out] type (optional) type of decoded cv::Mat returned by cv::imread
@param[out] width (optional) width of decoded cv::Mat returned by cv::imread
@param[out] height (optional) width of decoded cv::Mat returned by cv::imread
*/
CV_EXPORTS_W bool imreadheader(const String& filename, int flags, 
                                CV_OUT int* type = NULL, CV_OUT int* width = NULL, CV_OUT int* height = NULL);

/**
^like the previous
*/
struct ImageSize // or pair_of_int or tuple_of_int
{
int width;
int height;
};
CV_EXPORTS_W bool imreadheadermulti(const String& filename, int flags, 
                                CV_OUT int* type = NULL, CV_OUT std::vector<ImageSize>* sizes = NULL);

As for mentioned imquery() - yes, it gives you the option to distinguish JPEG from TIFF, but the image size is an open question still - so you basically don't know if there any way to load it.

P.S. @brian-armstrong-discord I apologize for I've just mentioned yet another interface for the case. Quite unsure should I create a separate discussion for that. Feel free to provide the feedback on that.
P.P.S. About 'to throw or not to throw' - I'm pretty unsure that it has any connection to the iface. To enable throwing means just to break the backward compatibility and previously written error handling.
P.P.P.S. Yup, both imreadheader() and imquery() aren't so good regarding the perf - on imread() you eventually reread the header. I think we can't fix that without classes/output_token_parameters which could use the state. But even with current perf restrictions, it's good just to have functions like those in the imgcodecs interface.

Contributor

Turim commented Nov 15, 2017

@alalek , IMHO

  • nextPage() can't be ignored as just redundand for nowadays we're able to handle multipage TIFF images;
  • I hope imageHeader/width and imageHeader/height still can be a subject for a discussion;

I think the case could be mantained for the imgcodecs to be more clean. Nowadays the only what a imgcodecs' user can is try to load the image via, say, imread and check the retval. But in the prod code it's just sort of gambling with user's memory (i.e. working set) - every try to load image, say, 10000x10000 or even bigger leads to a memory consumption peak with empty retval.

Am I right that you prefer not to add struct/classes?
The another way to mantain the case could be code like that:

/**
@param filename Name of file to be loaded
@param flags Flag that can take values of cv::ImreadModes
@param[out] type (optional) type of decoded cv::Mat returned by cv::imread
@param[out] width (optional) width of decoded cv::Mat returned by cv::imread
@param[out] height (optional) width of decoded cv::Mat returned by cv::imread
*/
CV_EXPORTS_W bool imreadheader(const String& filename, int flags, 
                                CV_OUT int* type = NULL, CV_OUT int* width = NULL, CV_OUT int* height = NULL);

/**
^like the previous
*/
struct ImageSize // or pair_of_int or tuple_of_int
{
int width;
int height;
};
CV_EXPORTS_W bool imreadheadermulti(const String& filename, int flags, 
                                CV_OUT int* type = NULL, CV_OUT std::vector<ImageSize>* sizes = NULL);

As for mentioned imquery() - yes, it gives you the option to distinguish JPEG from TIFF, but the image size is an open question still - so you basically don't know if there any way to load it.

P.S. @brian-armstrong-discord I apologize for I've just mentioned yet another interface for the case. Quite unsure should I create a separate discussion for that. Feel free to provide the feedback on that.
P.P.S. About 'to throw or not to throw' - I'm pretty unsure that it has any connection to the iface. To enable throwing means just to break the backward compatibility and previously written error handling.
P.P.P.S. Yup, both imreadheader() and imquery() aren't so good regarding the perf - on imread() you eventually reread the header. I think we can't fix that without classes/output_token_parameters which could use the state. But even with current perf restrictions, it's good just to have functions like those in the imgcodecs interface.

@alalek

This comment has been minimized.

Show comment
Hide comment
@alalek

alalek Nov 15, 2017

Contributor

but the image size is an open question still

In case of imquery proposal - size is return value of mentioned function.

multipage TIFF

I prefer to handle the most universal cases and I prefer not to add some rarely used hacks (like setScale() or readNext()) into widely used interfaces.
Image is a single surface of pixels (check "wiki"-like definitions).
If you want to work with multipage images - welcome to VideoCapture interface, which is initially designed to handle set of images, including cases of different frames resolutions.

Throw

Check amount of useful information between "bool" result and "exception" with message and potentially stack-trace to the reason of problem.

Contributor

alalek commented Nov 15, 2017

but the image size is an open question still

In case of imquery proposal - size is return value of mentioned function.

multipage TIFF

I prefer to handle the most universal cases and I prefer not to add some rarely used hacks (like setScale() or readNext()) into widely used interfaces.
Image is a single surface of pixels (check "wiki"-like definitions).
If you want to work with multipage images - welcome to VideoCapture interface, which is initially designed to handle set of images, including cases of different frames resolutions.

Throw

Check amount of useful information between "bool" result and "exception" with message and potentially stack-trace to the reason of problem.

@Turim

This comment has been minimized.

Show comment
Hide comment
@Turim

Turim Nov 15, 2017

Contributor

@alalek , thanks for clarifying.

size as the retval

You're correct, my mistake.

I prefer not to add some rarely used hacks

AFAIK imreadmulti() supports it and, I presume, it's the only goal.
So we're able to read multiple images, but can get the Size only for the 1st - I presume - the more clean would be to add imquerymulti() or to deprecate imreadmulti().

Contributor

Turim commented Nov 15, 2017

@alalek , thanks for clarifying.

size as the retval

You're correct, my mistake.

I prefer not to add some rarely used hacks

AFAIK imreadmulti() supports it and, I presume, it's the only goal.
So we're able to read multiple images, but can get the Size only for the 1st - I presume - the more clean would be to add imquerymulti() or to deprecate imreadmulti().

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Apr 25, 2018

Contributor

Should I go ahead and just abandon this? The imquery approach isn't very appetizing, and if this isn't mergeable, we might as well kill it.

Contributor

brian-armstrong-discord commented Apr 25, 2018

Should I go ahead and just abandon this? The imquery approach isn't very appetizing, and if this isn't mergeable, we might as well kill it.

@brian-armstrong-discord

This comment has been minimized.

Show comment
Hide comment
@brian-armstrong-discord

brian-armstrong-discord Aug 1, 2018

Contributor

Are there any plans to add something like imquery? We'd love to get off this fork, especially now that opencv has changed substantially.

Contributor

brian-armstrong-discord commented Aug 1, 2018

Are there any plans to add something like imquery? We'd love to get off this fork, especially now that opencv has changed substantially.

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