Skip to content

Commit

Permalink
media/media_codec: Replace construct_never_null and infallible `unw…
Browse files Browse the repository at this point in the history
…rap()` pattern with something more sensible (#248)

These functions return a pointer directly instead of an error code with
the pointer as argument, making them unsuitable for use with
`construct_never_null`.  Because there is no error code, returning a
manual success value still requires an infallible `unwrap()` call to
unpack the `Result`.

Replace this with a new helper function that does not unnecessarily wrap
the resulting pointer in a `Result` when there is no result code to base
this off, and match the signature to a function that returns the pointer
directly.

At the same time reduce the size of some `unsafe {}` blocks to wrap just
what is unsafe.
  • Loading branch information
MarijnS95 authored Mar 26, 2022
1 parent 6e0189a commit 22a11ae
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 34 deletions.
52 changes: 18 additions & 34 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, NdkMediaError, Result};
use super::{construct_never_null, get_never_null, NdkMediaError, Result};
use crate::native_window::NativeWindow;
use std::{
convert::TryInto,
Expand Down Expand Up @@ -40,7 +40,7 @@ impl MediaFormat {

pub fn new() -> Self {
Self {
inner: unsafe { NonNull::new(ffi::AMediaFormat_new()).unwrap() },
inner: NonNull::new(unsafe { ffi::AMediaFormat_new() }).unwrap(),
}
}

Expand Down Expand Up @@ -88,7 +88,7 @@ impl MediaFormat {
let name = CString::new(key).unwrap();
let mut out = ptr::null();
unsafe { ffi::AMediaFormat_getString(self.as_ptr(), name.as_ptr(), &mut out) }
.then(|| unsafe { CStr::from_ptr(out).to_str().unwrap() })
.then(|| unsafe { CStr::from_ptr(out) }.to_str().unwrap())
}

pub fn set_i32(&self, key: &str, value: i32) {
Expand Down Expand Up @@ -193,25 +193,25 @@ impl MediaCodec {
pub fn from_codec_name(name: &str) -> Option<Self> {
let c_string = CString::new(name).unwrap();
Some(Self {
inner: unsafe { NonNull::new(ffi::AMediaCodec_createCodecByName(c_string.as_ptr()))? },
inner: NonNull::new(unsafe { ffi::AMediaCodec_createCodecByName(c_string.as_ptr()) })?,
})
}

pub fn from_decoder_type(mime_type: &str) -> Option<Self> {
let c_string = CString::new(mime_type).unwrap();
Some(Self {
inner: unsafe {
NonNull::new(ffi::AMediaCodec_createDecoderByType(c_string.as_ptr()))?
},
inner: NonNull::new(unsafe {
ffi::AMediaCodec_createDecoderByType(c_string.as_ptr())
})?,
})
}

pub fn from_encoder_type(mime_type: &str) -> Option<Self> {
let c_string = CString::new(mime_type).unwrap();
Some(Self {
inner: unsafe {
NonNull::new(ffi::AMediaCodec_createEncoderByType(c_string.as_ptr()))?
},
inner: NonNull::new(unsafe {
ffi::AMediaCodec_createEncoderByType(c_string.as_ptr())
})?,
})
}

Expand Down Expand Up @@ -315,25 +315,13 @@ impl MediaCodec {

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

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

#[cfg(feature = "api-level-28")]
Expand Down Expand Up @@ -473,14 +461,10 @@ impl OutputBuffer<'_> {

#[cfg(feature = "api-level-28")]
pub fn format(&self) -> MediaFormat {
unsafe {
let inner = construct_never_null(|res| {
*res = ffi::AMediaCodec_getBufferFormat(self.codec.as_ptr(), self.index);
0
})
.unwrap();
MediaFormat { inner }
}
let inner = get_never_null(|| unsafe {
ffi::AMediaCodec_getBufferFormat(self.codec.as_ptr(), self.index)
});
MediaFormat { inner }
}

pub fn flags(&self) -> u32 {
Expand Down
9 changes: 9 additions & 0 deletions ndk/src/media/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,12 @@ fn construct_never_null<T>(
};
Ok(non_null)
}

fn get_never_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) }
}
}

0 comments on commit 22a11ae

Please sign in to comment.