C# Bindings/API

[shmuelie]

I wanted to start the conversation on the C# Bindings/API

Primarily at this point that means designing the API surface.

[lilith]

Hi!

Two things:

1. All flowgraph* and flownode* API surfaces will be replaced by a single JSON api. This will cut the API surface down drastically.
2. There's a basic ruby wrapper here: https://github.com/imazen/imageflow/tree/master/wrappers/ruby/lib

Here's the subset of the API that would be used by the C# bindings. (in ruby FFI syntax). Check imageflow.h for param names.

• attach_function :flow_context_create, [], :pointer
• attach_function :flow_context_destroy, [:pointer], :void
• attach_function :flow_context_error_and_stacktrace, [:pointer, :pointer, :uint, :bool], :int64
• attach_function :flow_context_has_error, [:pointer], :bool
• attach_function :flow_context_error_reason, [:pointer], :int
• attach_function :flow_context_clear_error, [:pointer], :void
• attach_function :flow_io_create_for_file, [:pointer, :flow_io_mode, :string, :pointer], :pointer
• attach_function :flow_io_create_from_memory, [:pointer, :flow_io_mode, :pointer, :uint64, :pointer, :pointer], :pointer
• attach_function :flow_io_create_for_output_buffer, [:pointer, :pointer], :pointer
• attach_function :flow_io_get_output_buffer, [:pointer, :pointer, :pointer, :pointer], :bool
• attach_function :flow_job_add_io, [:pointer, :pointer, :pointer, :int32, :flow_direction], :bool
• attach_function :flow_job_create, [:pointer], :pointer
• attach_function :flow_job_destroy, [:pointer, :pointer], :bool
• attach_function :flow_job_configure_recording, [:pointer, :pointer, :bool, :bool, :bool, :bool, :bool], :bool
• attach_function :flow_job_execute, [:pointer, :pointer, :pointer], :bool, blocking: true
• attach_function :flow_job_get_decoder_info, [:pointer, :pointer, :int32, :pointer], :pointer, blocking: true

Note that that the last two API functions will be refactored to use JSON instead of structures. This refactoring hasn't taken place yet.

Hopefully this helps provide a sense of the (limited) scope of work required. We have no documentation right now about the JSON schema that will be used, but hope to publish that soon.

[lilith]

FFI API docs are now online. The interface should be pretty stable, but there is a slight chance that memory ownership semantics will be forced to change as more of the core is ported to rust.

These are uploaded as part of CI, and should be updated daily.

https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/imageflow_core/abi/index.html

[lilith]

Ideally we won't see FFI breakage, but rather spend more time refining the JSON message API, which should be much easier to debug.

[shmuelie]

@nathanaeljones trying to view the index gives me a 403

[lilith]

I fixed the link, sorry! Did some reorganization. There are also windows binaries of libimageflowrs.dll available, although the JSON message API only responds to "teapot" at the moment.

https://ci.appveyor.com/project/imazen/imageflow/build/1.0.307/job/6edn741gtjnouyk9/artifacts

[lilith]

Let me know if I can improve the clarity of the API docs. There's not much to document on imageflow_send_json yet, but those schemas should be coming this week.

[lilith]

@SamuelEnglard, will #81 be problematic for the C# API? I don't know that you'll need to use the error reporting API until we get to implementing custom I/O wrappers in C# for, say, Stream classes, at which point catch{} and serialize will be the pattern for all function delegates/pointers involved. I/O error reporting isn't great in general, so it kinda depends on where you want to set that bar.

[shmuelie]

81 will be an issue in that there's just no way to not transfer the C string memory to the GC's control. The best we could do would be to have C# allocate the error message memory and we fill it.

[lilith]

New docs, small API changes, but much higher confidence level: https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/imageflowrs/index.html

[lilith]

Ruby bindings have been updated and are passing all tests again. Over 60% of bugs were discrepancies between camelCasing and snake_casing.

[lilith]

The JSON API docs now live here:

https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/context_json_api.txt https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/job_json_api.txt

There is finally some documentation on packaging native binaries in .NET: https://github.com/dotnet/docs/blob/master/docs/core/tutorials/libraries.md#how-to-use-native-dependencies

Would you plan to target .NET Standard 1.5?

[shmuelie]

I'd try to target as low as I can, so hopefully 1.3.

[mms-]

If you can use help testing the c# wrapper let me know!

[lilith]

BTW, I'm happy to write the P/Invokes if that would help. It's reasonably trivial for me - the non-trivial bits are tooling/packaging/CI/release/scripts due to the cognitive space and context switching required.

If you want, the low-level bindings (and perhaps JSON (de)serialization schema/classes) could even go in this repo, under bindings/dotnet, so that they can be tested against each release. (Can the .NET Core tooling fit in 100MB? I could install it on the docker containers.)

[shmuelie]

My thinking/hoping is to have the P/Invoke and JSON (de)serialization code generated by T4 or Scripty. That way as you make changes they are updated with no issue.

The Core tooling should, really depends on the size of our dependancies

[lilith]

P/Invoke is easier by hand. I've invested incredible amounts of time into trying to automate that stuff - CppSharp, SWIG, and at least six others. There's just too much loss of information to make it work.

As far as JSON serialization, the easiest path would probably be to find something that works similar to Serde.rs and duplicate the types. Serde can't generate schemas or Swagger or anything yet, so my instinct would be to set up roundtrip tests between C# and Rust to ensure nothing is lost.

[shmuelie]

I only plan to automate the functions themselves. The structs and such I'd write by hand.

I plan to use JSON.NET under the hood, but for the best performance I don't want to use the reflection API.

[lilith]

I only plan to automate the functions themselves. The structs and such I'd write by hand.

Do you need me to make a C header file? What's the expected input and tool?

I plan to use JSON.NET under the hood, but for the best performance I don't want to use the reflection API.

Fair enough. I don't think reflection can handle discriminated unions in C# anyway (just F#) - so code generation might come in handy there.

[lilith]

I was overthinking this. C# needs to do very little parsing. Primarily, it needs to serialize - which is easier.

I could expose a 'mirror' API in libimageflow that just parses and re-serializes. If we disable whitespace and have the same rules for empty field omission, then it should be easy to compare the strings and verify that Rust is actually seeing everything that C# is sending. Thoughts?

[lilith]

Docs moved to: https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/imageflow/index.html

[shmuelie]

Do you need me to make a C header file? What's the expected input and tool?

A C or even C++ header would be easiest to work with. The easiest tool to work with would be the P/Onvoke Interop Assistant. It's not perfect in its generation but put its output is a good starting point.

Fair enough. I don't think reflection can handle discriminated unions in C# anyway (just F#) - so code generation might come in handy there.

Not sure myself so a good point.

[lilith]

Here's a C header: https://gist.github.com/nathanaeljones/74f58b2d3aee3831eb3efdd316db77e0

All 4 structs are opaque. Keeping track of 4 kinds of pointers isn't hard, but I discovered today that P/Invoke supports typed pointers via unsafe struct. http://www.codeproject.com/Articles/339290/PInvoke-pointer-safety-Replacing-IntPtr-with-unsaf

Also, good reference on GC/Pinvoke race conditions: https://blogs.msdn.microsoft.com/bclteam/2005/03/15/safehandle-a-reliability-case-study-brian-grunkemeyer/ I like GC.KeepAlive() for it's explicit handling of the issue.

I don't have the interop assistant handy - can you let me know what that header file produces?

[shmuelie]

I personally create structs that wrap the pointers usually, since it removes the need for unsafe and can have helper code.

I had to modify the header file slightly for the tool to be happy. Two issues where that I didn't have the imports for the int types (so I just stubbed them) and that just doing empty struct declarations is not enough for it (so I just typedef-ed them as void*). The modified header and output are here

[lilith]

It looks like we're getting "ref IntPtr". Any idea why? Typedef? I can fully erase the types if you want.

[shmuelie]

I made the structs void instead of void* and that got fixed. Updated the gist

[lilith]

I've added automatic header generation as part of the build process. Here's one with types erased:

https://gist.github.com/nathanaeljones/023fe6f3e271d6e72cf0b8c3de5013b2

[shmuelie]

As I'm going through the C header I have some questions.

Why do you use uint8_t in some locations for strings and in others char, what's the logic here? Trying to figure out how to marshal and how to deal with on the C# side.

[shmuelie]

Another question: What will be the name of the native library? The file.

[lilith]

imageflow.dll on windows, libimageflow.so and libimageflow.dylib elsewhere. Dllimport "imageflow" should work.

The latest AppVeyor build should have a working imageflow.dll

On Nov 24, 2016 12:41 PM, "Shmueli Englard" notifications@github.com wrote:

Another question: What will be the name of the native library? The file.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-262836589, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGlnwt7kYs91Uh7iMkhZj0p48tYhH95ks5rBeh3gaJpZM4IsGeU .

[shmuelie]

imageflow.dll on windows, libimageflow.so and libimageflow.dylib elsewhere. Dllimport "imageflow" should work.

The latest AppVeyor build should have a working imageflow.dll

Thanks. And my other question?

[lilith]

It should only be relevant for C users; treat all strings and buffers as unsigned byte arrays.

On Nov 24, 2016 2:22 PM, "Shmueli Englard" notifications@github.com wrote:

imageflow.dll on windows, libimageflow.so and libimageflow.dylib elsewhere. Dllimport "imageflow" should work.

The latest AppVeyor build should have a working imageflow.dll

Thanks. And my other question?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-262846042, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGln92xG6tnfWiFfvqsJEf5vQ3_mXaJks5rBgAdgaJpZM4IsGeU .

[shmuelie]

It should only be relevant for C users; treat all strings and buffers as unsigned byte arrays.

Ok. And strings are encoded as?

[lilith]

UTF-8/ASCII - everywhere except for imageflow_io_create_for_file, where filename needs to be the operating system code page used for fopen.

I'm writing some clarifications on strings. i8/u8/libc::c_uint8_t/libc::c_int8_t/libc::c_char are all signature compatible, but I didn't offer enough detail.

Unless there's a corresponding length, assume null-termination for strings in either direction. imageflow_context_error_and_stacktrace is the only function that both returns the length and writes a null-character/

[lilith]

Also, Happy Thanksgiving!

[shmuelie]

Also, Happy Thanksgiving!

Thanks, you too!

[lilith]

New ABI docs are uploading. c_char should now be used more consistently for null-terminated strings, u8 for length-only strings. UTF-8 except for the OS filename in imageflow_io_create_for_file.

[shmuelie]

Thanks. Makes that much easier to understand. Doing length only will be a pain in C# but can be done

[shmuelie]

Are all jobs blocking?

[lilith]

Currently only the calling thread is used.

On Nov 24, 2016 9:12 PM, "Shmueli Englard" notifications@github.com wrote:

Are all jobs blocking?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-262877609, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGln4ZirpFZ2HmQflKkXLvzh9jdCvksks5rBmAngaJpZM4IsGeU .

[lilith]

Yes. No async in 1.0.

On Nov 24, 2016 9:18 PM, "Nathanael Jones" nathanael.jones@gmail.com wrote:

Currently only the calling thread is used.

On Nov 24, 2016 9:12 PM, "Shmueli Englard" notifications@github.com wrote:

Are all jobs blocking?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-262877609, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGln4ZirpFZ2HmQflKkXLvzh9jdCvksks5rBmAngaJpZM4IsGeU .

[shmuelie]

Ok, simple enough.

[shmuelie]

JobIO is a struct based on the Header but docs say it's an enum?

[lilith]

The name is currently overloaded; it's an opaque pointer. (Also, Rust enums are unions, not c enums)

On Nov 28, 2016 9:21 PM, "Shmueli Englard" notifications@github.com wrote:

JobIO is a struct based on the Header but docs say it's an enum?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-263472369, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGln9C5E1R_n3aZ4UlRAWZ3_zwtwp5Tks5rC6gzgaJpZM4IsGeU .

[shmuelie]

The name is currently overloaded; it's an opaque pointer.

That works

(Also, Rust enums are unions, not c enums)

As the Header has them as enums I can treat them as such?

[lilith]

Rust enums annotated with #[repr(C)] are C enums; others are struct unions-ish. The header file is perfectly correct, but the docs categorize by Rust terms instead of C. I may have a fix for this.

On Nov 28, 2016 10:07 PM, "Shmueli Englard" notifications@github.com wrote:

The name is currently overloaded; it's an opaque pointer.

That works

(Also, Rust enums are unions, not c enums)

As the Header has them as enums I can treat them as such?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/imazen/imageflow/issues/67#issuecomment-263477625, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGln333iSkh5OfG4cW34rF6V_kqGOcHks5rC7MngaJpZM4IsGeU .

[shmuelie]

Or at least document "correction"

[lilith]

Should be improved: https://s3-us-west-1.amazonaws.com/imageflow-nightlies/master/doc/imageflow/index.html

[shmuelie]

Much clearer! Thanks.

May I suggest removing the C#/.NET references for how things work? While great for me, developers writing bindings for other languages might not get them and be confused.

[lilith]

I will definitely do that when we get other languages going.

I've just finished getting imageflow_tool.exe into a 'playable' state. Latest artifacts at https://ci.appveyor.com/project/imazen/imageflow

imageflow_tool examples --export-build-examples=all will create an examples folder with some scripts to run.

Would you mind giving that a look? I've written the last 4kloc on < 4hrs sleep, and would like another set of eyes on the tool before I publish the update.

[shmuelie]

I'll give it a try ASAP