From d8f89322ffa3cdfcc9ef78134819f143a76f1900 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Fri, 27 Aug 2021 00:51:13 +0200 Subject: [PATCH] ndk/native_window: Use `release`/`acquire` fns for `Drop` and `Clone` Much like `ALooper` and `AHardwareBuffer` the reference counter of `ANativeWindow` must be properly incremented on `Clone`, in turn allowing us to also `_release` the window as soon as it is dropped. --- ndk-glue/src/lib.rs | 15 +++++++++------ ndk/CHANGELOG.md | 1 + ndk/src/media/media_codec.rs | 4 ++-- ndk/src/native_window.rs | 32 +++++++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/ndk-glue/src/lib.rs b/ndk-glue/src/lib.rs index 45540d8b..ecbe6ae5 100644 --- a/ndk-glue/src/lib.rs +++ b/ndk-glue/src/lib.rs @@ -167,7 +167,7 @@ pub unsafe fn init( let activity = NativeActivity::from_ptr(activity); ndk_context::initialize_android_context(activity.vm().cast(), activity.activity().cast()); - NATIVE_ACTIVITY = Some(activity); + NATIVE_ACTIVITY.replace(activity); let mut logpipe: [RawFd; 2] = Default::default(); libc::pipe(logpipe.as_mut_ptr()); @@ -207,7 +207,7 @@ pub unsafe fn init( { let mut locked_looper = LOOPER.lock().unwrap(); - *locked_looper = Some(foreign); + locked_looper.replace(foreign); signal_looper_ready.notify_one(); } @@ -275,7 +275,10 @@ unsafe extern "C" fn on_window_focus_changed( } unsafe extern "C" fn on_window_created(activity: *mut ANativeActivity, window: *mut ANativeWindow) { - *NATIVE_WINDOW.write().unwrap() = Some(NativeWindow::from_ptr(NonNull::new(window).unwrap())); + NATIVE_WINDOW + .write() + .unwrap() + .replace(NativeWindow::clone_from_ptr(NonNull::new(window).unwrap())); wake(activity, Event::WindowCreated); } @@ -300,7 +303,7 @@ unsafe extern "C" fn on_window_destroyed( wake(activity, Event::WindowDestroyed); let mut native_window_guard = NATIVE_WINDOW.write().unwrap(); assert_eq!(native_window_guard.as_ref().unwrap().ptr().as_ptr(), window); - *native_window_guard = None; + native_window_guard.take(); } unsafe extern "C" fn on_input_queue_created( @@ -313,7 +316,7 @@ unsafe extern "C" fn on_input_queue_created( // future code cleans it up and sets it back to `None` again. let looper = locked_looper.as_ref().expect("Looper does not exist"); input_queue.attach_looper(looper, NDK_GLUE_LOOPER_INPUT_QUEUE_IDENT); - *INPUT_QUEUE.write().unwrap() = Some(input_queue); + INPUT_QUEUE.write().unwrap().replace(input_queue); wake(activity, Event::InputQueueCreated); } @@ -326,7 +329,7 @@ unsafe extern "C" fn on_input_queue_destroyed( assert_eq!(input_queue_guard.as_ref().unwrap().ptr().as_ptr(), queue); let input_queue = InputQueue::from_ptr(NonNull::new(queue).unwrap()); input_queue.detach_looper(); - *input_queue_guard = None; + input_queue_guard.take(); } unsafe extern "C" fn on_content_rect_changed(activity: *mut ANativeActivity, rect: *const ARect) { diff --git a/ndk/CHANGELOG.md b/ndk/CHANGELOG.md index 83faeebe..d4059a26 100644 --- a/ndk/CHANGELOG.md +++ b/ndk/CHANGELOG.md @@ -4,6 +4,7 @@ - **Breaking:** `Configuration::country()` now returns `None` when the country is unset (akin to `Configuration::language()`) - Add `MediaCodec` and `MediaFormat` bindings. (#216) - **Breaking:** Upgrade to [`ndk-sys 0.4.0`](../ndk-sys/CHANGELOG.md#040-TODO-YET-UNRELEASED) and use new `enum` newtype wrappers. (#245) +- ndk/native_window: Use `release`/`acquire` for `Drop` and `Clone` respectively. (#207) # 0.6.0 (2022-01-05) diff --git a/ndk/src/media/media_codec.rs b/ndk/src/media/media_codec.rs index 1b6b54cb..5cc860ca 100644 --- a/ndk/src/media/media_codec.rs +++ b/ndk/src/media/media_codec.rs @@ -384,13 +384,13 @@ impl MediaCodec { } #[cfg(feature = "api-level-26")] - pub fn set_input_surface(&self, surface: NativeWindow) -> Result<()> { + pub fn set_input_surface(&self, surface: &NativeWindow) -> Result<()> { let status = unsafe { ffi::AMediaCodec_setInputSurface(self.as_ptr(), surface.ptr().as_ptr()) }; NdkMediaError::from_status(status) } - pub fn set_output_surface(&self, surface: NativeWindow) -> Result<()> { + pub fn set_output_surface(&self, surface: &NativeWindow) -> Result<()> { let status = unsafe { ffi::AMediaCodec_setOutputSurface(self.as_ptr(), surface.ptr().as_ptr()) }; NdkMediaError::from_status(status) diff --git a/ndk/src/native_window.rs b/ndk/src/native_window.rs index 05de25cb..2cbdaf2d 100644 --- a/ndk/src/native_window.rs +++ b/ndk/src/native_window.rs @@ -1,7 +1,7 @@ -//! Bindings for `ANativeWindow` +//! Bindings for [`ffi::ANativeWindow`] use std::ptr::NonNull; -#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +#[derive(Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct NativeWindow { ptr: NonNull, } @@ -9,13 +9,39 @@ pub struct NativeWindow { unsafe impl Send for NativeWindow {} unsafe impl Sync for NativeWindow {} +impl Drop for NativeWindow { + fn drop(&mut self) { + unsafe { ffi::ANativeWindow_release(self.ptr.as_ptr()) } + } +} + +impl Clone for NativeWindow { + fn clone(&self) -> Self { + unsafe { + ffi::ANativeWindow_acquire(self.ptr.as_ptr()); + Self { ptr: self.ptr } + } + } +} + impl NativeWindow { + /// Assumes ownership of `ptr` + /// /// # Safety - /// `ptr` must be a valid pointer to an Android `ANativeWindow`. + /// `ptr` must be a valid pointer to an Android [`ffi::ANativeWindow`]. pub unsafe fn from_ptr(ptr: NonNull) -> Self { Self { ptr } } + /// Acquires ownership of `ptr` + /// + /// # Safety + /// `ptr` must be a valid pointer to an Android [`ffi::ANativeWindow`]. + pub unsafe fn clone_from_ptr(ptr: NonNull) -> Self { + ffi::ANativeWindow_acquire(ptr.as_ptr()); + Self::from_ptr(ptr) + } + pub fn ptr(&self) -> NonNull { self.ptr }