From 22a11ae45c6b497fabcda19a7a49bd192ef536ec Mon Sep 17 00:00:00 2001
From: Marijn Suijten <marijns95@gmail.com>
Date: Sat, 26 Mar 2022 23:39:07 +0100
Subject: [PATCH] media/media_codec: Replace `construct_never_null` and
 infallible `unwrap()` 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.
---
 ndk/src/media/media_codec.rs | 52 +++++++++++++-----------------------
 ndk/src/media/mod.rs         |  9 +++++++
 2 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/ndk/src/media/media_codec.rs b/ndk/src/media/media_codec.rs
index a63dd54c..aa5de593 100644
--- a/ndk/src/media/media_codec.rs
+++ b/ndk/src/media/media_codec.rs
@@ -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,
@@ -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(),
         }
     }
 
@@ -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) {
@@ -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())
+            })?,
         })
     }
 
@@ -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")]
@@ -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 {
diff --git a/ndk/src/media/mod.rs b/ndk/src/media/mod.rs
index 5e7d93f6..97c1a6ac 100644
--- a/ndk/src/media/mod.rs
+++ b/ndk/src/media/mod.rs
@@ -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) }
+    }
+}