From 5ce5f261b2034e138867220115ec569d1c040c62 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 25 Dec 2023 12:10:30 +0100 Subject: [PATCH] ndk/image_reader: Don't special-case `ImgreaderNoBufferAvailable` in non-async functions Both async and non-async `acquire_next/latest_image()` functions will return `ImgreaderNoBufferAvailable` when the producer has not provided a buffer that is either ready for consumption or that can be blocked on (either inside a non-async method, or by returning the accompanying fence file descriptor). After all, if there's no buffer submitted by a producer, there is no fence to wait on. Hence the current API signatures create the false assumption that only non-async functions can "not have a buffer available at all", when the exact same is true for `_async()` functions, in order to provide an image buffer with a fence that is signalled when it is ready for reading. Instead of special-casing this error in the `_async()` functions, make all functions return a flat `Result` and remove the `Option`, allowing callers to match on the variant themselves. --- ndk/CHANGELOG.md | 1 + ndk/src/media/image_reader.rs | 34 ++++++++++++++++++---------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/ndk/CHANGELOG.md b/ndk/CHANGELOG.md index 73093f48..927af448 100644 --- a/ndk/CHANGELOG.md +++ b/ndk/CHANGELOG.md @@ -2,6 +2,7 @@ - Move `MediaFormat` from `media::media_codec` to its own `media::media_format` module. (#442) - media_format: Expose `MediaFormat::copy()` and `MediaFormat::clear()` from API level 29. (#449) +- **Breaking:** image_reader: Don't special-case `ImgreaderNoBufferAvailable` in non-async functions. (#457) # 0.8.0 (2023-10-15) diff --git a/ndk/src/media/image_reader.rs b/ndk/src/media/image_reader.rs index 5c611c0d..e35b9600 100644 --- a/ndk/src/media/image_reader.rs +++ b/ndk/src/media/image_reader.rs @@ -208,21 +208,22 @@ impl ImageReader { construct(|res| unsafe { ffi::AImageReader_getMaxImages(self.as_ptr(), res) }) } + /// Returns [`MediaError::ImgreaderNoBufferAvailable`] when no [`Image`] is being rendered to by + /// a producer. #[doc(alias = "AImageReader_acquireNextImage")] - pub fn acquire_next_image(&self) -> Result> { - let res = construct_never_null(|res| unsafe { + pub fn acquire_next_image(&self) -> Result { + let inner = construct_never_null(|res| unsafe { ffi::AImageReader_acquireNextImage(self.as_ptr(), res) - }); + })?; - match res { - Ok(inner) => Ok(Some(Image { inner })), - Err(MediaError::ImgreaderNoBufferAvailable) => Ok(None), - Err(e) => Err(e), - } + Ok(Image { inner }) } /// Acquire the next [`Image`] from the image reader's queue asynchronously. /// + /// Returns [`MediaError::ImgreaderNoBufferAvailable`] when no [`Image`] is being rendered to by + /// a producer. + /// /// # Safety /// If the returned file descriptor is not [`None`], it must be awaited before attempting to /// access the [`Image`] returned. @@ -244,21 +245,22 @@ impl ImageReader { }) } + /// Returns [`MediaError::ImgreaderNoBufferAvailable`] when no [`Image`] is being rendered to by + /// a producer. #[doc(alias = "AImageReader_acquireLatestImage")] - pub fn acquire_latest_image(&self) -> Result> { - let res = construct_never_null(|res| unsafe { + pub fn acquire_latest_image(&self) -> Result { + let inner = construct_never_null(|res| unsafe { ffi::AImageReader_acquireLatestImage(self.as_ptr(), res) - }); - - if let Err(MediaError::ImgreaderNoBufferAvailable) = res { - return Ok(None); - } + })?; - Ok(Some(Image { inner: res? })) + Ok(Image { inner }) } /// Acquire the latest [`Image`] from the image reader's queue asynchronously, dropping older images. /// + /// Returns [`MediaError::ImgreaderNoBufferAvailable`] when no [`Image`] is being rendered to by + /// a producer. + /// /// # Safety /// If the returned file descriptor is not [`None`], it must be awaited before attempting to /// access the [`Image`] returned.