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

Add support for FlyCapture 2 C and C++ API. #6

Merged
merged 3 commits into from
Jul 19, 2014

Conversation

jpsacha
Copy link
Member

@jpsacha jpsacha commented Jun 14, 2014

Lightly tested with Java 7 and Windows x64 - there are no know issues at present. SDK initialization, camera initialization and querying cameras properties works. .

There are some hacks in presets/FlyCapture2_C.java to deal with fc2Property and fc2PropertyInfo not being generated by the Parser (from typedef struct), see line 525 and 545 in C:\Program Files\Point Grey Research\FlyCapture2\include\C\FlyCapture2Defs_C.h.

Theoretically the wrappers can be also created for Linux.

@saudet
Copy link
Member

saudet commented Jun 15, 2014

Before we go further with FlyCapture, I've just remembered that the copyright in the header files did not allow me to publish the processed results, under the GPL + exception for example. I got permission from PGR just to make sure, and it was fine back then, but I think we'll need to do the same for the new set of header files as well...

@saudet
Copy link
Member

saudet commented Jun 26, 2014

Could you update your branch with the changes for the C++ API as well so I can check out that problem with the enum ErrorType? Thanks!

@jpsacha
Copy link
Member Author

jpsacha commented Jun 26, 2014

C++ API is on separate branch: flycapture2cpp. I did not do PR for current flycapture2cpp (C++ API) since it does not compile due to enum ErrorType errors. For testing C++ API please checkout flycapture2cpp from my fork.

It was my mistake originally not to put flycapture and flandmark on a separate branches. This way I could have done a PR request for flandmark separately from flycapture. I was waiting for you to accept first PR for flycapture C API, then I would do a separate PR for flandmark (with separate branches I would not have to wait).

@saudet
Copy link
Member

saudet commented Jun 26, 2014

I see, I'll go take a look there then!

If we don't encounter too many problems though, I'd like to accept the C and C++ API together. What do you think? I'll be checking the C++ API out soon.

BTW, we can still modify this branch to contain whatever you want... You'll need to squash things to get only one commit in there eventually though.

@jpsacha
Copy link
Member Author

jpsacha commented Jun 26, 2014

Do whatever is more convenient for you. My intention to keep things separate, especially between flycapture and flandmark, was to help you merge changes into the main repo,

At the moment I do not have much more thing related to flycapture. I was planing to concentrate on C++ API. Once the issue with enum TypeError is resolved I will start wrapping and testing the rest of C++ API.

There are some "hacks" in presets / FlyCapture2_C.java l.59 and l.61 to deal with some struct wrappers not being generated (for fc2Property and fc2PropertyInfo), I am not clear if that is due really to Java limitations (not able to create "aliases" for structs that are identical). Can you take a look at them too?

@saudet
Copy link
Member

saudet commented Jun 28, 2014

You could create another PR for flandmark, and if you're satisfied with its state, I'll merge it!

I fixed the issues that were causing problems with the ErrorType here:
bytedeco/javacpp@dd14511

Java doesn't support something like typedef in C, and I'm not sure how to handle those situations automatically. In this case, it seems most appropriate to use @Cast and @Name annotations to at least keep the association visible to the programmer. We can get that effect with these two lines:

    .put(new Info("fc2TriggerDelayInfo").cast().pointerTypes("fc2PropertyInfo"))
    .put(new Info("fc2TriggerDelay").cast().pointerTypes("fc2Property"))

(BTW, according to Java Code Conventions, when breaking lines, commas are left at the end of lines, but other operators including dots are placed at the beginning.)

@saudet saudet mentioned this pull request Jul 8, 2014
@jpsacha
Copy link
Member Author

jpsacha commented Jul 13, 2014

Changes to javacpp fixed issue with enum TypeError. I added remaining FlyCapture headers, got stuck on another error:

jniFlyCapture2.cpp(15879) : error C2039: 'CallbackHandle' : is not a member of '`global namespace''

I am not clear how to proceed. Can you take a look? The code is on my fork on branch flycapture2cpp:
https://github.com/jpsacha/javacpp-presets/tree/c47eeb3d2e3a1f177bb805881b12433ddeb8e1a3

@saudet
Copy link
Member

saudet commented Jul 13, 2014

Indeed, there's a @Namespace missing inside the Parser.typedef() method. I haven't tested it yet, but can you try to replace the corresponding portion with the following?

            // some opaque data type
            Info info = infoMap.getFirst(defName);
            if (info == null || !info.skip) {
                decl.text = "@Opaque ";
                if (context.namespace != null && context.group == null) {
                    decl.text += "@Namespace(\"" + context.namespace + "\") ";
                }
                decl.text += "public static class " + dcl.javaName + " extends Pointer {\n" +
                             "    public " + dcl.javaName + "() { }\n" +
                             "    public " + dcl.javaName + "(Pointer p) { super(p); }\n" +
                             "}";
            }

Thanks!

@jpsacha
Copy link
Member Author

jpsacha commented Jul 13, 2014

That change to Parser.typedef() worked. I got some link issue now, will be looking into that later.

@jpsacha
Copy link
Member Author

jpsacha commented Jul 13, 2014

@saudet
Copy link
Member

saudet commented Jul 13, 2014

Ok, it looks good. There's a few small things we could fix:

  • Include your name in the copyright at the top of the files
  • Update the version in the pom.xml file
  • Make the coding style more similar to the rest of the code base

But those are things I can fix myself when I adjust it and test it for Linux anyway.

So, squash all those changes into one commit in the branch of this PR and I'll be happy to merge it 👍 Thanks!

@jpsacha
Copy link
Member Author

jpsacha commented Jul 15, 2014

I will prepare updated PR with the latest version.

BTW: I added a simple examples of using FlyCapture 2 wrapper to javacv-examples on develop branch. Basic tests with camera look fine.

@jpsacha
Copy link
Member Author

jpsacha commented Jul 15, 2014

This PR is superseded by PR #10.

@jpsacha jpsacha closed this Jul 15, 2014
@jpsacha jpsacha reopened this Jul 15, 2014
@jpsacha
Copy link
Member Author

jpsacha commented Jul 15, 2014

Reopened with rebased and squashed changes, as requested. Note: flandmark commits are excluded, so this is only FlyCapture 2 stuff, including its C++ API.

@saudet
Copy link
Member

saudet commented Jul 19, 2014

Strange, some of my old commits are showing up in your PR. Is this normal?

@jpsacha
Copy link
Member Author

jpsacha commented Jul 19, 2014

I do not know if that is normal. Note that those two commits were made after the initial PR. I did rebasing on my master branch using command

git rebase -i origin/master

Those two commits showed up there. I squished all my commits, but left the two unchanged. The two commits were initially merged into my master, and they were needed due to changes in relative paths in cppbuild. During rebasing the merge disappeared and the commits were left, so effectively in PR those two commits are only once (no duplicates). In my local repo, the second commit is shown as the head of origin/master (I did not pull your commits from today yet):

image

Nothing unusual there, it just looks strange in GitHub.

@saudet saudet changed the title Add initial support for FlyCapture v.2 C API. Add support for FlyCapture 2 C and C++ API. Jul 19, 2014
saudet added a commit that referenced this pull request Jul 19, 2014
Add support for FlyCapture 2 C and C++ API.
@saudet saudet merged commit e8f8f46 into bytedeco:master Jul 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants