From ce7a56284f10c0b42c92c17bd0e3b77df8a2ef1a Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 13 Jan 2015 22:18:17 -0800 Subject: [PATCH] Fix a few major nivision Java wrapper issues. - sliceByteBuffer() was not setting native order on the duplicated buffer. This caused all array-copyin functions to generate bad values. - Correctly handle unsigned byte and unsigned short values. These could read/write to bad locations previously. - Implement custom version of imaqReadFile() to always pass in NULL for the colorTable. Eventually a more-complete version should be written. Also this works around a crash in imaqGetErrorText() by not calling it from throwJavaException(). It's not clear why imaqGetErrorText() is crashing at present (my best guess is there's still something fishy with multiple C++ lib versions getting loaded somehow), as this used to work. Instead, the exception now just gives the error code without the error message, which is not user friendly but at least doesn't crash. This will be fixed in a future commit by creating our own version of imaqGetErrorText() based on the information available in the header file. Change-Id: I4d099e62ee41f8e2a50089806561be191cb5d9d7 --- .../src/main/java/com/ni/vision/NIVision.java | 128 ++++++++---------- wpilibj/wpilibJavaJNI/lib/NIVisionJNI.cpp | 15 +- wpilibj/wpilibJavaJNI/nivision/gen_java.py | 72 +++++++--- .../wpilibJavaJNI/nivision/nivision_2011.ini | 4 +- 4 files changed, 117 insertions(+), 102 deletions(-) diff --git a/wpilibj/wpilibJavaDevices/src/main/java/com/ni/vision/NIVision.java b/wpilibj/wpilibJavaDevices/src/main/java/com/ni/vision/NIVision.java index 4955a12c83..e0863530f8 100644 --- a/wpilibj/wpilibJavaDevices/src/main/java/com/ni/vision/NIVision.java +++ b/wpilibj/wpilibJavaDevices/src/main/java/com/ni/vision/NIVision.java @@ -51,7 +51,7 @@ public class NIVision { } public static ByteBuffer sliceByteBuffer(ByteBuffer bb, int offset, int size) { - ByteBuffer new_bb = bb.duplicate(); + ByteBuffer new_bb = bb.duplicate().order(ByteOrder.nativeOrder()); new_bb.position(offset); new_bb.limit(size); return new_bb; @@ -4734,16 +4734,16 @@ public class NIVision { super.setBuffer(backing, offset, 4); } public void read() { - B = backing.getShort(0); - G = backing.getShort(1); - R = backing.getShort(2); - alpha = backing.getShort(3); + B = (short)(backing.get(0) & 0xff); + G = (short)(backing.get(1) & 0xff); + R = (short)(backing.get(2) & 0xff); + alpha = (short)(backing.get(3) & 0xff); } public void write() { - backing.putShort(0, B); - backing.putShort(1, G); - backing.putShort(2, R); - backing.putShort(3, alpha); + backing.put(0, (byte)(B & 0xff)); + backing.put(1, (byte)(G & 0xff)); + backing.put(2, (byte)(R & 0xff)); + backing.put(3, (byte)(alpha & 0xff)); } public int size() { return 4; @@ -12969,16 +12969,16 @@ public class NIVision { super.setBuffer(backing, offset, 8); } public void read() { - B = backing.getInt(0); - G = backing.getInt(2); - R = backing.getInt(4); - alpha = backing.getInt(6); + B = (int)(backing.getShort(0) & 0xffff); + G = (int)(backing.getShort(2) & 0xffff); + R = (int)(backing.getShort(4) & 0xffff); + alpha = (int)(backing.getShort(6) & 0xffff); } public void write() { - backing.putInt(0, B); - backing.putInt(2, G); - backing.putInt(4, R); - backing.putInt(6, alpha); + backing.putShort(0, (short)(B & 0xffff)); + backing.putShort(2, (short)(G & 0xffff)); + backing.putShort(4, (short)(R & 0xffff)); + backing.putShort(6, (short)(alpha & 0xffff)); } public int size() { return 8; @@ -13505,16 +13505,16 @@ public class NIVision { super.setBuffer(backing, offset, 4); } public void read() { - L = backing.getShort(0); - S = backing.getShort(1); - H = backing.getShort(2); - alpha = backing.getShort(3); + L = (short)(backing.get(0) & 0xff); + S = (short)(backing.get(1) & 0xff); + H = (short)(backing.get(2) & 0xff); + alpha = (short)(backing.get(3) & 0xff); } public void write() { - backing.putShort(0, L); - backing.putShort(1, S); - backing.putShort(2, H); - backing.putShort(3, alpha); + backing.put(0, (byte)(L & 0xff)); + backing.put(1, (byte)(S & 0xff)); + backing.put(2, (byte)(H & 0xff)); + backing.put(3, (byte)(alpha & 0xff)); } public int size() { return 4; @@ -13553,16 +13553,16 @@ public class NIVision { super.setBuffer(backing, offset, 4); } public void read() { - V = backing.getShort(0); - S = backing.getShort(1); - H = backing.getShort(2); - alpha = backing.getShort(3); + V = (short)(backing.get(0) & 0xff); + S = (short)(backing.get(1) & 0xff); + H = (short)(backing.get(2) & 0xff); + alpha = (short)(backing.get(3) & 0xff); } public void write() { - backing.putShort(0, V); - backing.putShort(1, S); - backing.putShort(2, H); - backing.putShort(3, alpha); + backing.put(0, (byte)(V & 0xff)); + backing.put(1, (byte)(S & 0xff)); + backing.put(2, (byte)(H & 0xff)); + backing.put(3, (byte)(alpha & 0xff)); } public int size() { return 4; @@ -13601,16 +13601,16 @@ public class NIVision { super.setBuffer(backing, offset, 4); } public void read() { - I = backing.getShort(0); - S = backing.getShort(1); - H = backing.getShort(2); - alpha = backing.getShort(3); + I = (short)(backing.get(0) & 0xff); + S = (short)(backing.get(1) & 0xff); + H = (short)(backing.get(2) & 0xff); + alpha = (short)(backing.get(3) & 0xff); } public void write() { - backing.putShort(0, I); - backing.putShort(1, S); - backing.putShort(2, H); - backing.putShort(3, alpha); + backing.put(0, (byte)(I & 0xff)); + backing.put(1, (byte)(S & 0xff)); + backing.put(2, (byte)(H & 0xff)); + backing.put(3, (byte)(alpha & 0xff)); } public int size() { return 4; @@ -13652,13 +13652,13 @@ public class NIVision { b = backing.getDouble(0); a = backing.getDouble(8); L = backing.getDouble(16); - alpha = backing.getShort(24); + alpha = (short)(backing.get(24) & 0xff); } public void write() { backing.putDouble(0, b); backing.putDouble(8, a); backing.putDouble(16, L); - backing.putShort(24, alpha); + backing.put(24, (byte)(alpha & 0xff)); } public int size() { return 32; @@ -13700,13 +13700,13 @@ public class NIVision { Z = backing.getDouble(0); Y = backing.getDouble(8); X = backing.getDouble(16); - alpha = backing.getShort(24); + alpha = (short)(backing.get(24) & 0xff); } public void write() { backing.putDouble(0, Z); backing.putDouble(8, Y); backing.putDouble(16, X); - backing.putShort(24, alpha); + backing.put(24, (byte)(alpha & 0xff)); } public int size() { return 32; @@ -19495,7 +19495,7 @@ public class NIVision { } ByteBuffer rv_buf = ByteBuffer.allocateDirect(8).order(ByteOrder.nativeOrder()); long rv_addr = getByteBufferAddress(rv_buf); - _imaqParticleFilter4(dest.getAddress(), source.getAddress(), getByteBufferAddress(criteria_buf), criteriaCount, options.getAddress(), roi.getAddress(), rv_addr+0); + _imaqParticleFilter4(dest.getAddress(), source.getAddress(), getByteBufferAddress(criteria_buf), criteriaCount, options.getAddress(), roi == null ? 0 : roi.getAddress(), rv_addr+0); int numParticles; numParticles = rv_buf.getInt(0); return numParticles; @@ -20837,33 +20837,17 @@ public class NIVision { } private static native int _imaqOpenAVI(long fileName); - public static class ReadFileResult { - public RGBValue colorTable; - public int numColors; - private ReadFileResult(ByteBuffer rv_buf) { - colorTable = new RGBValue(rv_buf, 0); - colorTable.read(); - numColors = rv_buf.getInt(8); + public static void imaqReadFile(Image image, String fileName) { + ByteBuffer fileName_buf; + byte[] fileName_bytes; + try { + fileName_bytes = fileName.getBytes("UTF-8"); + } catch (UnsupportedEncodingException e) { + fileName_bytes = new byte[0]; } - } - - public static ReadFileResult imaqReadFile(Image image, String fileName) { - ByteBuffer fileName_buf = null; - if (fileName != null) { - byte[] fileName_bytes; - try { - fileName_bytes = fileName.getBytes("UTF-8"); - } catch (UnsupportedEncodingException e) { - fileName_bytes = new byte[0]; - } - fileName_buf = ByteBuffer.allocateDirect(fileName_bytes.length+1); - putBytes(fileName_buf, fileName_bytes, 0, fileName_bytes.length).put(fileName_bytes.length, (byte)0); - } - ByteBuffer rv_buf = ByteBuffer.allocateDirect(8+8).order(ByteOrder.nativeOrder()); - long rv_addr = getByteBufferAddress(rv_buf); - _imaqReadFile(image.getAddress(), fileName == null ? 0 : getByteBufferAddress(fileName_buf), rv_addr+0, rv_addr+8); - ReadFileResult rv = new ReadFileResult(rv_buf); - return rv; + fileName_buf = ByteBuffer.allocateDirect(fileName_bytes.length+1); + putBytes(fileName_buf, fileName_bytes, 0, fileName_bytes.length).put(fileName_bytes.length, (byte)0); + _imaqReadFile(image.getAddress(), getByteBufferAddress(fileName_buf), 0, 0); } private static native void _imaqReadFile(long image, long fileName, long colorTable, long numColors); diff --git a/wpilibj/wpilibJavaJNI/lib/NIVisionJNI.cpp b/wpilibj/wpilibJavaJNI/lib/NIVisionJNI.cpp index 135dd79a32..0a6e7ffa4e 100644 --- a/wpilibj/wpilibJavaJNI/lib/NIVisionJNI.cpp +++ b/wpilibj/wpilibJavaJNI/lib/NIVisionJNI.cpp @@ -14,10 +14,12 @@ static void throwJavaException(JNIEnv *env) { jclass je = env->FindClass("com/ni/vision/VisionException"); int err = imaqGetLastError(); - char* err_text = imaqGetErrorText(err); - char* full_err_msg = (char*)malloc(30+strlen(err_text)); - sprintf(full_err_msg, "imaqError: %d: %s", err, err_text); - imaqDispose(err_text); + //char* err_text = imaqGetErrorText(err); + //char* full_err_msg = (char*)malloc(30+strlen(err_text)); + //sprintf(full_err_msg, "imaqError: %d: %s", err, err_text); + //imaqDispose(err_text); + char* full_err_msg = (char*)malloc(30); + sprintf(full_err_msg, "imaqError: %d", err); env->ThrowNew(je, full_err_msg); free(full_err_msg); } @@ -1456,11 +1458,6 @@ JNIEXPORT jint JNICALL Java_com_ni_vision_NIVision__1imaqOpenAVI(JNIEnv* env, jc return (jint)rv; } -/* J: void imaqReadFile(Image image, String fileName) - * JN: void imaqReadFile(long image, long fileName, long colorTable, long numColors) - * C: int imaqReadFile(Image* image, const char* fileName, RGBValue* colorTable, int* numColors) - */ - JNIEXPORT void JNICALL Java_com_ni_vision_NIVision__1imaqReadFile(JNIEnv* env, jclass , jlong image, jlong fileName, jlong colorTable, jlong numColors) { int rv = imaqReadFile((Image*)image, (const char*)fileName, (RGBValue*)colorTable, (int*)numColors); diff --git a/wpilibj/wpilibJavaJNI/nivision/gen_java.py b/wpilibj/wpilibJavaJNI/nivision/gen_java.py index 4c53601a98..d0fbb8fa11 100644 --- a/wpilibj/wpilibJavaJNI/nivision/gen_java.py +++ b/wpilibj/wpilibJavaJNI/nivision/gen_java.py @@ -9,15 +9,18 @@ except ImportError: from nivision_parse import * +# base, cast-out-pre, cast-out-post, cast-in-pre, cast-in-post java_accessor_map = { - "B": "", - "C": "Char", - "S": "Short", - "I": "Int", - "J": "Long", - "F": "Float", - "D": "Double", - "Z": "Boolean", + "B": ("", "", "", "", ""), + "C": ("Char", "", "", "", ""), + "S": ("Short", "", "", "", ""), + "I": ("Int", "", "", "", ""), + "J": ("Long", "", "", "", ""), + "F": ("Float", "", "", "", ""), + "D": ("Double", "", "", "", ""), + "Z": ("Boolean", "", "", "", ""), + "X": ("", "(short)(", " & 0xff)", "(byte)(", " & 0xff)"), + "Y": ("Short", "(int)(", " & 0xffff)", "(short)(", " & 0xffff)"), } java_size_map = { @@ -63,9 +66,9 @@ java_types_map = { ("int", None): JavaType("int", "int", "jint", "I"), ("char", None): JavaType("byte", "byte", "jbyte", "B"), ("wchar_t", None): JavaType("char", "char", "jchar", "C"), - ("unsigned char", None): JavaType("short", "short", "jshort", "S"), + ("unsigned char", None): JavaType("short", "short", "jshort", "X"), ("short", None): JavaType("short", "short", "jshort", "S"), - ("unsigned short", None): JavaType("int", "int", "jint", "I"), + ("unsigned short", None): JavaType("int", "int", "jint", "Y"), ("unsigned", None): JavaType("int", "int", "jint", "I"), ("unsigned int", None): JavaType("int", "int", "jint", "I"), ("uInt32", None): JavaType("int", "int", "jint", "I"), @@ -431,8 +434,8 @@ structEmit.toArg = "{fname}.getAddress()" # java type jtypeEmit = JavaEmitData() -jtypeEmit.addBackingRead("{fname} = {backing}.get{jaccessor}({foffset});") -jtypeEmit.addBackingWrite("{backing}.put{jaccessor}({foffset}, {fname});") +jtypeEmit.addBackingRead("{fname} = {jaccessor_cast_out_pre}{backing}.get{jaccessor}({foffset}){jaccessor_cast_out_post};") +jtypeEmit.addBackingWrite("{backing}.put{jaccessor}({foffset}, {jaccessor_cast_in_pre}{fname}{jaccessor_cast_in_post});") # string - array of characters strSizedEmit = JavaEmitData() @@ -561,7 +564,7 @@ class JavaStructEmitHelper: struct_sz = None buftype = "ByteBuffer" array_size = jtype.array_size or "" - jaccessor = None + jaccessor = (None, None, None, None, None) writeBufs = [] backingRead = [] @@ -612,7 +615,7 @@ for (int i=0, off={foffset}; i<%s; i++, off += {struct_sz}) elif jtype.jni_sig[1] == 'B': typeemit = byteArrayEmit elif jtype.jni_sig[1] in java_accessor_map: - buftype = "%sBuffer" % java_accessor_map[jtype.jni_sig[1]] + buftype = "%sBuffer" % java_accessor_map[jtype.jni_sig[1]][0] struct_sz = java_size_map[jtype.jni_sig[1]] typeemit = jtypeArrayEmit else: @@ -660,7 +663,11 @@ for (int i=0, off={foffset}; i<%s; i++, off += {struct_sz}) struct_sz=struct_sz, array_size=array_size, buftype=buftype, - jaccessor=jaccessor, + jaccessor=jaccessor[0], + jaccessor_cast_out_pre=jaccessor[1], + jaccessor_cast_out_post=jaccessor[2], + jaccessor_cast_in_pre=jaccessor[3], + jaccessor_cast_in_post=jaccessor[4], backing=backing) jconstruct = [x.format(**fargs) for x in construct] jwritebufs = [x.format(**fargs) for x in writeBufs] @@ -922,7 +929,7 @@ public class {classname} {{ }} public static ByteBuffer sliceByteBuffer(ByteBuffer bb, int offset, int size) {{ - ByteBuffer new_bb = bb.duplicate(); + ByteBuffer new_bb = bb.duplicate().order(ByteOrder.nativeOrder()); new_bb.position(offset); new_bb.limit(size); return new_bb; @@ -1129,10 +1136,12 @@ public class {classname} {{ static void throwJavaException(JNIEnv *env) {{ jclass je = env->FindClass("{packagepath}/VisionException"); int err = imaqGetLastError(); - char* err_text = imaqGetErrorText(err); - char* full_err_msg = (char*)malloc(30+strlen(err_text)); - sprintf(full_err_msg, "imaqError: %d: %s", err, err_text); - imaqDispose(err_text); + //char* err_text = imaqGetErrorText(err); + //char* full_err_msg = (char*)malloc(30+strlen(err_text)); + //sprintf(full_err_msg, "imaqError: %d: %s", err, err_text); + //imaqDispose(err_text); + char* full_err_msg = (char*)malloc(30); + sprintf(full_err_msg, "imaqError: %d", err); env->ThrowNew(je, full_err_msg); free(full_err_msg); }} @@ -1491,6 +1500,29 @@ JNIEXPORT jint JNICALL Java_{package}_{classname}__1IMAQdxGetImageData(JNIEnv* e return (jint)actualBufferNumber; }}""".format(package=self.package.replace(".", "_"), classname=self.classname), file=self.outc) + elif name == "imaqReadFile": + print(""" + public static void imaqReadFile(Image image, String fileName) {{ + ByteBuffer fileName_buf; + byte[] fileName_bytes; + try {{ + fileName_bytes = fileName.getBytes("UTF-8"); + }} catch (UnsupportedEncodingException e) {{ + fileName_bytes = new byte[0]; + }} + fileName_buf = ByteBuffer.allocateDirect(fileName_bytes.length+1); + putBytes(fileName_buf, fileName_bytes, 0, fileName_bytes.length).put(fileName_bytes.length, (byte)0); + _imaqReadFile(image.getAddress(), getByteBufferAddress(fileName_buf), 0, 0); + }} + private static native void _imaqReadFile(long image, long fileName, long colorTable, long numColors);""".format(), file=self.out) + print(""" +JNIEXPORT void JNICALL Java_{package}_{classname}__1imaqReadFile(JNIEnv* env, jclass , jlong image, jlong fileName, jlong colorTable, jlong numColors) +{{ + int rv = imaqReadFile((Image*)image, (const char*)fileName, (RGBValue*)colorTable, (int*)numColors); + if (rv == 0) throwJavaException(env); +}}""".format(package=self.package.replace(".", "_"), + classname=self.classname), file=self.outc) + return if self.config_getboolean(name, "exclude", fallback=False): return diff --git a/wpilibj/wpilibJavaJNI/nivision/nivision_2011.ini b/wpilibj/wpilibJavaJNI/nivision/nivision_2011.ini index 025d2f18ba..572c391f9c 100644 --- a/wpilibj/wpilibJavaJNI/nivision/nivision_2011.ini +++ b/wpilibj/wpilibJavaJNI/nivision/nivision_2011.ini @@ -265,6 +265,7 @@ arraysize=measurements:numMeasurements [imaqParticleFilter4] arraysize=criteria:criteriaCount outparams=numParticles +nullok=roi ; Morphology functions [imaqFindCircles] @@ -391,7 +392,8 @@ size=data:dataSize # unclear whether dataSize is input or output parameter exclude=True [imaqReadFile] -nullok=colorTable,numColors +;nullok=colorTable,numColors +;arraysize=colorTable:numColors [imaqWriteAVIFrame] size=data:dataLength [imaqWriteBMPFile]