fix incorrect ImageFrame memory ownership handling

[tobyxdd]

See cosyneco/MediaPipe.NET#29

[sr229]

I assume you would want to copy this pointer before you want to pass this to .NET. Has this been tested and memory profiled?

[Speykious]

I'm really not sure of this change.

If anything, providing a function to create a new image frame with a copied pixel data that MediaPipe owns is a very good idea - however, removing the option to move an existing pixel data? I don't know.

[Speykious]

Do you know if it's possible to leave a byte[] to unmanaged memory in C#?

[tobyxdd]

Do you know if it's possible to leave a byte[] to unmanaged memory in C#?

I'm not exactly familiar with C#, but perhaps something like GCHandle.Alloc(someArr, GCHandleType.Pinned) would work?

[tobyxdd]

But tbh I think that's beside the point. If pixel_data is not copied, either way you will need some extra work to manually ensure it's not freed before MediaPipe is done using it, and that it's freed afterwards (to prevent memory leaks). Much easier to just make a copy.

[Speykious]

But tbh I think that's beside the point. If pixel_data is not copied, either way you will need some extra work to manually ensure it's not freed before MediaPipe is done using it, and that it's freed afterwards (to prevent memory leaks). Much easier to just make a copy.

It is indeed easier to copy the pixel data, but definitely not more performant. I would prefer, as I said, to have both options available, so going that extra mile to avoid one more O(n) operation is fine by me.

I'm familiar with the notion of ownership and borrowing thanks to Rust, so something we can do is to add a property on ImageFrame that asserts whether or not it owns the pixel data.

[sr229]

Considering we're aiming for C99-like ABI compatibility, this is alright. ImageFrame* is easier to interpolate than unique_ptr.

[Speykious]

I'll allow the replacement. I think the only way to make the native side own a pixel data from the C# side is by creating an unmanaged array, which requires to either make our own class or to use the UnmanagedArray library, which is kind of a bit of work compared to normal byte[]s. Not to mention that I think it wouldn't work well with libraries such as SeeShark where we already don't have any problem in the first place because of how we deal with the memory, but where it could still very likely bite us back in the future.

Just change the name of the function and we good :)

[Speykious]

I approb

[sr229]

Thanks for your contributions! Next time, we encourage you to sign your commits so we can authenticate it's really yours to prevent commit masquerading.