Retina module interface update by albenoit · Pull Request #861 · opencv/opencv

albenoit · 2013-04-30T12:56:08Z

Following OpenCV classes interfaces design, here are the changes:
_updated Retina class interface : now a cleaner abstract class
_updated doc (refined and completed)

Old paths can have problems with cross-compilation

OpenCV Manager verison incremeneted; Docs and tests updated accordingly; COnstant for Manager initialization added.

…ut_ui

http://code.opencv.org/issues/2926

sample code : https://github.com/Itseez/opencv/blob/master/samples/cpp/tutorial_code/features2D/SURF_descriptor.cpp Bug opencv#2888

…Info()

…in eclipse

…DMatch)

…torials

Retina devel newinterface

Retina devel

…avy retinacolor process

vpisarev · 2013-05-02T20:27:51Z

Hi Alex,

excellent work! thanks a lot, the API looks much cleaner now!
The only issue I see is the use of "Mat&" in some "get*" methods, e.g. "virtual void getParvo(Mat& p) const = 0;"
Such a form is not "mainstream" in the modern OpenCV coding conventions.
If the output matrix is assigned, then the better form is "virtual Mat getParvo() const = 0;"
If the data is copied, the preferable form is "virtual void getParvo(OutputArray p) const = 0;"
Both these forms are automatically wrappable in Python, Java etc.
With "virtual void getParvo(Mat& p) const = 0;" it's not clear whether the matrix is just output parameter or it's both input and output parameter.

Regards,
Vadim

albenoit · 2013-05-03T05:50:05Z

Hi Vadim,
Thanks for the remarks.
I will do the suggested changes in a few time to comply with the OpenCV
coding convention.
I will push a new version on my current pull request... and also do very
minor updates on the documentation.

Regards.
Alex

2013/5/2 Vadim Pisarevsky notifications@github.com

Hi Alex,

excellent work! thanks a lot, the API looks much cleaner now!
The only issue I see is the use of "Mat&" in some "get*" methods, e.g.
"virtual void getParvo(Mat& p) const = 0;"
Such a form is not "mainstream" in the modern OpenCV coding conventions.
If the output matrix is assigned, then the better form is "virtual Mat
getParvo() const = 0;"
If the data is copied, the preferable form is "virtual void
getParvo(OutputArray p) const = 0;"
Both these forms are automatically wrappable in Python, Java etc.
With "virtual void getParvo(Mat& p) const = 0;" it's not clear whether the
matrix is just output parameter or it's both input and output parameter.

Regards,
Vadim

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-17362584
.

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

albenoit · 2013-05-03T07:38:55Z

Hi again Vadim,

well, regarding your suggestions : replace Mat by OutputArray for data copy
accessors ex "virtual void getParvo(OutputArray p) const = 0;"

I also want to replace
void RetinaImpl::run(const Mat inputMatToConvert)
by
void RetinaImpl::run(InputArray inputMatToConvert)

But in both cases, methods that were usable with Mat :
_ Mat accessors such as 'at' in my void
RetinaImpl::convertValarrayBuffer2cvMat method.
_ casts : 'Mat<Vec<T, 4> >(inputMatToConvert)' in my bool
RetinaImpl::_convertCvMat2ValarrayBuffer method

are no more accessible but where usefull for Mat and valarray data exchange.

Then, is there a nice way to use Mat objects within RetinaImpl class while
showing on the interface InputArray and OutputArray with efficient methods
(no buffer copies).

Thank you and sorry for all the time spent.
Regards
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi Vadim,
Thanks for the remarks.
I will do the suggested changes in a few time to comply with the OpenCV
coding convention.
I will push a new version on my current pull request... and also do very
minor updates on the documentation.

Regards.
Alex

2013/5/2 Vadim Pisarevsky notifications@github.com

Hi Alex,

excellent work! thanks a lot, the API looks much cleaner now!
The only issue I see is the use of "Mat&" in some "get*" methods, e.g.
"virtual void getParvo(Mat& p) const = 0;"
Such a form is not "mainstream" in the modern OpenCV coding conventions.
If the output matrix is assigned, then the better form is "virtual Mat
getParvo() const = 0;"
If the data is copied, the preferable form is "virtual void
getParvo(OutputArray p) const = 0;"
Both these forms are automatically wrappable in Python, Java etc.
With "virtual void getParvo(Mat& p) const = 0;" it's not clear whether
the matrix is just output parameter or it's both input and output parameter.

Regards,
Vadim

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-17362584
.

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

… (const Mat&)

albenoit · 2013-05-03T09:47:18Z

Hi Vadim,
sorry for the previous email with bad questions... i forgot you have
a marvelous Wiki with all the required answers !
then, i did the changes you proposed. It works on my machine, i did a push
to validate on the buildbot.

Stay tuned.
Regards.
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi again Vadim,

well, regarding your suggestions : replace Mat by OutputArray for data
copy accessors ex "virtual void getParvo(OutputArray p) const = 0;"

I also want to replace
void RetinaImpl::run(const Mat inputMatToConvert)
by
void RetinaImpl::run(InputArray inputMatToConvert)

But in both cases, methods that were usable with Mat :
_ Mat accessors such as 'at' in my void
RetinaImpl::convertValarrayBuffer2cvMat method.
_ casts : 'Mat<Vec<T, 4> >(inputMatToConvert)' in my bool
RetinaImpl::_convertCvMat2ValarrayBuffer method

are no more accessible but where usefull for Mat and valarray data
exchange.

Then, is there a nice way to use Mat objects within RetinaImpl class while
showing on the interface InputArray and OutputArray with efficient methods
(no buffer copies).

Thank you and sorry for all the time spent.
Regards
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi Vadim,
Thanks for the remarks.
I will do the suggested changes in a few time to comply with the OpenCV
coding convention.
I will push a new version on my current pull request... and also do very
minor updates on the documentation.

Regards.
Alex

2013/5/2 Vadim Pisarevsky notifications@github.com

Hi Alex,

excellent work! thanks a lot, the API looks much cleaner now!
The only issue I see is the use of "Mat&" in some "get*" methods, e.g.
"virtual void getParvo(Mat& p) const = 0;"
Such a form is not "mainstream" in the modern OpenCV coding conventions.
If the output matrix is assigned, then the better form is "virtual Mat
getParvo() const = 0;"
If the data is copied, the preferable form is "virtual void
getParvo(OutputArray p) const = 0;"
Both these forms are automatically wrappable in Python, Java etc.
With "virtual void getParvo(Mat& p) const = 0;" it's not clear whether
the matrix is just output parameter or it's both input and output parameter.

Regards,
Vadim

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-17362584
.

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

albenoit · 2013-05-03T11:31:29Z

Hi again,

everything seems to rock now. Buildbot is green and i corrected as you
recommended.
If everything is ok, why not pushing to 2.4 branch to reduce release delays
since only Retina constructor is changed from a user point of view. But you
better know what to do than me.

For Gsoc, as i told you, not this year but next year, it can be great if
there are some opportunities ! How long mentors should stay there ?
See you soon.
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi Vadim,
sorry for the previous email with bad questions... i forgot you have
a marvelous Wiki with all the required answers !
then, i did the changes you proposed. It works on my machine, i did a push
to validate on the buildbot.

Stay tuned.
Regards.
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi again Vadim,

well, regarding your suggestions : replace Mat by OutputArray for data
copy accessors ex "virtual void getParvo(OutputArray p) const = 0;"

I also want to replace
void RetinaImpl::run(const Mat inputMatToConvert)
by
void RetinaImpl::run(InputArray inputMatToConvert)

But in both cases, methods that were usable with Mat :
_ Mat accessors such as 'at' in my void
RetinaImpl::convertValarrayBuffer2cvMat method.
_ casts : 'Mat<Vec<T, 4> >(inputMatToConvert)' in my bool
RetinaImpl::_convertCvMat2ValarrayBuffer method

are no more accessible but where usefull for Mat and valarray data
exchange.

Then, is there a nice way to use Mat objects within RetinaImpl class
while showing on the interface InputArray and OutputArray with efficient
methods (no buffer copies).

Thank you and sorry for all the time spent.
Regards
Alex

2013/5/3 alexandre benoit benoit.alexandre.vision@gmail.com

Hi Vadim,
Thanks for the remarks.
I will do the suggested changes in a few time to comply with the OpenCV
coding convention.
I will push a new version on my current pull request... and also do very
minor updates on the documentation.

Regards.
Alex

2013/5/2 Vadim Pisarevsky notifications@github.com

Hi Alex,

excellent work! thanks a lot, the API looks much cleaner now!
The only issue I see is the use of "Mat&" in some "get*" methods, e.g.
"virtual void getParvo(Mat& p) const = 0;"
Such a form is not "mainstream" in the modern OpenCV coding conventions.
If the output matrix is assigned, then the better form is "virtual Mat
getParvo() const = 0;"
If the data is copied, the preferable form is "virtual void
getParvo(OutputArray p) const = 0;"
Both these forms are automatically wrappable in Python, Java etc.
With "virtual void getParvo(Mat& p) const = 0;" it's not clear whether
the matrix is just output parameter or it's both input and output parameter.

Regards,
Vadim

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-17362584
.

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

vpisarev · 2013-05-03T20:12:48Z

pushing to 2.4 is impossible, unfortunately, because the changes break binary compatibility (not sure about source-level compatibility). In 2.4 we only accept changes that preserve binary compatibility. And this is one of the goals of the new API design style in 3.0 - to minimize effect on the header when implementation is changed.

That is, if some of the changes in pull requests are bug fixes and other local improvements that do not affect headers in any way, a separate pull request for 2.4 should be created. This pull request should definitely go to 3.0

albenoit · 2013-05-04T06:37:58Z

Hi Vadim,
Ok, that's fine, let's target API 3.0 as initially planned. In some time i
will do some more push still on the AP1 3.0 for namespace change and
memory footprint reduction.
Thanks for your help.
Alex

2013/5/3 Vadim Pisarevsky notifications@github.com

pushing to 2.4 is impossible, unfortunately, because the changes break
binary compatibility (not sure about source-level compatibility). In 2.4 we
only accept changes that preserve binary compatibility. And this is one of
the goals of the new API design style in 3.0 - to minimize effect on the
header when implementation is changed.

That is, if some of the changes in pull requests are bug fixes and other
local improvements that do not affect headers in any way, a separate pull
request for 2.4 should be created. This pull request should definitely go
to 3.0

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/861#issuecomment-17415556
.

Alexandre BENOIT,
Assistant Professor / Maître de Conférence
Image processing and visual scene classification,
LISTIC Lab / IUT Annecy
benoit.alexandre.vision.googlepages.com

vpisarev · 2013-05-07T12:37:58Z

👍

Andrey Kamaev and others added 30 commits April 4, 2013 15:43

Fix binary directory used in CMake try_compile commands

fa64f28

Old paths can have problems with cross-compilation

OpenCV verison incremented

1e332d6

OpenCV Manager verison incremeneted; Docs and tests updated accordingly; COnstant for Manager initialization added.

Merge pull request opencv#753 from taka-no-me:fix_osx_fat_java

e22983d

Merge pull request opencv#755 from taka-no-me:add_checkinclude

6a0f605

Merge pull request opencv#756 from taka-no-me:libavcodec53.25.0

6f28861

Merge pull request opencv#758 from taka-no-me:do_not_use_opengl_witho…

e7885bb

…ut_ui

Merge pull request opencv#759 from asmorkalov:android_common_fixes

7c402d1

Merge pull request opencv#762 from vpisarev:cvt_fix

579f723

Merge pull request opencv#764 from taka-no-me:svd

1d7ff72

Merge pull request opencv#766 from taka-no-me:try_compile_fix

c34820b

Merge pull request opencv#767 from asmorkalov:version_increment

6570215

Merge pull request opencv#757 from jackculpepper:master

bfac291

Merge pull request opencv#763 from gpsinghsandhu:video-doc-minor-typo

3785439

Build warning fixes.

a914088

Merge pull request opencv#760 from alekcac:2.4

80fed87

Merge pull request opencv#770 from asmorkalov:winrt

652606f

changed surfFeaturesFinder::find() to allow CV_8UC1 type images

2b1e13f

http://code.opencv.org/issues/2926

some typo

69127e4

Feature description tutorials made in sync with sample code

d51d05f

sample code : https://github.com/Itseez/opencv/blob/master/samples/cpp/tutorial_code/features2D/SURF_descriptor.cpp Bug opencv#2888

another update

87b84a4

fix all redefine build errors on some Intel OCL

bcc086b

move the "cpu device checking" from supportsFeatures() to queryDevice…

5022bc8

…Info()

Fix the problem of device selection on hybrid video systems.

8cc5b98

Catch exceptions when large allocations fail

64b5784

copying '.classpath' and '.project' to build dir, useful for opening …

18b2d6b

…in eclipse

test for the issue opencv#2901

09fe5cd

fix for opencv#2901 (en exception was raised when getting empty MatOf…

4c31c26

…DMatch)

Workaround hanging of ocl module when ocl API in not really used

3400d83

Merge pull request opencv#772 from gpsinghsandhu:featureDescriptionTu…

5163986

…torials

Waning fixes for Android samples

f89cc19

albenoit added 17 commits April 27, 2013 21:22

forgot method name update

3b15eef

activated parallelized gradient computation

7370469

Merge pull request #1 from albenoit/retina_devel_newinterface

a4bc167

Retina devel newinterface

Merge pull request #2 from albenoit/retina_devel

9e1ce1e

Retina devel

updated comments and new retina interface use

16dea7b

minor chnges for conflict removal

9549949

resolved conflicts, updated retina class interface and optimized a he…

72742f5

…avy retinacolor process

updated doc

1976b3d

corrections required for portability. To be validated by buildbot

b70b5a3

updated doc and interface code to solve compile warnings and errors

80e65e0

minor error correction

a659e28

updated/corrected documentation

25b482f

updated sources to correct buildbot warnings

9c26387

updated doc+minor typo/buildbot reports correction

9adde75

trying to solve valarray pointer access problem, enhancing documentation

cf6e833

enhancing documentation

f5d2d7b

enhancing documentation

b58d9f1

nicer interface with the use of InputArray and OutptArray instated of…

a5acc9e

… (const Mat&)

ghost assigned vpisarev May 3, 2013

vpisarev added a commit that referenced this pull request May 12, 2013

Merge pull request #861 from albenoit:master

f1e8f69

opencv-pushbot merged commit a5acc9e into opencv:master May 12, 2013

savuor pushed a commit to nickyu-zhu/opencv that referenced this pull request Oct 27, 2023

Merge pull request opencv#861 from SamFC10:sam-darknet

e7fb489

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Retina module interface update#861

Retina module interface update#861
opencv-pushbot merged 2166 commits intoopencv:masterfrom
albenoit:master

albenoit commented Apr 30, 2013

Uh oh!

vpisarev commented May 2, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

vpisarev commented May 3, 2013

Uh oh!

albenoit commented May 4, 2013

Uh oh!

vpisarev commented May 7, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

Uh oh!

Conversation

albenoit commented Apr 30, 2013

Uh oh!

vpisarev commented May 2, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

albenoit commented May 3, 2013

Uh oh!

vpisarev commented May 3, 2013

Uh oh!

albenoit commented May 4, 2013

Uh oh!

vpisarev commented May 7, 2013

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants