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 flow_graph_* and flow_node_* 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.

[lilith]

Woot! This is still under development: https://github.com/jaredpar/pinvoke

Also, the integration server is now running RTM 1.0.

[shmuelie]

@nathanaeljones That's awesome!

[lilith]

I'd like to spike a partial wrapper for Imageflow that doesn't do any abstractions at the P/Invoke layer, but just uses IntPtr. Do you have the raw output from a P/Invoke generation tool handy?

[shmuelie]

Not home tonight but tomorrow night I should be able to get that to you 👍

[shmuelie]

About two months late but... https://gist.github.com/SamuelEnglard/557c7b549a2a30a245a1b8a21fd9d2a7
Includes C# and the C header used

[RossLieberman]

Thanks @SamuelEnglard . I was curious on the project and checked in yesterday and saw there wasn't much movement lately. I'm interested in these C# bindings you've been working on. I wish I had time to test or help out. But I'm happy to see your commit.

[shmuelie]

@RossLieberman Yeah... Massive shake ups at work have me falling behind on lots of stuff. Finally trying to get back into it

[CShepartd]

Thanks for work on .Net bind @SamuelEnglard
When do u plan publish on nuget?

[shmuelie]

@CShepartd planning on publishing to NuGet once their is a public API. The current code is all internal. Can't really give a time line on that.

[lilith]

@SamuelEnglard Thank you, but that's not the right header; you should be using https://github.com/imazen/imageflow/blob/master/bindings/headers/imageflow_pinvoke.h

[shmuelie]

@nathanaeljones I had feeling it wasn't but figured better try and redo. Leaving work now, when I get home I'll regenerate (and check the primary libraries signatures)

[shmuelie]

@nathanaeljones Updated the gist

[OpenSpacesAndPlaces]

@SamuelEnglard What's the best path for using this in a .Net Core app in the meantime?
We're converting over a .Net Framework CMS to .Net Core, and we used ImageResizer pretty extensively.

I expect until this becomes more official I'll at least need to create some kind of routing rules/middleware on my own - no worries there.

I was able to take the binding work here, target .Net 2.0 Core Preview 2 - and then create my own local nuget package for import. That all worked fine:
https://github.com/imazen/imageflow/tree/master/bindings/dotnet/Imageflow

As you've already mentioned, and I've seen in the source - the C# bindings don't seem to have any publicly exposed methods. In destination project I'm not able to-do anything besides see the namespaces exist effectively.

I don't have an issue with having to-do build out to make this work, but I'm trying to avoid unnecessary work and/or false starts.

Thanks in advance for the help!

[shmuelie]

@OpenSpacesAndPlaces your best bet at this time is to use the HTTP server (imageflow_server) for now. It's not the best solution but for a CMS that should work.

[shmuelie]

As a general update:

Current issue I'm dealing with is how to work with images embedded in the JSON stream. Since one of the big goals of imageflow is performance I do not want to do it the quick and easy way. Right now that means I may have to write my own JSON parsing system. Obviously don't want to do that so been playing around trying to see what options I have.

My goal for the C#/.NET bindings is that .NET users can use this with no understanding of native code ideas. That means lots of wrapping logic. As noted, the current code is all the raw wrappings, P/Invoke code that I do not want end users to deal with. I am going to abstract that away with more .NET styled APIs.

[lilith]

Maybe we should table implementing embedded images, and assume that we'll use the job api instead of the build api (the job api takes pointers to buffers).

[shmuelie]

That's been one thought I had as well. I may just do that

[lilith]

I think it's very acceptable for 1.0 to only support byte arrays for input and output.

[CShepartd]

I had the same problem like @OpenSpacesAndPlaces and give up with ImageFlow and created own multiplatform P/Invoke wrapper of FreeImage (based on https://github.com/matgr1/FreeImage-dotnet-core). I needed only resize images and working well

ImageFlow is one of the biggest disappointment on my list (slow development, bugs, no goals/milestones etc.). Just sad

[lilith]

Today I wrote a minimal C# wrapper for Imageflow: https://github.com/imazen/imageflow/tree/master/bindings/dn

This exposes both the JSON graph API and the ImageResizer4 quersytring API.

@CShepartd I'm sorry that imageflow has been disappointing for you. Is this primarily about the C# wrapper, or do you have concerns about the project itself? We do have a roadmap, and bugs are to be expected. We anticipated that more people would give imageflow a try and provide feedback, but that volume has been extremely low.

[lilith]

Note: 9700421

• Modify signature of imageflow_io_create_for_file(context, mode, filename) -> * imageflow_job_io (Remove imageflow_cleanup_with parameter)
• Modify signature of imageflow_io_create_from_buffer(context, buffer, buffer_byte_coutn, lifteime) (Remove imageflow_cleanup_with parameter)
• Remove CleanupWith enumeration (Just don't reuse contexts much)
• Mark imageflow_context_error_code(context) as unstabilized (don't depend on this)
• Remove imageflow_context_raise_error (no usage scenario)
• Remove imageflow_context_add_to_callstack (no usage scenario)
• Remove imageflow_context_clear_error(context) -> void (replaced with try_clear)

Improvements to ABI

• Add panic recovery to all methods
• Add null-parameter checking to all methods
• And most-significant-bit checking (for size_t) to all methods
• Add imageflow_context_error_as_exit_code(context) -> i32 (and stabilize values)
• Add imageflow_context_error_as_http_code(context) -> 32 (and stabilize values)
• Add imageflow_context_error_try_clear(context) -> bool
• Add imageflow_context_error_recoverable(context) -> bool

[lilith]

Quick, any reason not to merge context and job? Contexts are smaller/quicker now. JobContext would be simpler. Io objects would get an io_id upon creation.

Can you think of any use case that would suffer from this change?

/cc: @SamuelEnglard

[shmuelie]

Nothing off the top my head

[lilith]

Thoughts on this API?

bool imageflow_abi_compatible(uint32_t imageflow_abi_ver_major, uint32_t imageflow_abi_ver_minor);

uint32_t imageflow_abi_version_major(void);

uint32_t imageflow_abi_version_minor(void);

void* imageflow_context_create(uint32_t imageflow_abi_ver_major, uint32_t imageflow_abi_ver_minor);

bool imageflow_context_begin_terminate(void* context);

void imageflow_context_destroy(void* context);

bool imageflow_context_has_error(void* context);

bool imageflow_context_error_recoverable(void* context);

bool imageflow_context_error_try_clear(void* context);

int32_t imageflow_context_error_code(void* context);

int32_t imageflow_context_error_as_exit_code(void* context);

int32_t imageflow_context_error_as_http_code(void* context);

bool imageflow_context_error_write_to_buffer(void* context, char* buffer, size_t buffer_length, size_t* bytes_written);

bool imageflow_context_print_and_exit_if_error(void* context);

bool imageflow_json_response_read(void* context, void const* response_in, int64_t* status_as_http_code_out, uint8_t const** buffer_utf8_no_nulls_out, size_t* buffer_size_out);

bool imageflow_json_response_destroy(void* context, void* response);

void const* imageflow_context_send_json(void* context, char const* method, uint8_t const* json_buffer, size_t json_buffer_size);

bool imageflow_context_add_file(void* context, int32_t io_id, imageflow_direction direction, imageflow_io_mode mode, char const* filename);

bool imageflow_context_add_input_buffer(void* context, int32_t io_id, uint8_t const* buffer, size_t buffer_byte_count, imageflow_lifetime lifetime);

bool imageflow_context_add_output_buffer(void* context, int32_t io_id);

bool imageflow_context_get_output_buffer_by_id(void* context, int32_t io_id, uint8_t const** result_buffer, size_t* result_buffer_length);

void* imageflow_context_memory_allocate(void* context, size_t bytes, char const* filename, int32_t line);

bool imageflow_context_memory_free(void* context, void* pointer, char const* filename, int32_t line);

[lilith]

Implemented.

[shmuelie]

Looks good to me

[lilith]

@SamuelEnglard I'm planning to move the new .NET bindings to their own repository (We now have a versioned ABI, and a separate repo allows for a much quicker release cycle for the bindings).

To reduce confusion I'd like to move your bindings elsewhere as well.

[lilith]

The bindings are now on NuGet https://www.nuget.org/packages/Imageflow.Net/
You must also install the correct Imageflow.NativeBindings package
https://www.nuget.org/packages?q=Imageflow.NativeRuntime

The project has been moved to https://github.com/imazen/imageflow-dotnet
The plan is to test the latest version of the bindings from the Imageflow CI, but allow the bindings a faster test/release cycle (2-minute CI builds instead of 60m).

[lilith]

Please test out Imageflow.Net and provide feedback at https://github.com/imazen/imageflow-dotnet/issues