Skip to content

Commit

Permalink
media_codec: Uphold non-null guarantees in some erratic scenarios
Browse files Browse the repository at this point in the history
The Android documentation does not comment on when functions could
possibly return `null`, and while discussion in [#248] finds these
unlikely enough to be communicated back to the user there is no 100%
guarantee and the resulting pointers should as such always be checked
(either at all, or also when `cfg!(debug_assertions)` happens to be
disabled).

Note that this `NonNull::new(..).unwrap()` pattern is found in many
other places in the `ndk`.

[#248]: #248
  • Loading branch information
MarijnS95 committed Mar 28, 2022
1 parent 22a11ae commit c557ef4
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
14 changes: 10 additions & 4 deletions ndk/src/media/media_codec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{construct_never_null, get_never_null, NdkMediaError, Result};
use super::{get_unlikely_to_be_null, NdkMediaError, Result};
use crate::native_window::NativeWindow;
use std::{
convert::TryInto,
Expand Down Expand Up @@ -239,6 +239,7 @@ impl MediaCodec {

#[cfg(feature = "api-level-26")]
pub fn create_input_surface(&self) -> Result<NativeWindow> {
use super::construct_never_null;
unsafe {
let ptr = construct_never_null(|res| {
ffi::AMediaCodec_createInputSurface(self.as_ptr(), res)
Expand All @@ -249,6 +250,7 @@ impl MediaCodec {

#[cfg(feature = "api-level-26")]
pub fn create_persistent_input_surface() -> Result<NativeWindow> {
use super::construct_never_null;
unsafe {
let ptr =
construct_never_null(|res| ffi::AMediaCodec_createPersistentInputSurface(res))?;
Expand Down Expand Up @@ -315,12 +317,14 @@ impl MediaCodec {

#[cfg(feature = "api-level-28")]
pub fn input_format(&self) -> MediaFormat {
let inner = get_never_null(|| unsafe { ffi::AMediaCodec_getInputFormat(self.as_ptr()) });
let inner =
get_unlikely_to_be_null(|| unsafe { ffi::AMediaCodec_getInputFormat(self.as_ptr()) });
MediaFormat { inner }
}

pub fn output_format(&self) -> MediaFormat {
let inner = get_never_null(|| unsafe { ffi::AMediaCodec_getOutputFormat(self.as_ptr()) });
let inner =
get_unlikely_to_be_null(|| unsafe { ffi::AMediaCodec_getOutputFormat(self.as_ptr()) });
MediaFormat { inner }
}

Expand Down Expand Up @@ -434,6 +438,7 @@ impl InputBuffer<'_> {
let mut out_size = 0;
let buffer_ptr =
ffi::AMediaCodec_getInputBuffer(self.codec.as_ptr(), self.index, &mut out_size);
assert!(!buffer_ptr.is_null());
slice::from_raw_parts_mut(buffer_ptr, out_size as usize)
}
}
Expand All @@ -452,6 +457,7 @@ impl OutputBuffer<'_> {
let mut _out_size = 0;
let buffer_ptr =
ffi::AMediaCodec_getOutputBuffer(self.codec.as_ptr(), self.index, &mut _out_size);
assert!(!buffer_ptr.is_null());
slice::from_raw_parts(
buffer_ptr.add(self.info.offset as usize),
self.info.size as usize,
Expand All @@ -461,7 +467,7 @@ impl OutputBuffer<'_> {

#[cfg(feature = "api-level-28")]
pub fn format(&self) -> MediaFormat {
let inner = get_never_null(|| unsafe {
let inner = get_unlikely_to_be_null(|| unsafe {
ffi::AMediaCodec_getBufferFormat(self.codec.as_ptr(), self.index)
});
MediaFormat { inner }
Expand Down
13 changes: 7 additions & 6 deletions ndk/src/media/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ fn construct_never_null<T>(
Ok(non_null)
}

fn get_never_null<T>(get_ptr: impl FnOnce() -> *mut T) -> NonNull<T> {
/// Function is not expected to ever return `null`, but this
/// cannot be validated through the Android documentation.
///
/// As such this function always asserts on `null` values,
/// even when `cfg!(debug_assertions)` is disabled.
fn get_unlikely_to_be_null<T>(get_ptr: impl FnOnce() -> *mut T) -> NonNull<T> {
let result = get_ptr();
if cfg!(debug_assertions) {
NonNull::new(result).expect("result should never be null")
} else {
unsafe { NonNull::new_unchecked(result) }
}
NonNull::new(result).expect("result should never be null")
}

0 comments on commit c557ef4

Please sign in to comment.