Fabio/Auxio
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

musikr: partially clean up ffi mod

Browse source
This commit is contained in:
Alexander Capehart 2025-02-10 10:18:41 -07:00
parent 5c4d0ab5f6
commit 20785300bb
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
2 changed files with 192 additions and 88 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

272
musikr/src/main/jni/src/taglib/ffi.rs
View file

@ -17,8 +17,10 @@ pub(crate) mod bindings {
#[namespace = "TagLib"] #[namespace = "TagLib"]
type FileRef; type FileRef;
fn isNull(self: Pin<&FileRef>) -> bool; #[cxx_name = "isNull"]
fn file(self: Pin<&FileRef>) -> *mut BaseFile; fn thisIsNull(self: Pin<&FileRef>) -> bool;
#[cxx_name = "file"]
fn thisFile(self: Pin<&FileRef>) -> *mut BaseFile;
#[namespace = "taglib_shim"] #[namespace = "taglib_shim"]
type RustIOStream; type RustIOStream;
@ -34,14 +36,19 @@ pub(crate) mod bindings {
#[namespace = "TagLib"] #[namespace = "TagLib"]
#[cxx_name = "File"] #[cxx_name = "File"]
type BaseFile; type BaseFile;
fn audioProperties(self: Pin<&BaseFile>) -> *mut AudioProperties; #[cxx_name = "audioProperties"]
fn thisAudioProperties(self: Pin<&BaseFile>) -> *mut AudioProperties;
#[namespace = "TagLib"] #[namespace = "TagLib"]
type AudioProperties; type AudioProperties;
fn lengthInMilliseconds(self: Pin<&AudioProperties>) -> i32; #[cxx_name = "lengthInMilliseconds"]
fn bitrate(self: Pin<&AudioProperties>) -> i32; fn thisLengthInMilliseconds(self: Pin<&AudioProperties>) -> i32;
fn sampleRate(self: Pin<&AudioProperties>) -> i32; #[cxx_name = "bitrate"]
fn channels(self: Pin<&AudioProperties>) -> i32; fn thisBitrate(self: Pin<&AudioProperties>) -> i32;
#[cxx_name = "sampleRate"]
fn thisSampleRate(self: Pin<&AudioProperties>) -> i32;
#[cxx_name = "channels"]
fn thisChannels(self: Pin<&AudioProperties>) -> i32;
#[namespace = "TagLib::Ogg"] #[namespace = "TagLib::Ogg"]
#[cxx_name = "File"] #[cxx_name = "File"]
@ -49,19 +56,20 @@ pub(crate) mod bindings {
#[namespace = "TagLib::Ogg"] #[namespace = "TagLib::Ogg"]
type XiphComment; type XiphComment;
unsafe fn fieldListMap(self: Pin<&XiphComment>) -> &SimplePropertyMap; #[cxx_name = "fieldListMap"]
unsafe fn thisFieldListMap(self: Pin<&XiphComment>) -> &SimplePropertyMap;
#[namespace = "TagLib::Ogg::Vorbis"] #[namespace = "TagLib::Ogg::Vorbis"]
#[cxx_name = "File"] #[cxx_name = "File"]
type VorbisFile; type VorbisFile;
#[cxx_name = "tag"] #[cxx_name = "tag"]
unsafe fn tag(self: Pin<&VorbisFile>) -> *mut XiphComment; unsafe fn vorbisThisTag(self: Pin<&VorbisFile>) -> *mut XiphComment;
#[namespace = "TagLib::Ogg::Opus"] #[namespace = "TagLib::Ogg::Opus"]
#[cxx_name = "File"] #[cxx_name = "File"]
type OpusFile; type OpusFile;
#[cxx_name = "tag"] #[cxx_name = "tag"]
unsafe fn opusTag(self: Pin<&OpusFile>) -> *mut XiphComment; unsafe fn opusThisTag(self: Pin<&OpusFile>) -> *mut XiphComment;
#[namespace = "TagLib::FLAC"] #[namespace = "TagLib::FLAC"]
#[cxx_name = "File"] #[cxx_name = "File"]
@ -115,94 +123,166 @@ pub(crate) mod bindings {
#[namespace = "taglib_shim"] #[namespace = "taglib_shim"]
type Property; type Property;
fn key(self: Pin<&Property>) -> &TagString; #[cxx_name = "key"]
unsafe fn value(self: Pin<&Property>) -> &StringList; fn thisKey(self: Pin<&Property>) -> &TString;
#[cxx_name = "value"]
#[namespace = "TagLib"] unsafe fn thisValue(self: Pin<&Property>) -> &TStringList;
type StringList;
#[namespace = "taglib_shim"]
fn StringList_to_vector(string_list: Pin<&StringList>) -> UniquePtr<CxxVector<TagString>>;
#[namespace = "TagLib"] #[namespace = "TagLib"]
#[cxx_name = "String"] #[cxx_name = "String"]
type TagString; type TString;
#[cxx_name = "toCString"]
unsafe fn thisToCString(self: Pin<&TString>, unicode: bool) -> *const c_char;
#[namespace = "TagLib"]
#[cxx_name = "StringList"]
type TStringList;
#[namespace = "taglib_shim"] #[namespace = "taglib_shim"]
unsafe fn toCString(self: Pin<&TagString>, unicode: bool) -> *const c_char; fn StringList_to_vector(string_list: Pin<&TStringList>) -> UniquePtr<CxxVector<TString>>;
} }
} }
impl bindings::FileRef { impl bindings::FileRef {
pub fn file_or(&self) -> Option<&bindings::BaseFile> { pub fn file_or(&self) -> Option<&bindings::BaseFile> {
unsafe { let file = unsafe {
// SAFETY: This pin only lasts for the scope of this function. // SAFETY:
// Nothing that can change the memory address of self is returned, // - This pin is only used in this unsafe scope.
// only the address of the file pointer. // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(&*self); // not change address by C++ semantics.
if !pinned_self.isNull() { // - The file data is a pointer that does not depend on the
pinned_self.file().as_ref() // address of self.
let this = Pin::new_unchecked(&*self);
// Note: This is not the rust ptr "is_null", but a taglib isNull method
// that checks for file validity. Without this check, we can get corrupted
// file ptrs.
if !this.thisIsNull() {
Some(this.thisFile())
} else { } else {
None None
} }
} };
file.and_then(|file| unsafe {
// SAFETY:
// - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
// - The null-safe version is being used.
// - This points to a C++FFI type ensured to be valid by cxx's codegen.
// - There are no datapaths that will yield any mutable pointers or references
// to this, ensuring that it will not be mutated as per the aliasing rules.
file.as_ref()
})
} }
} }
impl bindings::BaseFile { impl bindings::BaseFile {
pub fn audio_properties(&self) -> Option<&bindings::AudioProperties> { pub fn audio_properties(&self) -> Option<&bindings::AudioProperties> {
let props = unsafe { let props = unsafe {
let pinned_self = Pin::new_unchecked(self); // SAFETY:
pinned_self.audioProperties() // - This pin is only used in this unsafe scope.
// - The pin is used as a C++ this pointer in the ffi call, which does
// not change address by C++ semantics.
// - The audio properties data is a pointer that does not depend on the
// address of self.
let this: Pin<&bindings::BaseFile> = Pin::new_unchecked(self);
this.thisAudioProperties()
}; };
unsafe { unsafe {
// SAFETY:
// - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
// - The null-safe version is being used.
// - This points to a C++FFI type ensured to be valid by cxx's codegen.
// - There are no datapaths that will yield any mutable pointers or references
// to this, ensuring that it will not be mutated as per the aliasing rules.
props.as_ref() props.as_ref()
} }
} }
pub fn as_opus(&self) -> Option<&bindings::OpusFile> { pub fn as_opus(&self) -> Option<&bindings::OpusFile> {
let ptr_self = self as *const Self;
let opus_file = unsafe { let opus_file = unsafe {
bindings::File_asOpus(self as *const Self) // SAFETY:
// This FFI function will be a simple C++ dynamic_cast, which checks if
// the file can be cased down to an opus file. If the cast fails, a null
// pointer is returned, which will be handled by as_ref's null checking.
bindings::File_asOpus(ptr_self)
}; };
unsafe { unsafe {
// SAFETY:
// - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
// - The null-safe version is being used.
// - This points to a C++FFI type ensured to be valid by cxx's codegen.
// - There are no datapaths that will yield any mutable pointers or references
// to this, ensuring that it will not be mutated as per the aliasing rules.
opus_file.as_ref() opus_file.as_ref()
} }
} }
pub fn as_vorbis(&self) -> Option<&bindings::VorbisFile> { pub fn as_vorbis(&self) -> Option<&bindings::VorbisFile> {
let ptr_self = self as *const Self;
let vorbis_file = unsafe { let vorbis_file = unsafe {
bindings::File_asVorbis(self as *const Self) // SAFETY:
// This FFI function will be a simple C++ dynamic_cast, which checks if
// the file can be cased down to an opus file. If the cast fails, a null
// pointer is returned, which will be handled by as_ref's null checking.
bindings::File_asVorbis(ptr_self)
}; };
unsafe { unsafe {
// SAFETY:
// - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
// - The null-safe version is being used.
// - This points to a C++FFI type ensured to be valid by cxx's codegen.
// - There are no datapaths that will yield any mutable pointers or references
// to this, ensuring that it will not be mutated as per the aliasing rules.
vorbis_file.as_ref() vorbis_file.as_ref()
} }
} }
} }
impl bindings::AudioProperties { impl bindings::AudioProperties {
pub fn length_ms(&self) -> i32 { pub fn length_in_milliseconds(&self) -> i32 {
unsafe { unsafe {
let pinned_self = Pin::new_unchecked(self); // SAFETY:
pinned_self.lengthInMilliseconds() // - This pin is only used in this unsafe scope.
// - The pin is used as a C++ this pointer in the ffi call, which does
// not change address by C++ semantics.
// - The value is copied and thus not dependent on the address of self.
let this = Pin::new_unchecked(self);
this.thisLengthInMilliseconds()
} }
} }
pub fn bitrate_kbps(&self) -> i32 { pub fn bitrate(&self) -> i32 {
unsafe { unsafe {
let pinned_self = Pin::new_unchecked(self); // SAFETY:
pinned_self.bitrate() // - This pin is only used in this unsafe scope.
// - The pin is used as a C++ this pointer in the ffi call, which does
// not change address by C++ semantics.
// - The value is copied and thus not dependent on the address of self.
let this = Pin::new_unchecked(self);
this.thisBitrate()
} }
} }
pub fn sample_rate_hz(&self) -> i32 { pub fn sample_rate(&self) -> i32 {
unsafe { unsafe {
let pinned_self = Pin::new_unchecked(self); // SAFETY:
pinned_self.sampleRate() // - This pin is only used in this unsafe scope.
// - The pin is used as a C++ this pointer in the ffi call, which does
// not change address by C++ semantics.
// - The value is copied and thus not dependent on the address of self.
let this = Pin::new_unchecked(self);
this.thisSampleRate()
} }
} }
pub fn channel_count(&self) -> i32 { pub fn channels(&self) -> i32 {
unsafe { unsafe {
let pinned_self = Pin::new_unchecked(self); // SAFETY:
pinned_self.channels() // - This pin is only used in this unsafe scope.
// - The pin is used as a C++ this pointer in the ffi call, which does
// not change address by C++ semantics.
// - The value is copied and thus not dependent on the address of self.
let this = Pin::new_unchecked(self);
this.thisChannels()
} }
} }
} }
@ -210,11 +290,13 @@ impl bindings::AudioProperties {
impl bindings::OpusFile { impl bindings::OpusFile {
pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> { pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> {
let tag = unsafe { let tag = unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
pinned_self.opusTag() // - The value is a pointer that does not depend on the address of self.
let this = Pin::new_unchecked(self);
this.opusThisTag()
}; };
unsafe { unsafe {
// SAFETY: This pointer is a valid type, and can only used and accessed // SAFETY: This pointer is a valid type, and can only used and accessed
@ -227,11 +309,13 @@ impl bindings::OpusFile {
impl bindings::VorbisFile { impl bindings::VorbisFile {
pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> { pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> {
let tag = unsafe { let tag = unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
pinned_self.tag() // - The value is a pointer that does not depend on the address of self.
let this = Pin::new_unchecked(self);
this.vorbisThisTag()
}; };
unsafe { unsafe {
// SAFETY: This pointer is a valid type, and can only used and accessed // SAFETY: This pointer is a valid type, and can only used and accessed
@ -244,11 +328,13 @@ impl bindings::VorbisFile {
impl bindings::XiphComment { impl bindings::XiphComment {
pub fn field_list_map(&self) -> &bindings::SimplePropertyMap { pub fn field_list_map(&self) -> &bindings::SimplePropertyMap {
unsafe { unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
pinned_self.fieldListMap() // - The value is a reference that does not depend on the address of self.
let this = Pin::new_unchecked(self);
this.thisFieldListMap()
} }
} }
} }
@ -256,11 +342,14 @@ impl bindings::XiphComment {
impl bindings::SimplePropertyMap { impl bindings::SimplePropertyMap {
pub fn to_hashmap(&self) -> HashMap<String, Vec<String>> { pub fn to_hashmap(&self) -> HashMap<String, Vec<String>> {
let cxx_vec = unsafe { let cxx_vec = unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
bindings::SimplePropertyMap_to_vector(pinned_self) // - The value is a unique_ptr to a copied vector that is not dependent
// on the address of self.
let this = Pin::new_unchecked(self);
bindings::SimplePropertyMap_to_vector(this)
}; };
cxx_vec.iter().map(|property| property.to_tuple()).collect() cxx_vec.iter().map(|property| property.to_tuple()).collect()
} }
@ -269,29 +358,41 @@ impl bindings::SimplePropertyMap {
impl bindings::Property { impl bindings::Property {
pub fn to_tuple(&self) -> (String, Vec<String>) { pub fn to_tuple(&self) -> (String, Vec<String>) {
unsafe { unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
let key = pinned_self.key().to_string(); // - The values returned are copied and thus not dependent on the address
let value = pinned_self.value().to_vec(); // of self.
let this = Pin::new_unchecked(self);
let key = this.thisKey().to_string();
let value = this.thisValue().to_vec();
(key, value) (key, value)
} }
} }
} }
impl ToString for bindings::TagString { impl ToString for bindings::TString {
fn to_string(&self) -> String { fn to_string(&self) -> String {
let c_str = unsafe { let c_str = unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let this = Pin::new_unchecked(self); // not change address by C++ semantics.
this.toCString(true) // - The value returned are pointers and thus not dependent on the address
// of self.
let this: Pin<&bindings::TString> = Pin::new_unchecked(self);
this.thisToCString(true)
}; };
unsafe { unsafe {
// SAFETY: This is an output from C++ with a null pointer // SAFETY:
// by design. It will not be mutated and is instantly copied // - This is a C-string returned by a C++ method guaranteed to have
// into rust. // a null terminator.
// - This C-string is fully allocated and owned by the TagString instance,
// in a continous block from start to null terminator.
// - This C-string will be non-null even if empty.
// - This pointer will not be mutated before it's entirely copied into
// rust.
// - This C-string is copied to a rust string before TagString is destroyed.
CStr::from_ptr(c_str) CStr::from_ptr(c_str)
} }
.to_string_lossy() .to_string_lossy()
@ -299,14 +400,17 @@ impl ToString for bindings::TagString {
} }
} }
impl bindings::StringList { impl bindings::TStringList {
pub fn to_vec(&self) -> Vec<String> { pub fn to_vec(&self) -> Vec<String> {
let cxx_values = unsafe { let cxx_values = unsafe {
// SAFETY: This will not exist beyond the scope of this function, // SAFETY:
// and will only be used over ffi as a c++ this pointer (which is // - This pin is only used in this unsafe scope.
// also pinned) // - The pin is used as a C++ this pointer in the ffi call, which does
let pinned_self = Pin::new_unchecked(self); // not change address by C++ semantics.
bindings::StringList_to_vector(pinned_self) // - The value returned is a unique ptr to a copied vector that is not
// dependent on the address of self.
let this = Pin::new_unchecked(self);
bindings::StringList_to_vector(this)
}; };
cxx_values.iter().map(|value| value.to_string()).collect() cxx_values.iter().map(|value| value.to_string()).collect()
} }

8
musikr/src/main/jni/src/taglib/mod.rs
View file

@ -75,10 +75,10 @@ impl FileRef {
// Extract data from C++ objects // Extract data from C++ objects
let file = file_ref.file_or().and_then(|file| { let file = file_ref.file_or().and_then(|file| {
let audio_properties = file.audio_properties().map(|props| AudioProperties { let audio_properties = file.audio_properties().map(|props| AudioProperties {
length_in_milliseconds: props.length_ms(), length_in_milliseconds: props.length_in_milliseconds(),
bitrate_in_kilobits_per_second: props.bitrate_kbps(), bitrate_in_kilobits_per_second: props.bitrate(),
sample_rate_in_hz: props.sample_rate_hz(), sample_rate_in_hz: props.sample_rate(),
number_of_channels: props.channel_count(), number_of_channels: props.channels(),
}); });
if let Some(vorbis_file) = file.as_vorbis() { if let Some(vorbis_file) = file.as_vorbis() {