musikr: start putting unsafe stuff into ffi mod

Aiming for like 3 layers of abstraction:
Layer 1: Top-level taglib-esque API translated to jni
Layer 2: Slightly extended unsafe wrappers over bindings
Level 3: Raw taglib bindings and shims

musikr/src/main/jni/shim/file_shim.cpp

@@ -2,37 +2,40 @@
 
 namespace taglib_shim {
 
-// File conversion functions
+const TagLib::Ogg::File* File_asOgg(const TagLib::File* file) {
-TagLib::Ogg::Vorbis::File* File_asVorbis(TagLib::File* file) {
+    return dynamic_cast<const TagLib::Ogg::File*>(file);
-    return dynamic_cast<TagLib::Ogg::Vorbis::File*>(file);
 }
 
-TagLib::Ogg::Opus::File* File_asOpus(TagLib::File* file) {
+const TagLib::Ogg::Vorbis::File* File_asVorbis(const TagLib::File* file) {
-    return dynamic_cast<TagLib::Ogg::Opus::File*>(file);
+    return dynamic_cast<const TagLib::Ogg::Vorbis::File*>(file);
 }
 
-TagLib::MPEG::File* File_asMPEG(TagLib::File* file) {
+const TagLib::Ogg::Opus::File* File_asOpus(const TagLib::File* file) {
-    return dynamic_cast<TagLib::MPEG::File*>(file);
+    return dynamic_cast<const TagLib::Ogg::Opus::File*>(file);
 }
 
-TagLib::FLAC::File* File_asFLAC(TagLib::File* file) {
+const TagLib::MPEG::File* File_asMPEG(const TagLib::File* file) {
-    return dynamic_cast<TagLib::FLAC::File*>(file);
+    return dynamic_cast<const TagLib::MPEG::File*>(file);
 }
 
-TagLib::MP4::File* File_asMP4(TagLib::File* file) {
+const TagLib::FLAC::File* File_asFLAC(const TagLib::File* file) {
-    return dynamic_cast<TagLib::MP4::File*>(file);
+    return dynamic_cast<const TagLib::FLAC::File*>(file);
 }
 
-TagLib::RIFF::WAV::File* File_asWAV(TagLib::File* file) {
+const TagLib::MP4::File* File_asMP4(const TagLib::File* file) {
-    return dynamic_cast<TagLib::RIFF::WAV::File*>(file);
+    return dynamic_cast<const TagLib::MP4::File*>(file);
 }
 
-TagLib::WavPack::File* File_asWavPack(TagLib::File* file) {
+const TagLib::RIFF::WAV::File* File_asWAV(const TagLib::File* file) {
-    return dynamic_cast<TagLib::WavPack::File*>(file);
+    return dynamic_cast<const TagLib::RIFF::WAV::File*>(file);
 }
 
-TagLib::APE::File* File_asAPE(TagLib::File* file) {
+const TagLib::WavPack::File* File_asWavPack(const TagLib::File* file) {
-    return dynamic_cast<TagLib::APE::File*>(file);
+    return dynamic_cast<const TagLib::WavPack::File*>(file);
+}
+
+const TagLib::APE::File* File_asAPE(const TagLib::File* file) {
+    return dynamic_cast<const TagLib::APE::File*>(file);
 }
 
 } // namespace taglib_shim 

musikr/src/main/jni/shim/file_shim.hpp

@@ -17,13 +17,14 @@
 namespace taglib_shim {
 
 // File conversion functions
-TagLib::Ogg::Vorbis::File* File_asVorbis(TagLib::File* file);
+const TagLib::Ogg::File* File_asOgg(const TagLib::File* file);
-TagLib::Ogg::Opus::File* File_asOpus(TagLib::File* file);
+const TagLib::Ogg::Vorbis::File* File_asVorbis(const TagLib::File* file);
-TagLib::MPEG::File* File_asMPEG(TagLib::File* file);
+const TagLib::Ogg::Opus::File* File_asOpus(const TagLib::File* file);
-TagLib::FLAC::File* File_asFLAC(TagLib::File* file);
+const TagLib::MPEG::File* File_asMPEG(const TagLib::File* file);
-TagLib::MP4::File* File_asMP4(TagLib::File* file);
+const TagLib::FLAC::File* File_asFLAC(const TagLib::File* file);
-TagLib::RIFF::WAV::File* File_asWAV(TagLib::File* file);
+const TagLib::MP4::File* File_asMP4(const TagLib::File* file);
-TagLib::WavPack::File* File_asWavPack(TagLib::File* file);
+const TagLib::RIFF::WAV::File* File_asWAV(const TagLib::File* file);
-TagLib::APE::File* File_asAPE(TagLib::File* file);
+const TagLib::WavPack::File* File_asWavPack(const TagLib::File* file);
+const TagLib::APE::File* File_asAPE(const TagLib::File* file);
 
 } // namespace taglib_shim 

musikr/src/main/jni/src/taglib/ffi.rs

@@ -1,3 +1,8 @@
+use std::ffi::CStr;
+use std::pin::Pin;
+use std::string::ToString;
+use std::collections::HashMap;
+
 #[cxx::bridge]
 pub(crate) mod bindings {
     unsafe extern "C++" {
@@ -13,7 +18,7 @@ pub(crate) mod bindings {
         #[namespace = "TagLib"]
         type FileRef;
         fn isNull(self: Pin<&FileRef>) -> bool;
-        fn file(self: Pin<&FileRef>) -> *mut File;
+        fn file(self: Pin<&FileRef>) -> *mut BaseFile;
 
         #[namespace = "taglib_shim"]
         type RustIOStream;
@@ -27,8 +32,9 @@ pub(crate) mod bindings {
         fn new_FileRef_from_stream(stream: UniquePtr<RustIOStream>) -> UniquePtr<FileRef>;
 
         #[namespace = "TagLib"]
-        type File;
+        #[cxx_name = "File"]
-        fn audioProperties(self: Pin<&File>) -> *mut AudioProperties;
+        type BaseFile;
+        fn audioProperties(self: Pin<&BaseFile>) -> *mut AudioProperties;
 
         #[namespace = "TagLib"]
         type AudioProperties;
@@ -37,29 +43,33 @@ pub(crate) mod bindings {
         fn sampleRate(self: Pin<&AudioProperties>) -> i32;
         fn channels(self: Pin<&AudioProperties>) -> i32;
 
-        #[namespace = "TagLib::Ogg::Vorbis"]
+        #[namespace = "TagLib::Ogg"]
         #[cxx_name = "File"]
-        type VorbisFile;
+        type OggFile;
-        unsafe fn tag(self: Pin<&VorbisFile>) -> *mut XiphComment;
-
-        #[namespace = "TagLib::FLAC"]
-        #[cxx_name = "File"]
-        type FLACFile;
-
-        #[namespace = "TagLib::Ogg::Opus"]
-        #[cxx_name = "File"]
-        type OpusFile;
-        unsafe fn tag(self: Pin<&OpusFile>) -> *mut XiphComment;
 
         #[namespace = "TagLib::Ogg"]
         type XiphComment;
         unsafe fn fieldListMap(self: Pin<&XiphComment>) -> &SimplePropertyMap;
 
+        #[namespace = "TagLib::Ogg::Vorbis"]
+        #[cxx_name = "File"]
+        type VorbisFile;
+        #[cxx_name = "tag"]
+        unsafe fn tag(self: Pin<&VorbisFile>) -> *mut XiphComment;
+
+        #[namespace = "TagLib::Ogg::Opus"]
+        #[cxx_name = "File"]
+        type OpusFile;
+        #[cxx_name = "tag"]
+        unsafe fn opusTag(self: Pin<&OpusFile>) -> *mut XiphComment;
+
+        #[namespace = "TagLib::FLAC"]
+        #[cxx_name = "File"]
+        type FLACFile;
         #[namespace = "TagLib::MPEG"]
         #[cxx_name = "File"]
         type MPEGFile;
 
-
         #[namespace = "TagLib::MP4"]
         #[cxx_name = "File"]
         type MP4File;
@@ -78,26 +88,30 @@ pub(crate) mod bindings {
 
         // File conversion functions
         #[namespace = "taglib_shim"]
-        unsafe fn File_asVorbis(file: *mut File) -> *mut VorbisFile;
+        unsafe fn File_asOgg(file: *const BaseFile) -> *const OggFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asOpus(file: *mut File) -> *mut OpusFile;
+        unsafe fn File_asVorbis(file: *const BaseFile) -> *const VorbisFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asMPEG(file: *mut File) -> *mut MPEGFile;
+        unsafe fn File_asOpus(file: *const BaseFile) -> *const OpusFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asFLAC(file: *mut File) -> *mut FLACFile;
+        unsafe fn File_asMPEG(file: *const BaseFile) -> *const MPEGFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asMP4(file: *mut File) -> *mut MP4File;
+        unsafe fn File_asFLAC(file: *const BaseFile) -> *const FLACFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asWAV(file: *mut File) -> *mut WAVFile;
+        unsafe fn File_asMP4(file: *const BaseFile) -> *const MP4File;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asWavPack(file: *mut File) -> *mut WavPackFile;
+        unsafe fn File_asWAV(file: *const BaseFile) -> *const WAVFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asAPE(file: *mut File) -> *mut APEFile;
+        unsafe fn File_asWavPack(file: *const BaseFile) -> *const WavPackFile;
+        #[namespace = "taglib_shim"]
+        unsafe fn File_asAPE(file: *const BaseFile) -> *const APEFile;
 
         #[namespace = "TagLib"]
         type SimplePropertyMap;
         #[namespace = "taglib_shim"]
-        fn SimplePropertyMap_to_vector(field_list_map: Pin<&SimplePropertyMap>) -> UniquePtr<CxxVector<Property>>;
+        fn SimplePropertyMap_to_vector(
+            field_list_map: Pin<&SimplePropertyMap>,
+        ) -> UniquePtr<CxxVector<Property>>;
 
         #[namespace = "taglib_shim"]
         type Property;
@@ -114,7 +128,186 @@ pub(crate) mod bindings {
         type TagString;
         #[namespace = "taglib_shim"]
         unsafe fn toCString(self: Pin<&TagString>, unicode: bool) -> *const c_char;
-        #[namespace = "taglib_shim"]
+    }
-        fn isEmpty(self: Pin<&TagString>) -> bool;
+}
+
+impl bindings::FileRef {
+    pub fn file_or(&self) -> Option<&bindings::BaseFile> {
+        unsafe {
+            // SAFETY: This pin only lasts for the scope of this function.
+            // Nothing that can change the memory address of self is returned,
+            // only the address of the file pointer.
+            let pinned_self = Pin::new_unchecked(&*self);
+            if !pinned_self.isNull() {
+                pinned_self.file().as_ref()
+            } else {
+                None
+            }
+        }
+    }
+}
+
+impl bindings::BaseFile {
+    pub fn audio_properties(&self) -> Option<&bindings::AudioProperties> {
+        let props = unsafe {
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.audioProperties()
+        };
+        unsafe {
+            props.as_ref()
+        }
+    }
+    
+    pub fn as_opus(&self) -> Option<&bindings::OpusFile> {
+        let opus_file = unsafe {
+            bindings::File_asOpus(self as *const Self)
+        };
+        unsafe {
+            opus_file.as_ref()
+        }
+    }
+
+    pub fn as_vorbis(&self) -> Option<&bindings::VorbisFile> {
+        let vorbis_file = unsafe {
+            bindings::File_asVorbis(self as *const Self)
+        };
+        unsafe {
+            vorbis_file.as_ref()
+        }
+    }
+}
+
+impl bindings::AudioProperties {
+    pub fn length_ms(&self) -> i32 {
+        unsafe {
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.lengthInMilliseconds()
+        }
+    }
+
+    pub fn bitrate_kbps(&self) -> i32 {
+        unsafe {
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.bitrate()
+        }
+    }
+
+    pub fn sample_rate_hz(&self) -> i32 {
+        unsafe {
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.sampleRate()
+        }
+    }
+
+    pub fn channel_count(&self) -> i32 {
+        unsafe {
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.channels()
+        }
+    }
+}
+
+impl bindings::OpusFile {
+    pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> {
+        let tag = unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.opusTag()
+        };
+        unsafe { 
+            // SAFETY: This pointer is a valid type, and can only used and accessed
+            // via this function and thus cannot be mutated, satisfying the aliasing rules.
+            tag.as_ref()
+        }
+    }
+}
+
+impl bindings::VorbisFile {
+    pub fn xiph_comments(&self) -> Option<&bindings::XiphComment> {
+        let tag = unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.tag()
+        };
+        unsafe { 
+            // SAFETY: This pointer is a valid type, and can only used and accessed
+            // via this function and thus cannot be mutated, satisfying the aliasing rules.
+            tag.as_ref()
+        }
+    }
+}
+
+impl bindings::XiphComment {
+    pub fn field_list_map(&self) -> &bindings::SimplePropertyMap {
+        unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            pinned_self.fieldListMap()
+        }
+    }
+}
+
+impl bindings::SimplePropertyMap {
+    pub fn to_hashmap(&self) -> HashMap<String, Vec<String>> {
+        let cxx_vec = unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            bindings::SimplePropertyMap_to_vector(pinned_self)
+        };
+        cxx_vec.iter().map(|property| property.to_tuple()).collect()
+    }
+}
+
+impl bindings::Property {
+    pub fn to_tuple(&self) -> (String, Vec<String>) {
+        unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            let key = pinned_self.key().to_string();
+            let value = pinned_self.value().to_vec();
+            (key, value)
+        }
+    }
+}
+impl ToString for bindings::TagString {
+    fn to_string(&self) -> String {
+        let c_str = unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let this = Pin::new_unchecked(self);
+            this.toCString(true)
+        };
+        unsafe { 
+            // SAFETY: This is an output from C++ with a null pointer
+            // by design. It will not be mutated and is instantly copied
+            // into rust.
+            CStr::from_ptr(c_str) 
+        }
+            .to_string_lossy()
+            .to_string()
+    }
+}
+
+impl bindings::StringList {
+    pub fn to_vec(&self) -> Vec<String> {
+        let cxx_values = unsafe {
+            // SAFETY: This will not exist beyond the scope of this function,
+            // and will only be used over ffi as a c++ this pointer (which is
+            // also pinned)
+            let pinned_self = Pin::new_unchecked(self);
+            bindings::StringList_to_vector(pinned_self)
+        };
+        cxx_values.iter().map(|value| value.to_string()).collect()
     }
 }

musikr/src/main/jni/src/taglib/mod.rs

@@ -2,8 +2,6 @@ mod ffi;
 mod stream;
 
 use ffi::bindings;
-use std::ffi::CStr;
-use std::pin::{pin, Pin};
 use std::collections::HashMap;
 pub use stream::{RustStream, TagLibStream};
 
@@ -53,7 +51,7 @@ pub struct AudioProperties {
 
 // Safe wrapper for FileRef that owns extracted data
 pub struct FileRef {
-    file: File,
+    file: Option<File>,
 }
 
 impl FileRef {
@@ -75,127 +73,36 @@ impl FileRef {
         }
 
         // Extract data from C++ objects
-        let pinned_file_ref = unsafe { Pin::new_unchecked(file_ref.as_ref().unwrap()) };
+        let file = file_ref.file_or().and_then(|file| {
-        let file_ptr = pinned_file_ref.file();
+            let audio_properties = file.audio_properties().map(|props| AudioProperties {
+                length_in_milliseconds: props.length_ms(),
+                bitrate_in_kilobits_per_second: props.bitrate_kbps(),
+                sample_rate_in_hz: props.sample_rate_hz(),
+                number_of_channels: props.channel_count(),
+            });
 
-        // Extract audio properties
+            if let Some(vorbis_file) = file.as_vorbis() {
-        let audio_properties = {
+                let xiph_comments = vorbis_file
-            let pinned_file = unsafe { Pin::new_unchecked(&*file_ptr) };
+                    .xiph_comments()
-            let props_ptr = pinned_file.audioProperties();
+                    .map(|comments| comments.field_list_map().to_hashmap());
-            if !props_ptr.is_null() {
+
-                let props = unsafe { Pin::new_unchecked(&*props_ptr) };
+                Some(File::OGG {
-                Some(AudioProperties {
+                    audio_properties,
-                    length_in_milliseconds: props.lengthInMilliseconds(),
+                    xiph_comments,
-                    bitrate_in_kilobits_per_second: props.bitrate(),
+                })
-                    sample_rate_in_hz: props.sampleRate(),
+            } else if let Some(opus_file) = file.as_opus() {
-                    number_of_channels: props.channels(),
+                let xiph_comments = opus_file
+                    .xiph_comments()
+                    .map(|comments| comments.field_list_map().to_hashmap());
+
+                Some(File::Opus {
+                    audio_properties,
+                    xiph_comments,
                 })
             } else {
-                None
+                Some(File::Unknown { audio_properties })
             }
-        };
+        });
-
-        // Determine file type and create appropriate variant
-        let file = unsafe {
-            let mpeg_file = ffi::bindings::File_asMPEG(file_ptr);
-            if !mpeg_file.is_null() {
-                return Some(FileRef {
-                    file: File::MP3 { audio_properties }
-                });
-            }
-
-            let flac_file = ffi::bindings::File_asFLAC(file_ptr);
-            if !flac_file.is_null() {
-                return Some(FileRef {
-                    file: File::FLAC { audio_properties, xiph_comments: None }
-                });
-            }
-
-            let mp4_file = ffi::bindings::File_asMP4(file_ptr);
-            if !mp4_file.is_null() {
-                return Some(FileRef {
-                    file: File::MP4 { audio_properties }
-                });
-            }
-
-            let wav_file = ffi::bindings::File_asWAV(file_ptr);
-            if !wav_file.is_null() {
-                return Some(FileRef {
-                    file: File::WAV { audio_properties }
-                });
-            }
-
-            let wavpack_file = ffi::bindings::File_asWavPack(file_ptr);
-            if !wavpack_file.is_null() {
-                return Some(FileRef {
-                    file: File::WavPack { audio_properties }
-                });
-            }
-
-            let ape_file = ffi::bindings::File_asAPE(file_ptr);
-            if !ape_file.is_null() {
-                return Some(FileRef {
-                    file: File::APE { audio_properties }
-                });
-            }
-
-            let vorbis_file = ffi::bindings::File_asVorbis(file_ptr);
-            if !vorbis_file.is_null() {
-                let pinned_vorbis_file =  Pin::new_unchecked(&*vorbis_file);
-                let xiph_comments = pinned_vorbis_file.tag();
-                let pinned_xiph_comments = Pin::new_unchecked(&*xiph_comments);
-                let xiph_map = pinned_xiph_comments.fieldListMap();
-                let pinned_xiph_map = Pin::new_unchecked(xiph_map);
-                let xiph_comments = ffi::bindings::SimplePropertyMap_to_vector(pinned_xiph_map);
-                
-                let mut xiph_safe_map = XiphComments::new();
-                for property in xiph_comments.iter() {
-                    let pinned_property = Pin::new_unchecked(property);
-                    let tag_key = pinned_property.key();
-                    let pinned_key = Pin::new_unchecked(&*tag_key);
-                    let c_str = pinned_key.toCString(true);
-                    let key = CStr::from_ptr(c_str).to_string_lossy().to_string();
-
-                    let tag_values = pinned_property.value();
-                    let pinned_values = Pin::new_unchecked(&*tag_values);
-                    let cxx_vec_values = ffi::bindings::StringList_to_vector(pinned_values);
-                    let values = cxx_vec_values.iter().map(|value|  {
-                        let pinned_value = Pin::new_unchecked(value);
-                        let c_str = pinned_value.toCString(true);
-                        CStr::from_ptr(c_str).to_string_lossy().to_string()
-                    }).collect();
-
-                    xiph_safe_map.insert(key, values);
-                }
-
-                return Some(FileRef {
-                    file: File::OGG { audio_properties, xiph_comments: Some(xiph_safe_map) }
-                });
-            }
-
-            let opus_file = ffi::bindings::File_asOpus(file_ptr);
-            if !opus_file.is_null() {
-                let pinned_opus_file = Pin::new_unchecked(&*opus_file);
-                let xiph_comments = pinned_opus_file.tag();
-                let pinned_xiph_comments = Pin::new_unchecked(&*xiph_comments);
-                let xiph_map = pinned_xiph_comments.fieldListMap();
-                let pinned_xiph_map = Pin::new_unchecked(xiph_map);
-                let xiph_comments = ffi::bindings::SimplePropertyMap_to_vector(pinned_xiph_map);
-                
-                let mut xiph_safe_map = XiphComments::new();
-                for property in xiph_comments.iter() {
-                    let pinned_property = Pin::new_unchecked(property);
-                    let tag_key = pinned_property.key();
-                    let pinned_key = Pin::new_unchecked(&*tag_key);
-                }
-                
-                return Some(FileRef {
-                    file: File::Opus { audio_properties, xiph_comments: Some(xiph_safe_map) }
-                });
-            }
-
-            File::Unknown { audio_properties }
-        };
 
         // Clean up C++ objects - they will be dropped when file_ref is dropped
         drop(file_ref);
@@ -203,7 +110,7 @@ impl FileRef {
         Some(FileRef { file })
     }
 
-    pub fn file(&self) -> &File {
+    pub fn file(&self) -> &Option<File> {
         &self.file
     }
 }