bytedeco / javacpp-presets

The missing Java distribution of native C++ libraries
Other
2.65k stars 737 forks source link

What to do when an API depends on a large sub-API? #520

Open beligum opened 6 years ago

beligum commented 6 years ago

Hi,

I'm confronted with a library (the Linux API for The Imaging Source cameras) that depends on GStreamer (that in turn depends on GLib). When writing a preset for it, the API complains it can't find the GstElement class, which is part of the GStreamer framework.

What's the appropriate way to deal with these situations? Should I dig deep and create a preset for the entire GStreamer (and Glib?) API first, or do I have other options?

best,

b.

saudet commented 6 years ago

You could, but if it remains hidden behind, you could just build it as part of Imaging Source and not map the API.

saudet commented 6 years ago

We can assume that most Linux distros have GLib and GStreamer installed by default tough.

beligum commented 6 years ago

Yeah, that's what I thought, but one the main methods simply returns a GstElement object, so it's kind of part of the core API. I'll see if there's a way to work around it though...

saudet commented 6 years ago

Ah, then just wrap it as "Pointer" and pass it around opaquely.

beligum commented 6 years ago

I'll give that a try. Thing is I'll also need access to a bit of the fields in the GstElement object at one point

saudet commented 6 years ago

The easiest thing to do is to consider that portion of the API of GStreamer as the API of the camera and map the couple of things you need alongside.

beligum commented 6 years ago

Check. Makes sense. I'll try that and see where I land..

beligum commented 6 years ago

Hi Samuel,

I'm getting there I think...

However, there's a weird quirk in my API though: at a certain point I need a pointer to a GstElement (GStreamer element). As I said before, I don't want to have to port the entire GStreamer API, so I found a project that's already done that.

Now, the people of the gst1-java-core project have used JNA to map the GStreamer C API to Java and at a certain point, my GstElement looks like this:

Element src = bin.getElementByName(ELEMENT_NAME_SRC);
com.sun.jna.Pointer srcPtr = src.getNativeAddress();

So instead of a org.bytedeco.javacpp.Pointer object, I have a com.sun.jna.Pointer one. In the end, I thought it's only an address in memory right, so I tried to "convert" it like this:

Pointer tcamProp = new Pointer(srcPtr.getByteBuffer(0, com.sun.jna.Pointer.SIZE));

(where the docs say that SIZE is "Size of a native pointer, in bytes.")

But that doesn't seem to work all that well. Would you know any way of turning the JNA pointer object into a JavaCPP Pointer object? Or am I completely on the wrong track?

b.

saudet commented 6 years ago

If you just need to map the GstElement struct, it's probably easier to just do that. No need to map the whole API.

beligum commented 6 years ago

Yeah, but how? I'm getting the the GstElement object like this:

String cmd =
                        "tcamsrc serial=" + this.cameraSerialNumber + " name=" + ELEMENT_NAME_SRC +
                        " ! video/x-bayer,width=" + WIDTH + ",height=" + HEIGHT + ",framerate=" + FPS_NUM + "/" + FPS_DEN +
                        " ! tcamautoexposure name=" + ELEMENT_NAME_AUTOEXPOSURE +
                        " ! tcamwhitebalance name=" + ELEMENT_NAME_WHITEBALANCE +
                        " ! bayer2rgb" +
                        " ! videoconvert" +
                        " ! videoscale" +
                        " ! capsfilter caps=video/x-raw,width=" + DISPLAY_WIDTH + ",height=" + DISPLAY_HEIGHT +
                        "";
Bin bin = Bin.launch(cmd, true);
Element src = bin.getElementByName(ELEMENT_NAME_SRC);

This basically boots up the entire GStreamer lib behind the scenes, resulting in the necessary Element. I don't really see another way of getting my hands on that object any other way...

saudet commented 6 years ago

The fact that you're using GStreamer doesn't mean that you have to map the whole API...

saudet commented 6 years ago

For example, we can declare the couple of structs and functions manually as I did for libdc1394 here: https://github.com/bytedeco/javacpp-presets/blob/master/libdc1394/src/main/java/org/bytedeco/javacpp/presets/dc1394.java#L56

beligum commented 6 years ago

Whow, never knew this kind of setup was possible... Interesting, I'll have a look. We should start writing docs ;-)

beligum commented 6 years ago

I'm getting there slowly, thanks for the example!

Can I ask you a quick question, though. I have this in the API to register a callback function.

typedef void (*shared_callback)(std::shared_ptr<tcam::MemoryBuffer>, void*);
typedef void (*sink_callback)(tcam::MemoryBuffer*, void*);
typedef void (*c_callback)(const struct tcam_image_buffer*, void*);

class ImageSink : public SinkInterface
{
    bool registerCallback (shared_callback, void*);
    bool registerCallback (sink_callback, void*);
    bool registerCallback (c_callback, void*);
}

I'm a bit confused how to handle it. It generates this compiler error:

,-z,noexecstack -Wl,-Bsymbolic -Wall -fPIC -shared -o libjniTISCamera.so -L/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/cppbuild/linux-x86_64/lib/ -Wl,-rpath,/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/cppbuild/linux-x86_64/lib/ -ltcam 
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp: In function ‘jboolean Java_org_bytedeco_javacpp_TISCamera_00024ImageSink_registerCallback__Lorg_bytedeco_javacpp_TISCamera_00024shared_1callback_2Lorg_bytedeco_javacpp_Pointer_2(JNIEnv*, jobject, jobject, jobject)’:
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:1084:90: error: no matching function for call to ‘registerCallback(void (*)(tcam::MemoryBuffer, void*), char*&)’
         bool rvalue = (bool)ptr->registerCallback((ptr0 == NULL ? NULL : ptr0->ptr), ptr1);
                                                                                          ^
In file included from /home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:99:0:
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/cppbuild/linux-x86_64/include/ImageSink.h:52:10: note: candidate: bool tcam::ImageSink::registerCallback(shared_callback, void*) <near match>
     bool registerCallback (shared_callback, void*);

Which basically means the typedef got lost (I think). Can you point me in the right direction?

best,

b.

beligum commented 6 years ago

Also, I'm running into C code that has public std::enable_shared_from_this<ImageSource> statements like this:

class ImageSource : public SinkInterface, public std::enable_shared_from_this<ImageSource>
{

public:

    ImageSource ();

    ~ImageSource ();

and I have no clue what to do with it :-(

saudet commented 6 years ago

For callbacks, you'll need to use the exact type. char* can't be used for void*.

std::enable_shared_from_this<ImageSource> looks like something we can ignore.

beligum commented 6 years ago

I'm a bit confused regarding the callbacks... The code that's generated is this:

public static class sink_callback extends FunctionPointer {
    static { Loader.load(); }
    /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */
    public    sink_callback(Pointer p) { super(p); }
    protected sink_callback() { allocate(); }
    private native void allocate();
    public native void call(MemoryBuffer arg0, Pointer arg1);
}
...
public native @Cast("bool") boolean registerCallback(sink_callback arg0, Pointer arg1);

But I suspect there's something wrong because i receive quite a lot of errors. I should probably add a void* pointer somewhere, no?

saudet commented 6 years ago

Well, looks like there is a bug in JavaCPP that needs to be fixed there...

beligum commented 6 years ago

Ah. Can I help? Let me know if you need a testdriver.

saudet commented 6 years ago

Actually we can use a char* there. The problem seems to be a missing * after MemoryBuffer:

error: no matching function for call to ‘registerCallback(void (*)(tcam::MemoryBuffer, void*), char*&)’

That indicates you've probably overridden something incorrectly with an Info...

saudet commented 6 years ago

For example, I have no issues building the following.

class MemoryBuffer {};
typedef void (*sink_callback)(MemoryBuffer*, void*);
bool registerCallback(sink_callback, void*);
import org.bytedeco.javacpp.*;
import org.bytedeco.javacpp.annotation.*;
import org.bytedeco.javacpp.tools.*;

@Properties(value = @Platform(include = "Sink.h"), target = "Sink")
public class SinkConfig extends InfoMap {
    public void map(InfoMap map) { }
}

If you could provide a minimal example that fails, it would help! Thanks

beligum commented 6 years ago

That one does work, indeed, but this one doesn't (at least not in my case):

typedef void (*shared_callback)(std::shared_ptr<MemoryBuffer>, void*);
bool registerCallback (shared_callback, void*);

Note that I have this mapping set though (for another reason):

infoMap.put(new Info("std::shared_ptr<tcam::MemoryBuffer>").annotations("@SharedPtr").pointerTypes("MemoryBuffer"));
beligum commented 6 years ago

I think I know what's going on... Is it possible the JavaCPP compiler treats these two method definitions as one and the same and gets confused?

typedef void (*shared_callback)(std::shared_ptr<MemoryBuffer>, void*);
bool registerCallback (shared_callback, void*);

typedef void (*sink_callback)(tcam::MemoryBuffer*, void*);
bool registerCallback (sink_callback, void*);
beligum commented 6 years ago

Note though that typedef void (*shared_callback)(std::shared_ptr<MemoryBuffer>, void*); is unparsable (Java generation works, c++ doesn't compile), so I guess that's still a JavaCPP bug. This is the output:

/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp: In function ‘void Java_org_bytedeco_javacpp_TISCamera_00024shared_1callback_call(JNIEnv*, jobject, jobject, jobject)’:
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:1268:35: error: could not convert ‘adapter0’ from ‘SharedPtrAdapter<tcam::MemoryBuffer>’ to ‘tcam::MemoryBuffer’
         (*ptr->ptr)(adapter0, ptr1);
                                   ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp: In member function ‘void JavaCPP_org_bytedeco_javacpp_TISCamera_00024shared_1callback::operator()(tcam::MemoryBuffer, void*)’:
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:1297:59: error: no matching function for call to ‘SharedPtrAdapter<tcam::MemoryBuffer>::SharedPtrAdapter(tcam::MemoryBuffer&)’
     SharedPtrAdapter< ::tcam::MemoryBuffer > adapter0(arg0);
                                                           ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:454:5: note: candidate: SharedPtrAdapter<T>::SharedPtrAdapter(const S*) [with T = tcam::MemoryBuffer; SharedPtrAdapter<T>::S = std::shared_ptr<tcam::MemoryBuffer>]
     SharedPtrAdapter(const S* sharedPtr) : ptr(0), size(0), owner(0), sharedPtr(*(S*)sharedPtr) { }
     ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:454:5: note:   no known conversion for argument 1 from ‘tcam::MemoryBuffer’ to ‘const S* {aka const std::shared_ptr<tcam::MemoryBuffer>*}’
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:453:5: note: candidate: SharedPtrAdapter<T>::SharedPtrAdapter(SharedPtrAdapter<T>::S&) [with T = tcam::MemoryBuffer; SharedPtrAdapter<T>::S = std::shared_ptr<tcam::MemoryBuffer>]
     SharedPtrAdapter(      S& sharedPtr) : ptr(0), size(0), owner(0), sharedPtr(sharedPtr) { }
     ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:453:5: note:   no known conversion for argument 1 from ‘tcam::MemoryBuffer’ to ‘SharedPtrAdapter<tcam::MemoryBuffer>::S& {aka std::shared_ptr<tcam::MemoryBuffer>&}’
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:452:5: note: candidate: SharedPtrAdapter<T>::SharedPtrAdapter(const S&) [with T = tcam::MemoryBuffer; SharedPtrAdapter<T>::S = std::shared_ptr<tcam::MemoryBuffer>]
     SharedPtrAdapter(const S& sharedPtr) : ptr(0), size(0), owner(0), sharedPtr2(sharedPtr), sharedPtr(sharedPtr2) { }
     ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:452:5: note:   no known conversion for argument 1 from ‘tcam::MemoryBuffer’ to ‘const S& {aka const std::shared_ptr<tcam::MemoryBuffer>&}’
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:450:5: note: candidate: SharedPtrAdapter<T>::SharedPtrAdapter(const T*, size_t, void*) [with T = tcam::MemoryBuffer; size_t = long unsigned int]
     SharedPtrAdapter(const T* ptr, size_t size, void* owner) : ptr((T*)ptr), size(size), owner(owner),
     ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:450:5: note:   candidate expects 3 arguments, 1 provided
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:447:25: note: candidate: SharedPtrAdapter<tcam::MemoryBuffer>::SharedPtrAdapter(const SharedPtrAdapter<tcam::MemoryBuffer>&)
 template<class T> class SharedPtrAdapter {
                         ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:447:25: note:   no known conversion for argument 1 from ‘tcam::MemoryBuffer’ to ‘const SharedPtrAdapter<tcam::MemoryBuffer>&’
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:447:25: note: candidate: SharedPtrAdapter<tcam::MemoryBuffer>::SharedPtrAdapter(SharedPtrAdapter<tcam::MemoryBuffer>&&)
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:447:25: note:   no known conversion for argument 1 from ‘tcam::MemoryBuffer’ to ‘SharedPtrAdapter<tcam::MemoryBuffer>&&’
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:1348:43: error: invalid conversion from ‘void*’ to ‘SharedPtrAdapter<tcam::MemoryBuffer>::S* {aka std::shared_ptr<tcam::MemoryBuffer>*}’ [-fpermissive]
     adapter0.assign(rptr0, rsize0, rowner0);
                                           ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:455:10: note:   initializing argument 3 of ‘void SharedPtrAdapter<T>::assign(T*, size_t, SharedPtrAdapter<T>::S*) [with T = tcam::MemoryBuffer; size_t = long unsigned int; SharedPtrAdapter<T>::S = std::shared_ptr<tcam::MemoryBuffer>]’
     void assign(T* ptr, size_t size, S* owner) {
          ^
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp: In instantiation of ‘void SharedPtrAdapter<T>::assign(T*, size_t, SharedPtrAdapter<T>::S*) [with T = tcam::MemoryBuffer; size_t = long unsigned int; SharedPtrAdapter<T>::S = std::shared_ptr<tcam::MemoryBuffer>]’:
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:1348:43:   required from here
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/jniTISCamera.cpp:459:50: error: comparison between distinct pointer types ‘SharedPtrAdapter<tcam::MemoryBuffer>::S* {aka std::shared_ptr<tcam::MemoryBuffer>*}’ and ‘tcam::MemoryBuffer*’ lacks a cast
         this->sharedPtr = owner != NULL && owner != ptr ? *(S*)owner : S((T*)ptr);
saudet commented 6 years ago

It's probably going to be tricky to get the callback with std::shared_ptr to work yes, but do you need it? It doesn't look like we need it...

beligum commented 6 years ago

I don't think we need it too. At least, it kind of works now, but it's buggy as hell... After compiling in a ton of debug lines in the C++ code, I get the impression the MemoryBuffer pointer that gets passed to the callback is tampered with somehow and gets corrupted in the process. I try to copy out the data as soon and fast as possible, but 5 times out of 10, accessing the frame data after it returns from the callback crashes the entire process. Don't know what to do...

saudet commented 6 years ago

Are you allocating data in the callback?

beligum commented 6 years ago

Yes. The callback receives the image frames and I try to wrap them in opencv Mat objects. However, afaik, I don't allocate data in the call() method, but I try to do everything in the constructor. Here's my (messy) callback class, together with a recent stacktrace:

public class FrameCallback extends TISCamera.sink_callback
{
    //-----CONSTANTS-----

    //-----VARIABLES-----
    final String WINDOW = "Source";
    private Object lock = new Object();
    private TISCamera.DeviceInterface device;
    private TISCamera.VideoFormat format;
    private opencv_core.Mat raw;
    private opencv_core.Mat rgb;
    private CameraListener listener;
    private boolean stopped = false;

    //-----CONSTRUCTORS-----
    public FrameCallback(TISCamera.DeviceInterface device, CameraListener listener)
    {
        super();

        Logger.info("Creating callback object...");

        this.device = device;
        this.listener = listener;
        this.format = device.get_active_video_format();

                    this.raw = new opencv_core.Mat(format.get_size().height(), format.get_size().width(), opencv_core.CV_8UC1);
                    this.raw.step(format.get_pitch_size());
        //
                    this.rgb = new opencv_core.Mat(format.get_size().height(), format.get_size().width(), opencv_core.CV_8UC3);

        //            namedWindow(WINDOW, CV_WINDOW_AUTOSIZE);
    }

    //-----PUBLIC METHODS-----
    //No idea if we need this, got it from the FunctionPointer docs...
    private native void allocate();
    @Override
    public synchronized void call(TISCamera.MemoryBuffer frame, Pointer arg1)
    {
        Logger.info("call");
        Logger.info("is complete? " + frame.is_complete());
        Logger.info("is locked? " + frame.is_locked());

        try {
            if (!this.stopped && frame.is_complete() && !frame.is_locked()) {

                Logger.info("entered");
                if (!this.stopped) {
                    //System.out.println("New frame received...");

                    Logger.info("debug 1");
                    frame.lock();
                    Logger.info("debug 2");

                    //Note: clone seems to add a lot of stability...
//                    opencv_core.Mat raw = new opencv_core.Mat(format.get_size().height(), format.get_size().width(), opencv_core.CV_8UC1, frame.get_data(), format.get_pitch_size()).clone();
//                    opencv_core.Mat rgb = new opencv_core.Mat(format.get_size().height(), format.get_size().width(), opencv_core.CV_8UC3);

                    raw.data(frame.get_data());

                    Logger.info("debug 3");

                    opencv_imgproc.cvtColor(raw, rgb, opencv_imgproc.CV_BayerGB2RGB);

                    Logger.info("debug 4");

                    if (listener != null) {
                        Logger.info("debug 5");
                        //listener.onCameraFrame(0, rgb);
                        Logger.info("debug 6");
                    }

                    Logger.info("debug 7");

                    frame.unlock();

                    Logger.info("debug 8");

                    Logger.info("debug 9");

                    //                    imshow(WINDOW, this.rgbFrame);
                    //                    waitKey(25);
                }
            }
        }
        finally {
            //whatever happens, always give back the frame
            this.device.requeue_buffer(frame);
        }
    }

    public synchronized void free()
    {
        this.stopped = true;
        //                destroyWindow(WINDOW);
    }

    //-----PROTECTED METHODS-----

    //-----PRIVATE METHODS-----

}

stacktrace:

Stack: [0x00007fd97c5ea000,0x00007fd97cdeb000],  sp=0x00007fd97cde99c0,  free space=8190k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [libc.so.6+0x84512]  cfree+0x22
C  [libjniTISCamera.so+0x35a3b]  JavaCPP_org_bytedeco_javacpp_TISCamera_00024sink_1callback_allocate_callback+0xbb
C  [libtcam-libusb.so+0xb1d60]  tcam::AFU420Device::push_buffer()+0x2fc
C  [libtcam-libusb.so+0xb287e]  tcam::AFU420Device::transfer_callback(libusb_transfer*)+0x692
C  [libtcam-libusb.so+0xb2ac5]  tcam::AFU420Device::libusb_bulk_callback(libusb_transfer*)+0x6f
saudet commented 6 years ago

Ah, that just means your FrameCallback got garbage collected. You need to keep a reference to it somewhere in some field.

beligum commented 6 years ago

Sadly, that's already the case, it's being kept as a class variable, although in a separate thread:

if (useCallbacks) {
            //don't define this here, or it will get GC!!!
            this.frameCallback = new FrameCallback(device, listener);
            this.imageSink = new TISCamera.ImageSink();
            boolean registered1 = this.imageSink.registerCallback(frameCallback, null);
            device.set_sink(this.imageSink);
        }

Can you think of another reason it gets GC?

beligum commented 6 years ago

I get the impression the app being multi-threaded might be the cause of all problems, because I need to "re-load" the libraries after booting the capture thread, is that possible?

@Override
    public synchronized void run()
    {
        Loader.load(opencv_core.class);
        Loader.load(TISCamera.class);
        ...
saudet commented 6 years ago

It doesn't help if you load them before registering your callback?

beligum commented 6 years ago

That's the case, they get loaded first thing, then the camera is opened, then the callback is registered.

saudet commented 6 years ago

Since the thread isn't created by Java, we need to attach and detach it at every call to prevent memory leaks, but maybe the camera doesn't like that. Try to add @Platform(define = "NO_JNI_DETACH_THREAD", ...) to see if that helps.

beligum commented 6 years ago

Actually, it's the Java side that starts up the thread:

beligum commented 6 years ago

The NO_JNI_DETACH_THREAD doesn't really make a difference btw...

beligum commented 6 years ago

However, the stacktraces seem to have become more clear:

Stack: [0x00007fde59cb9000,0x00007fde59dba000],  sp=0x00007fde59db8630,  free space=1021k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
C  [libc.so.6+0x84512]  cfree+0x22
C  [libjniTISCamera.so+0x1ba3e]
j  org.bytedeco.javacpp.Pointer$NativeDeallocator.deallocate()V+27
j  org.bytedeco.javacpp.Pointer$DeallocatorReference.clear()V+49
j  org.bytedeco.javacpp.Pointer$DeallocatorThread.run()V+11
v  ~StubRoutines::call_stub
V  [libjvm.so+0x695ae6]  JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x1056
V  [libjvm.so+0x695ff1]  JavaCalls::call_virtual(JavaValue*, KlassHandle, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x321
V  [libjvm.so+0x696497]  JavaCalls::call_virtual(JavaValue*, Handle, KlassHandle, Symbol*, Symbol*, Thread*)+0x47
V  [libjvm.so+0x731cb0]  thread_entry(JavaThread*, Thread*)+0xa0
V  [libjvm.so+0xa7eaa3]  JavaThread::thread_main_inner()+0x103
V  [libjvm.so+0xa7ebec]  JavaThread::run()+0x11c
V  [libjvm.so+0x92da28]  java_start(Thread*)+0x108
C  [libpthread.so.0+0x76ba]  start_thread+0xca
saudet commented 6 years ago

Looks like typical heap corruption. Something is getting freed when it shouldn't...

beligum commented 6 years ago

Yeah, it looks like it's the MemoryBuffer object that's passed to the callback that's causing problems. The JavaCpp code looks like this:

public native void call(@ByVal @Cast("tcam::MemoryBuffer*") MemoryBuffer arg0, Pointer arg1);

and the calling c++ code looks like this:

void ImageSink::push_image (std::shared_ptr<MemoryBuffer> buffer)
{
...
if (callback != nullptr)
    {
      MemoryBuffer* bufferPtr = buffer.get();
      this->callback(bufferPtr, user_data);
    }
}
...

It's a pain in the ass to debug because the corruption happens in the JNI/JavaCpp glue code of these calls. Could you put me on the right track on how to debug this?

saudet commented 6 years ago

I don't think it happens in the glue code. It's just passing the pointer. Even an empty callback in Java doesn't work?

beligum commented 6 years ago

The Java callback works, but after a few seconds (and especially when there's heavier work in the Java callback code), the memory corruption happens. Adding the @ByVal @Cast("tcam::MemoryBuffer*") to the JavaCPP callback code seemed to improve stability, but it's still crashing. Actually, it either crashes at the end of the callback, or when this C++ code is executed after the callback returned: current_buffer_->clear();. I get the impression the buffer's memory is freed somehow where it shouldn't, but I can't debug it.

saudet commented 6 years ago

You could try to run AddressSanitizer or Valgrind on the native wrapper code: https://github.com/deeplearning4j/libnd4j/wiki/Debugging-libnd4j But if an empty callback works, it's probably something wrong with your Java code..

beligum commented 6 years ago

So, I decided to dig deep and go all the way to solve this...

The real problem is that the MemoryBuffer object gets (auto)destructed by JavaCPP, freeing the underlying framebuffer (in the destructor of MemoryBuffer), resulting in a crash on next write. I suspect that JavaCPP doesn't like mixed shared_ptr and real pointer datatype usage and gets confused.

All this happens when the memory buffer is given back to the queue, after handling it. It's implemented like this:

void requeue_buffer (std::shared_ptr<MemoryBuffer>);

However, since my callback function doesn't get passed a shared_ptr, but a real one (MemoryBuffer*), I wrote a simple test c++ method to try to overcome this:

void requeue_buffer_ptr (MemoryBuffer*);

that gets translated via JavaCPP to

public native void requeue_buffer_ptr(MemoryBuffer arg0);

(I have no clue how to adjust/play with the annotations on the MemoryBuffer object in JavaCPP)

Then, I've added debug/stacktrace code to the c++ destructor and as soon as the java callback ends, this happens:

2473574    <INFO> /home/bram/Projects/nevelland/dev/camera/TheImagingSource/driver/tiscamera/src/MemoryBuffer.cpp:96: Destructing MemoryBuffer
BACKTRACE ------------
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/linux-x86_64/libtcam-libusb.so(_ZN4tcam12MemoryBufferD2Ev+0x33) [0x7f98b4c98f89]
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/linux-x86_64/libjniTISCamera.so(JavaCPP_org_bytedeco_javacpp_TISCamera_00024sink_1callback_allocate_callback+0xbb) [0x7f98af42aafb]
/home/bram/Projects/Workspace/idea/javacpp-presets/tiscamera/target/classes/org/bytedeco/javacpp/linux-x86_64/libtcam-libusb.so(_ZN4tcam9ImageSink10push_imageESt10shared_ptrINS_12MemoryBufferEE+0x2f3) [0x7f98b4cafae3]
/usr/lib/tcam-0/libtcam-libusb.so(_ZN4tcam12AFU420Device11push_bufferEv+0x2fc) [0x7f989e2d90f4]
/usr/lib/tcam-0/libtcam-libusb.so(_ZN4tcam12AFU420Device17transfer_callbackEP15libusb_transfer+0x782) [0x7f989e2d9d02]
/usr/lib/tcam-0/libtcam-libusb.so(_ZN4tcam12AFU420Device20libusb_bulk_callbackEP15libusb_transfer+0x6f) [0x7f989e2d9fc5]
/lib/x86_64-linux-gnu/libusb-1.0.so.0(+0x915a) [0x7f98b49b515a]
/lib/x86_64-linux-gnu/libusb-1.0.so.0(+0xef08) [0x7f98b49baf08]
/lib/x86_64-linux-gnu/libusb-1.0.so.0(+0x8c49) [0x7f98b49b4c49]
/lib/x86_64-linux-gnu/libusb-1.0.so.0(libusb_handle_events_timeout_completed+0xd3) [0x7f98b49b5b53]
/usr/lib/tcam-0/libtcam-libusb.so(_ZN4tcam10UsbHandler13handle_eventsEv+0x72) [0x7f989e2f5816]
/usr/lib/tcam-0/libtcam-libusb.so(_ZNKSt12_Mem_fn_baseIMN4tcam10UsbHandlerEFvvELb1EEclIIEvEEvPS1_DpOT_+0x65) [0x7f989e2f7e95]
/usr/lib/tcam-0/libtcam-libusb.so(_ZNSt12_Bind_simpleIFSt7_Mem_fnIMN4tcam10UsbHandlerEFvvEEPS2_EE9_M_invokeIILm0EEEEvSt12_Index_tupleIIXspT_EEE+0x43) [0x7f989e2f7e29]
/usr/lib/tcam-0/libtcam-libusb.so(_ZNSt12_Bind_simpleIFSt7_Mem_fnIMN4tcam10UsbHandlerEFvvEEPS2_EEclEv+0x2c) [0x7f989e2f7d30]
/usr/lib/tcam-0/libtcam-libusb.so(_ZNSt6thread5_ImplISt12_Bind_simpleIFSt7_Mem_fnIMN4tcam10UsbHandlerEFvvEEPS4_EEE6_M_runEv+0x1c) [0x7f989e2f7cc0]
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xb8c80) [0x7f98b51b4c80]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x76ba) [0x7f98d5d386ba]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d) [0x7f98d565441d]
----------------------

The bottom line is: JavaCPP destroys the (pointer to) MemoryBuffer object when it shouldn't... No idea how to prevent this though...

For reference, this is my current JavaCPP mapping:

public class TISCamera implements InfoMapper
{
    public void map(InfoMap infoMap)
    {
        infoMap.put(new Info("std::map<std::string,int>").pointerTypes("StringIntMap").define());
        infoMap.put(new Info("std::vector<tcam::Property*>").pointerTypes("PropertyVector").define());

        infoMap.put(new Info("std::shared_ptr<tcam::MemoryBuffer>").annotations("@SharedPtr").pointerTypes("MemoryBuffer"));
        infoMap.put(new Info("std::vector<std::shared_ptr<tcam::MemoryBuffer> >").pointerTypes("SharedMemoryBufferVector").define());

        infoMap.put(new Info("std::shared_ptr<tcam::Property>").annotations("@SharedPtr").pointerTypes("Property"));
        infoMap.put(new Info("std::vector<std::shared_ptr<tcam::Property> >").pointerTypes("SharedPropertyVector").define());

        infoMap.put(new Info("tcam::SinkInterface::set_source").skip());
        infoMap.put(new Info("tcam::ImageSink::set_source").skip());

        infoMap.put(new Info().javaText("\n" +
                                        "public static class sink_callback extends FunctionPointer {\n" +
                                        "    static { Loader.load(); }\n" +
                                        "    public    sink_callback(Pointer p) { super(p); }\n" +
                                        "    protected sink_callback() { allocate(); }\n" +
                                        "    private native void allocate();\n" +
                                        "    public native void call(@ByVal @Cast(\"tcam::MemoryBuffer*\") MemoryBuffer arg0, Pointer arg1);\n" +
                                        "}").define());
        infoMap.put(new Info("ImageSink.h").linePatterns(".*typedef void \\(\\*sink_callback\\)\\(tcam::MemoryBuffer\\*, void\\*\\).*").skip());

        infoMap.put(new Info("tcam::ImageSink::registerCallback")
                                    .javaText("public native @Cast(\"bool\") boolean registerCallback(@Cast(\"sink_callback\") sink_callback arg0, Pointer arg1);"));
                infoMap.put(new Info("tcam::ImageSink::registerCallback").skip());
}
saudet commented 6 years ago

Didn't you say there was a version of the callback that didn't use std::shared_ptr?

beligum commented 6 years ago

Yes, that's the one I use. I hacked the c++ code (or at least the API I use from the Java side) to get away with all smart pointers and changed the code generation of sink_callback to use the @ByPtr annotation and I get the feeling it's all stable now...

saudet commented 6 years ago

Ah, you had @ByVal in there. Of course, that won't work at all. You got a compiler warning about that: https://github.com/bytedeco/javacpp-presets/issues/520#issuecomment-364007570

beligum commented 6 years ago

Yeah, that was the last issue that stood in the way of getting a stable stream. Bottom line is: don't use smart pointers with JavaCPP, they don't mix-and-mingle very well...

beligum commented 6 years ago

Pfff, man, the memory buffers still seem to get garbage collected randomly...

beligum commented 6 years ago

Ok, putting them in a simple java-side datastructure seems to be the solution...

saudet commented 6 years ago

What does the JNI wrapper code look like?