Adds mediapipe_text package

[craiglabenz]

Description

Adds the mediapipe_text package.

Design

The latest version of this PR builds on earlier drafts with several longterm-facing considerations, including better native memory management, improved abstractions to better support multiple tasks (e.g., reduced coupling), and an interface split for web-vs-IO implementations.

Better memory management

This PR does not use native finalizers, though they should still be straightforward to add in the future if/when that API is fully ironed out WRT out parameters in finalizer functions.

Objects created in native code

Dart wrappers around native objects typically created in native code now come with two primary constructors - one which accepts a native pointer and a default, unnamed constructor which accepts faked Dart objects. Imagining a native struct with a single integer field named index, Dart objects in this PR now assume the following form:

class Struct {
  Struct({required int index})
    : _index = index,
      _pointer = null;
  Struct.native(this._pointer);

  Pointer<bindings.Struct>? _pointer;

  int? _index;
  int get index => _index ??= _getIndex();
  int _getIndex() {
    if (_pointer == null || pointer.address == nullptr.address) {
      throw Exception('No native memory for Struct.index');
    }
    return _pointer!.ref.index;
  }
}

This implementation allows for surrounding Dart code to collect response objects from MediaPipe functions and only copy their data if it wants to read their values. If the surrounding Dart code does not want to read their values (because the results will merely be passed back to another MediaPipe function) then no memory is copied. Dart classes implemented in this way include Category and Classifications from mediapipe_core, and TextClassifierResult from mediapipe_task_text.

Objects created in Dart code

Objects that originate in Dart code and are passed to native code have a different design. These implement a copyToNative method which allocates the appropriate struct in native memory, copies all values onto that struct, and holds onto the pointer internally for later deallocation.

In practice, it is only the task Options class that follows this pattern, so currently only TextClassifierOptions in mediapipe_task_text exists; though each additional task is likely to add further classes.

Note that MediaPipe's pattern is to have task Options structs consist of inner structs, typically BaseOptions and ClassifierOptions. In Dart, these inner options are designed to know they only ever exist within an outer options class, and as such do not implement copyToNative. Instead, they implement assignToStruct and are passed the struct to which they should copy their values. They do not own that struct and are not responsible for freeing it - that is the job of the outer options class.

Web-vs-IO split

To support eventual web functionality, each package is now divided into two main worlds - io and web.

Shared interface

The common interface each world should implement is defined in an interface folder. This folder is not exported is nearly private. All classes are abstract and class names begin with I to indicate that they are interfaces. However, they are not strictly interfaces in implementation because they contain valid equality overrides and toString methods.

The the mediapipe_core package, the main barrel file looks like this:

// mediapipe_core.dart

export 'universal_mediapipe_core.dart'
    if (dart.library.html) 'src/web/mediapipe_core.dart'
    if (dart.library.io) 'src/io/mediapipe_core.dart';

Again, note that the interface is not exported. Also, note that at this time, src/web/ is empty. There is currently only an implementation within src/io/.

I/O implementation

The I/O implementation contains all the classes outlined in the Better memory management section earlier, as well as the concept of a TaskExecutor.

The TaskExecutor is a utility which pairs with a public-facing task-specific class (e.g., TextClassifier or TextEmbedder (coming soon!)) to actually run the specific task. In practice, this makes the TextClassifier's job to spin up a new isolate which will contain a TextClassifierExecutor instance, then send it messages (text strings) to be classified.

It remains to be seen what abstractions will be needed to have MediaPipe tasks be non-blocking on the web.

Better support for multiple tasks

The design of the abstract class TaskExecutor was ironed out while implementing the "embedText" task (which I currently have completed and stashed in a separate branch). This allows for minimum code duplication between multiple task executors, with individual classes (like TextClassifierExecutor) doing little more than supplying their correct methods from the ffigen-created bindings.

[Piinks]

More docs for sure to come, but top priority right now is how to structure this and #11 for licensing, adding third_party, and headers where appropriate.

[Piinks]

Still some comments from last review, but this can all likely wait pending #11, since it will need to be rebased on top of that when it lands. :)

[Piinks]

I may be misunderstanding, this is the text model, is the audio it is referring to here related to multimodal text classification?

[craiglabenz]

Oh this is a good point. This field is likely not necessary on TextClassifierResult and can be reserved for AudioClassifierResult and VisionClassifierResult.

[craiglabenz]

Looking into this more, because MP's behavior doesn't actually seem aligned with this docstring (which I copied from their Java documentation)

[craiglabenz]

This PR is now stale and redundant.

[craiglabenz]

Oh wait, no it's not.

[craiglabenz]

This should be ready for final 👀 , @Piinks. The very final commit addresses your question here.

[dcharkes]

Nice!

I have left comments on patterns that seem suspicious to me based on other FFI code that I've seen/written. I have suggested alternative patterns. It might be that the patterns you have chosen make sense based on how the code is going to be used. I don't see enough code in the codebase to suggest to me what the usage pattern is going to be.

General principles:

• Encapsulate FFI types, never show them to the user. The encapsulation classes implement Finalizable and have a dispose/free/release. These classes own the native memory, so allocate should be hidden in their (factory) constructors.
• Allocation logic abstracted in Allocators, because these are composable.
• Fakes should live in a test/ dir and implement a common interface.

It might be that these principles don't apply due to specific use cases. Happy to have a chat to understand the use cases better.

[dcharkes]

Nit: I'd either call ICategory BaseCategory and mark it as abstract base and use extend here, or call it ICategory and make it abstract interface class and use implements ICategory here.

I think a because you want to extend instead of implement Equateable, it should probably be base classes.

[craiglabenz]

Oh yes, that's a good point on naming. They should be Base* instead of I*. I went back and forth on class location and names so many times that I lost track of that detail.

You mentioned using the base class modifier. Which of its features do you expect will be helpful here? (Is it the lack of implementation outside of the library?)

[dcharkes]

Would it be possible to put the fakes in the test/ instead of src/io dir?
This would also enable reusing fakes across the IO and web implementation.

You already have a common interface ICategory that you implement. So the fakes could be a different class.

Or is there a reason why this is not possible?

[craiglabenz]

This would also enable reusing fakes across the IO and web implementation.

Good point - moving fake constructors into a location that supports this is desirable.

That point aside, I put the constructors here to allow end-user code to more easily test their integration with MediaPipe. My inspiration was the shared_preferences library, which put its testing helpers alongside the main library source. I'm open to other ideas, but am keen to preserve an easy testing story for apps using this library.

[dcharkes]

Hm, I'd maybe put them in a separate file and surface them through package:mediapipe-core/fakes.dart so that they don't show up in auto complete etc. in the users non-test code.

But I'm not a package developer, so if someone else has better best practises, please follow those!

[Piinks]

I think the intention of having publicly available fakes is to allow devs to use them in their own tests, rather then having to write their own fakes.

Rather than including it in the Category class, we usually make a FakeCategory class that extends Category.. would that be better?

[craiglabenz]

I spent a while yesterday and this morning exploring different solutions to this problem. I attempted to factor out a FakeCategory class, but that ran into challenges as various tiers of the class hierarchy had different ideas about what constructors made sense to declare and, as far as I could tell, something awkward was going to have to leak out somewhere.

For now, I've simply dropped the .fake name on that constructor and made it the default, unnamed constructor. Separately, in the IO implementation, there is still a named .native constructor which accepts a pointer. But, this .native named constructor is not visible in normal code completion because it is not declared on the stub file that is exported - only on the native implementation. Later, the web version will add its own special constructor which will also not be visible to surrounding code.

Thus, for a class like Category, end users will only see a plain Dart class with a typical, unnamed constructor that accepts raw values. They will be able to construct these freely in their own tests, and in production code will be none the wiser to receiving classes that are mere wrappers around native memory (or, eventually, data that arrived from JS interop).

[dcharkes]

Design question: Should this constructor take ownership of the pointer?

If we intend on making this class Finalizable and attaching a NativeFinalizer, then we should.
Then this class should have a delete() or release() method.
And then we should not be freeing the backing memory outside the class with using( in arena.

If we do not intend on making this class take ownership, then we should consider now allowing instances but making it an extension type on Pointer<bindings.Category>.
I believe this is not feasible because you implement an interface and you want to pass objects around.

[craiglabenz]

We could potentially use the Finalizable pattern for some of the MediaPipe objects, but everything within mediapipe_core is owned by higher level task result objects which have their own native methods to release memory. (Those methods have error_msg "out" parameters, which is what is currently blocking use of the Finalizable pattern.)

So, your question is applicable to classes like TextClassifierResult, which does own its pointer and implements a dispose method.

If the above all makes sense to you, I will move forward updating the docstrings to indicate this memory ownership, but leave this code as is.

[dcharkes]

That makes sense, if you intend some things to be used by users and other things to be just internal API.
(We should then probably try to answer some questions about how to hide the internal API from users.) String with docstrings sgtm.

[craiglabenz]

Docstrings added.

[dcharkes]

Iterable<Category> get categories => ...

This would enable you to create an iterator in _getCategories that lazily reads native memory instead of eagerly.

(Unless you expect categories to always be a small list.)

If Classifications is to implement Finalizable and have a native finalizer attached, then such iterator has to have a field pointing back to Classifications so that it stays alive until you're done iterating.

[craiglabenz]

I do expect the lists to always be small, but it still may be worth exposing an iterator to not always force complete hydration. I think this is probably a small optimization that is technically an improvement, so I'll make it.

Regarding the Finalizable angle and fields holding refs to keep things alive - as I said in another response - this object is owned by a wrapper object which does own and manage its own memory which cascades down to every Category instance.

[craiglabenz]

Converted!

[dcharkes]

Can you explain to me why we would like to expose the bindings.Struct types here?

I was hoping that we would encapsulate the native types inside Dart types and not leak the Pointer<buildings.Struct> type at all to the user ever.
I'm not sure if this is possible though, it depends on the use cases.

Also, because dart:ffi in general does not allow polymorphism on NativeTypes, you won't be able to do things such as calloc<NativeOptions>(), which means you'll make sub classes for every concrete type flowing into NativeOptions.

Consider whether this class should be hidden from users in mediapipe_core.dart.
(If you want to expose some common interface for task executors, split up this class in TaskExecutor and TaskExecutorBase, and don't export the latter in mediapipe_core.dart.)

[craiglabenz]

Can you explain to me why we would like to expose the bindings.Struct types here?

This connects to my attempt to have typed native structs for objects that originate in Dart code. An example is the TaskOptions subclasses that a Flutter application will instantiate to pass down to MediaPipe. My goal is to have those interactions typed all the way down to the ffigen-created bindings.

Doing this implies typed signatures of methods like createWorker and createResultsPointer in classes that extend TaskExecutor. It is potentially true that if those methods accepted Pointer<void> and cast their parameters locally, these types could be done away with. But, we know what the type of the pointers are, so at some point we would have to pretend we know less than we do, lest the analyzer would complain to the tune of

The argument type 'Pointer<TextClassifierOptions>' can't be assigned to the parameter type 'Pointer<Void>'. 

I was hoping that we would encapsulate the native types inside Dart types

Eventually, the bindings will accept a pointer to a typed struct. If I'm understanding everything correctly, we can acknowledge what that type is through the whole call chain, or treat it as a Pointer<void> until the last second when we cast it. Is that understanding correct?

Also, because dart:ffi in general does not allow polymorphism on NativeTypes, you won't be able to do things such as calloc(), which means you'll make sub classes for every concrete type flowing into NativeOptions.

Subclasses for every concrete type is already happening, because each TaskExecutor class implements its own fully unique, task-specific methods with varying signatures. For example, TextClassifierExecutor will implement classify(String text), whereas other executors will implement other functions (sometimes plural) with different signatures that cannot be easily standardized in a parent class.

I had hoped to avoid subclasses for every type, but I believe this detail would make it challenging and convoluted to do so.

[dcharkes]

I agree that if we're giving users Pointers, we should give them the correct type arguments and not Pointer<Void>. (Though what I had hoped was that we never give users Pointers only classes that internally hold a pointer.)

I had hoped to avoid subclasses for every type, but I believe this detail would make it challenging and convoluted to do so.

That doesn't really work when wrapping FFI code, because most of the FFI code cannot be polymorphic.

If you have a lot of repetitive code, consider writing a code generator! 🙈

[craiglabenz]

To clarify, the TaskExecutor subclasses are all implementation details that an app developer will never know about. They are completely concealed from actual Flutter code.

[dcharkes]

As an alternative, you can use part files to have separate files but a single scope.

[craiglabenz]

Indeed. I'm not sure this question has high stakes, but I may still pursue a small change like this.

[craiglabenz]

After realizing that all details unique to the native implementation are hidden by default (by way of not appearing in the stub file), I no longer consider this a value add and have removed this TODO.

[dcharkes]

In standalone dart, there is no such thing as a build dir.
Also, in Flutter, the build dir is configurable.

Consider writing the logs to buildConfig.outDir.resolve('build-log.txt').
And consider appending new logs instead of creating a file for every millisecond. (Also you're overwritting multiple logs in the same millisecond currently.)

Also, consider writing out the logs to stdout in addition to a file.

[craiglabenz]

In standalone dart, there is no such thing as a build dir. Also, in Flutter, the build dir is configurable.

The mediapipe-task-text package can't be used in standalone Dart - though maybe it should be able to 🤔 . Either way, the second detail is potentially trouble! I'll move these logs to the location you suggested.

And consider appending new logs instead of creating a file for every millisecond. (Also you're overwritting multiple logs in the same millisecond currently.)

Admittedly, this part is a little confusing. I write logs to the file every time because, during development, this whole process was crashing on me for opaque reasons and I wanted to capture as much information as possible. Regarding overwriting - that value is captured right at the start of the whole process and does not change, so I don't believe any logs will collide (unless they truly start in the same millisecond).

Also, consider writing out the logs to stdout in addition to a file.

I got into this crazy business because this process's stdout is not exposed to the developer (flutter/flutter#136386).

Also, all of this logging noise is not really code I ultimately want to keep in here. But, I've removed it twice so far and both times later had to add it back - so for now I just left it in to support further development leading up to our initial release.

[Piinks]

What is this file for?

[craiglabenz]

It is a placeholder for where the web implementation will go. I'm not actually sure whether it makes sense to commit it now or not - maybe it doesn't 🤷

[Piinks]

Why is this not immutable?

[craiglabenz]

Just the _isClosed flag that marks whether the native memory has been released. Is there another way to handle that?

[Piinks]

Same here

[craiglabenz]

Same as with mediapipe_core/src/web/mediaipipe_core.text - it is a placeholder for where the web implementation will go. I'm not actually sure whether it makes sense to commit it now or not - maybe it doesn't 🤷

[Piinks]

I think the intention of having publicly available fakes is to allow devs to use them in their own tests, rather then having to write their own fakes.

Rather than including it in the Category class, we usually make a FakeCategory class that extends Category.. would that be better?

[Piinks]

What is this for? I am not sure. 🙃

[craiglabenz]

The following code is in task_executor.dart:

abstract class TaskExecutor<..., Result extends TaskResult> {}

I could be talked out of this, but felt it was a little nicer to make the following impossible:

class TextClassificationExecutor extends TaskExecutor<..., WrongType> {}

Dismissing review as Addressed.

[charlieforward9]

If the surrounding Dart code does not want to read their values (because the results will merely be passed back to another MediaPipe function) then no memory is copied.

Is it possible to do this with a method channel implementation as well?

I am implementing a processing engine in native iOS that includes video capture, inference and cloud storage for better performance and less copying of data to Flutter. I did not consider FFI until I started following this along.

Any help would be greatly appreciated. Thank you.