-
Notifications
You must be signed in to change notification settings - Fork 43
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
Ffigen doesn't generate typedefs compatible with NativeCallable's APIs #431
Comments
Hello! So I checked ffigen's config functions -> expose-typedefs summary and it still would not generated the required typedefs The usecase I'm currently using to test this is using the whisper.h file with the following ffigen.yaml output: 'lib/src/whisper_bindings.dart'
headers:
entry-points:
- 'whisper.h'
include-directives:
- '**whisper.h'
name: 'WhisperBindings'
structs:
dependency-only: opaque
functions:
include:
- '.*'
symbol-address:
include:
- '.*'
expose-typedefs:
include:
- 'whisper_.*_callback'
description: 'Bindings to OpenAI Whisper model in cpp'
comments:
style: any
length: full I would think that maybe a similar option but in the typedefs section of the config would probably work for this issue. |
@liamappelbe could you have a look at this? |
@Piero512 I don't really understand what you're trying to do or what the problem is. Can you give a code example? |
Hello! I don't know exactly how to summarize it, but the way I ran into this issue is the following. I am in the progress on writing bindings to the aforementioned header file and while iterating, I decided that I wanted to retrieve the segments as soon as they get discovered. The library exposes users callbacks in the struct whisper_full_params struct whisper_full_params {
int n_threads;
int n_max_text_ctx; // max tokens to use from past text as prompt for the decoder
int offset_ms; // start offset in ms
int duration_ms; // audio duration to process in ms
// Rest of the header ommited for brevity
// called for every newly generated text segment
whisper_new_segment_callback new_segment_callback;
void * new_segment_callback_user_data;
// called on each progress update
whisper_progress_callback progress_callback;
void * progress_callback_user_data;
} and I thought that the NativeCallable class could do this work, since (unfortunately) this lib calls those callbacks from worker threads managed by itself.
typedef whisper_progress_callback = ffi.Pointer<
ffi.NativeFunction<
ffi.Void Function(
ffi.Pointer<whisper_context> ctx,
ffi.Pointer<whisper_state> state,
ffi.Int progress,
ffi.Pointer<ffi.Void> user_data)>>; but the type required for NativeCallback to generate that pointer in the nativeFunction getter is the following. typedef whisper_progress_callback_nc = ffi.Void
Function(
ffi.Pointer<whisper_context>,
ffi.Pointer<whisper_state>,
ffi.Int progress,
ffi.Pointer<ffi.Void>
); So I would like a solution where I don't have to manually write the typedef for that callback. Since FFI is not dynamic, I would guess the way to go would be to add a new setting to ffigen to generate the required typedefs so we can use this API more ergonomically. But any other solution is fine. |
Ok, sounds like this is primarily a feature request for ffigen, so I've transferred the issue. The request is for ffigen to detect typedefs of function pointers, and define a second typedef that unwraps the @Piero512 For issue (a), where the listener constructor isn't doing the type inference you expect, can you file a separate issue with code for this (back on the dart-lang/sdk repo)? Is this something that worked under |
Hopefully I find some free time this week to create a minimum reproducible example for issue A, but then, if those typedefs would be generated, I am sure the issue A would become moot. |
@Piero512 @liamappelbe RE:dart-archive/ffigen#621 (comment), do you have same examples of the use of the typedefs for both the pointers and the native functions? (In order to decide how common they are and what names to use.) Maybe @mannprerak2 also has something to say about this, as he added the typedefs earlier. typedef CXInclusionVisitor = ffi.Pointer<ffi.NativeFunction<CXInclusionVisitor_function>>;
typedef CXInclusionVisitor_function = ffi.Void Function(
CXFile included_file,
ffi.Pointer<CXSourceLocation> inclusion_stack,
ffi.UnsignedInt include_len,
CXClientData client_data); vs (breaking change) typedef CXInclusionVisitor_pointer = ffi.Pointer<ffi.NativeFunction<CXInclusionVisitor>>;
typedef CXInclusionVisitor = ffi.Void Function(
CXFile included_file,
ffi.Pointer<CXSourceLocation> inclusion_stack,
ffi.UnsignedInt include_len,
CXClientData client_data); |
No input from my side, when we added typedefs, all the types were inline since I wasn't aware of such usecase. As per the naming, maybe we should keep the name used by C intact, since C is exposing the typedef to a function pointer itself, but then we are further decomposing it to another typedef. This approach is also non breaking. |
Okay, maybe we should just land as is then. |
* Fix #614 * Update test expectations * Hacky fix * Refactor * Fix analysis * Fix analysis * Fix analysis
Hello!
Today I was trying out the new NativeCallable APIs and stumbled upon on a kinda annoying issue.
NativeCallable T generic parameter has this problem with ffigen that it requires specifying a function that only uses dart:ffi types, but there isn't currently a way to get one of these, since the generated typedefs usually come from pointer functions.
I think the NativeCallable API should not need to be changed, but say, a static function that removes the pointer and nativefunction from the signature would be useful? Although I do believe that the API should have bound T type to extend NativeFunction.
I haven't checked if ffigen's config section functions -> expose-typedefs would generate the required typedefs, but if it does, it should be documented in the NativeCallable class too.
The text was updated successfully, but these errors were encountered: