cosyneco / MediaPipe.NET.Runtime

Native runtime package for MediaPipe.NET.
MIT License
20 stars 6 forks source link

fix incorrect ImageFrame memory ownership handling #11

Closed tobyxdd closed 2 years ago

tobyxdd commented 2 years ago

See https://github.com/vignetteapp/MediaPipe.NET/pull/29

Speykious commented 2 years ago

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

tobyxdd commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.