Skip to content

Commit

Permalink
ndk/image_reader: Don't special-case ImgreaderNoBufferAvailable in …
Browse files Browse the repository at this point in the history
…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`,
while documenting that this is a common error for the caller to handle
gracefully.
  • Loading branch information
MarijnS95 committed Dec 25, 2023
1 parent 2d269e1 commit 70244c4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 16 deletions.
1 change: 1 addition & 0 deletions ndk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
34 changes: 18 additions & 16 deletions ndk/src/media/image_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<Image>> {
let res = construct_never_null(|res| unsafe {
pub fn acquire_next_image(&self) -> Result<Image> {
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.
Expand All @@ -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<Option<Image>> {
let res = construct_never_null(|res| unsafe {
pub fn acquire_latest_image(&self) -> Result<Image> {
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.
Expand Down

0 comments on commit 70244c4

Please sign in to comment.