kyamagu / mexopencv

Collection and a development kit of matlab mex functions for OpenCV library
http://kyamagu.github.io/mexopencv
Other
659 stars 318 forks source link

Issue in make for 2018a (ver. 9.4) #429

Open mmiozzi opened 5 years ago

mmiozzi commented 5 years ago

The second logic condition in row 280 if ~mexopencv.isOctave() && ~verLessThan('matlab', '9.4') should be changed in if ~mexopencv.isOctave() && verLessThan('matlab', '9.4') otherwise the option mex_flags = ['-R2017b' mex_flags]; still persists in 2018a (ver. 9.4)

amroamroamro commented 5 years ago

As the comment in the code suggests, that's the intended behavior:

    % real/imaginary storage format for complex arrays
    if ~mexopencv.isOctave() && ~verLessThan('matlab', '9.4')
        % keep using the "separate complex storage", as opposed to the
        % "interleaved complex storage" introduced in R2018a
        % (see MX_HAS_INTERLEAVED_COMPLEX)
        mex_flags = ['-R2017b' mex_flags];
    end

We want to pass the -R2017b flag for releases >= R2018a. Currently the mex command uses this flag by default, but the default might change in future versions, hence we explicitly set it as suggested in MATLAB docs:

https://www.mathworks.com/help/matlab/matlab_external/do-i-need-to-upgrade-my-mex-files-to-use-interleaved-complex.html

Either way we don't really use "complex arrays" in mexopencv, and we expect all inputs to be real anyway.

amroamroamro commented 5 years ago

Are you getting an error because of the flag?

amroamroamro commented 5 years ago

Oh I see now, I'm getting an error in R2018b when I run this:

>> mex -R2017b -largeArrayDims testmex.cpp
Error using mex
Calling MEX with both 'R2017b' and 'largeArrayDims' not supported. 

(same thing with mex -R2018a -largeArrayDims)

It seems mex treats these flags as mutually exclusive! It's weird because they're really not, and -largeArrayDims is the default anyway..

I'll have to change make.m to set one of the two flags based on the version:

MLofgrenWork commented 5 years ago

In addition to amroamroamro's last comment, I note that '-R2017b' (on line 280 of make.m) needs a space at the end of the string so that there is a space between this and the next flag.