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

Exported a high level stitcher for Scans to the DLL #10681

Merged
merged 1 commit into from
Jan 26, 2018

Conversation

hmaarrfk
Copy link
Contributor

allows Stitcher to be used for scans from within python.
I had to use very strange notation because I couldn't export the enum
Mode making the Cpython generated code unable to compile.

class Stitcher {
public:
enum Mode
    {
        PANORAMA = 0,
        SCANS = 1,
    };
...

Also removed duplicate code from the createStitcher function making
use of the Stitcher::create function

This pullrequest changes

allows Stitcher to be used for scans from within python.
I had to use very strange notation because I couldn't export the `enum`
`Mode` making the Cpython generated code unable to compile.

```c++
class Stitcher {
public:
enum Mode
    {
        PANORAMA = 0,
        SCANS = 1,
    };
...
```

Also removed duplicate code from the `createStitcher` function making
use of the `Stitcher::create` function
@terfendail
Copy link
Contributor

terfendail commented Jan 26, 2018

👍

@hmaarrfk
Copy link
Contributor Author

@terfendail is there a way to expose the enums to the python interface? If so, that would make it much easier to create these interfaces. To my knowledge, the only reason the function createStitcher exists is because python has no knowledge of the underlying enums

@hrnr
Copy link
Contributor

hrnr commented Jan 26, 2018

Hi,

it is possible and quite easy see https://github.com/opencv/opencv/blob/master/modules/stitching/misc/python/pyopencv_stitching.hpp

I agree with you that it would be much better to export the enum and create function, rather than creating yet another auxiliary function to create a Stitcher. There are already too many ways to create a Stitcher, it is a mess. I hope it will be cleaned in OpenCV 4.0.

Sorry for being late, but you were quite fast with merging this. I think we can still remove the new function, since there has been no release, right?

@hmaarrfk
Copy link
Contributor Author

@hrnr Thanks for explaining.

I've used SWIG before to export my C++ interfaces to Python and never actually created templates myself. Thanks for showing me the example.

I personally think the old function should be removed too, I just couldn't find an example quickly. I'll try to implement the change, compile and update the PR.

@hrnr
Copy link
Contributor

hrnr commented Jan 26, 2018

As far as I know OpenCV uses custom build binding generator and not SWIG. You can find it in the python module if you are interested.

@hmaarrfk
Copy link
Contributor Author

Hi @hrnr

I added

typedef Stitcher::Mode Mode;

template<>
PyObject* pyopencv_from(const Mode& value)
{
    return PyInt_FromLong(value);
}

but now I get

12>C:\Users\mark\git\opencv\modules\python\src2\cv2.cpp(1855): error C2129: static function 'bool pyopencv_to<Mode>(PyObject *,T &,const char *)' declared but not defined
12>        with
12>        [
12>            T=Mode
12>        ]

Is there an easy way to generate that function without having to manually add the keys to a string compare dictionary?

@hrnr
Copy link
Contributor

hrnr commented Jan 28, 2018

I'm not an expert in OpenCV python bindings, maybe @alalek knows how to do this properly?

@terfendail
Copy link
Contributor

@hmaarrfk Could you please check the change with latest master?
I've tried it and it compile fine at least for my configuration. Also it looks like it would be enough to have only
typedef Stitcher::Mode Mode;

However at the moment such typedefs are added to cv2 namespace so it could became cluttered quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants