Skip to content

Commit e3f9225

Browse files
authored
ndk/media/image_reader: Don't assume ownership of NativeWindow (#418)
When implementing proper ownership `acquire()` and `release()` semantics for `NativeWindow` in #207, I thought I had checked all existing calls to `NativeWindow::from_ptr()` to make sure that they return a pointer that we get ownership over, and have to clean up ourselves. This turns out to [not be the case for `AImageReader_getWindow()`]: The ANativeWindow is managed by this image reader. Do NOT call ANativeWindow_release on it. Instead, use AImageReader_delete. And can be trivially reproduced by creating an `ImageReader` and calling `.get_window()`. Dropping the `NativeWindow` is fine but subsequently dropping the `ImageReader` results in a NULL-pointer SEGFAULT. For now calling `clone_from_ptr()` is enough to first acquire an extra reference on the pointer so that ownership remains balanced, but in the future we'd like to investigate a [new non-owned `NativeWindow` type similar to `HardwareBuffer`]. As of writing the following calls to `from_ptr()` remain, that are all confirmed to transfer ownership and require cleanup via `_release()`: - `ASurfaceTexture_acquireANativeWindow()`; - `AMediaCodec_createInputSurface()`; - `AMediaCodec_createPersistentInputSurface()`; - `ANativeWindow_fromSurface()`. [not be the case for `AImageReader_getWindow()`]: https://developer.android.com/ndk/reference/group/media#aimagereader_getwindow [new non-owned `NativeWindow` type similar to `HardwareBuffer`]: #309
1 parent 3114143 commit e3f9225

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

ndk/src/media/image_reader.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,13 @@ impl ImageReader {
174174
MediaError::from_status(status)
175175
}
176176

177+
/// Get a [`NativeWindow`] that can be used to produce [`Image`]s for this [`ImageReader`].
178+
///
179+
/// <https://developer.android.com/ndk/reference/group/media#aimagereader_getwindow>
177180
pub fn get_window(&self) -> Result<NativeWindow> {
178181
unsafe {
179182
let ptr = construct_never_null(|res| ffi::AImageReader_getWindow(self.as_ptr(), res))?;
180-
Ok(NativeWindow::from_ptr(ptr))
183+
Ok(NativeWindow::clone_from_ptr(ptr))
181184
}
182185
}
183186

0 commit comments

Comments
 (0)