Conversation
|
@JasperDeSutter I've recently implemented this differently for What do you think, is it advantageous to have an ever-so-slightly-faster API that doesn't call For the cases where we have to call acquire ( But I didn't manage to write such a transmute since What do you think? |
|
Nice catch about the bitmap api, some of these gotchas are easy to miss in the documentation. The current design which is based on camera/video usage where frames are transmitted frequently and usually has a need to be well-optimised. You wouldn't normally lock the buffer if you can synchronously process or memcpy it before the system callback returns. However I didn't do any profiling to see if this actually matters at all so it is premature optimization. I agree with just removing the *Ref system if it leads to a cleaner API, let's not delve into more unsafe than is needed with the transmutes. Having the reference ( |
I wouldn't be surprised if they added that doc bit later, certain functions ( Lets get this PR in with all its documentation improvements and a leak-fix for EDIT: In that sense it's perhaps still worth unifying both APIs, but we'll make sure that |
…ng in docs
According to [`AndroidBitmap_getHardwareBuffer`]:
outBuffer On success, is set to a pointer to the AHardwareBuffer
associated with bitmap. This acquires a reference on the
buffer, and the client must call AHardwareBuffer_release
when finished with it.
By returning the `outBuffer` wrapped in `HardwareBuffer` we represent a
weak instead of strong reference, and don't `_release` it on `Drop`. If
we instead wrap it in a `HardwareBufferRef` we get the desired `release`
semantics on `Drop` and prevent resource leaks for the user.
Note that a similar API on [`AImage_getHardwareBuffer`] does **not**
aquire a strong reference, and should remain returning a weak
`HardwareBuffer`.
This feat isn't very well described in the docs, so we now echo what was
only stated on `HardwareBufferRef` across the `from_ptr()`/`from_jni()`
implementations.
Finally, just like `ForeignLooper` and `NativeWindow` this
`acquire`+`release` kind of type can implement `Clone` to allow the user
to take advantage of internal refcounting instead of having to re-wrap
it in `Rc` or `Arc`.
[`AndroidBitmap_getHardwareBuffer`]: https://developer.android.com/ndk/reference/group/bitmap#androidbitmap_gethardwarebuffer
[`AImage_getHardwareBuffer`]: https://developer.android.com/ndk/reference/group/media#aimage_gethardwarebuffer
ec72706 to
e88d142
Compare
Depends on #294, #295
According to
AndroidBitmap_getHardwareBuffer:By returning the
outBufferwrapped inHardwareBufferwe represent a weak instead of strong reference, and don't_releaseit onDrop. If we instead wrap it in aHardwareBufferRefwe get the desiredreleasesemantics onDropand prevent resource leaks for the user.Note that a similar API on
AImage_getHardwareBufferdoes not aquire a strong reference, and should remain returning a weakHardwareBuffer.This feat isn't very well described in the docs, so we now echo what was only stated on
HardwareBufferRefacross thefrom_ptr()/from_jni()implementations.Finally, just like
ForeignLooperandNativeWindowthisacquire+releasekind of type can implementCloneto allow the user to take advantage of internal refcounting instead of having to re-wrap it inRcorArc.