musikr: redo bridge api

Try to introduce some level of standardization in use.

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

@@ -36,14 +36,4 @@ namespace taglib_shim
         return dynamic_cast<TagLib::RIFF::WAV::File *>(file);
     }
 
-    // TagLib::WavPack::File *File_asWavPack(TagLib::File *file)
-    // {
-    //     return dynamic_cast<TagLib::WavPack::File *>(file);
-    // }
-
-    // TagLib::APE::File *File_asAPE(TagLib::File *file)
-    // {
-    //     return dynamic_cast<TagLib::APE::File *>(file);
-    // }
-
 } // namespace taglib_shim

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

@@ -24,7 +24,4 @@ namespace taglib_shim
     TagLib::FLAC::File *File_asFLAC(TagLib::File *file);
     TagLib::MP4::File *File_asMP4(TagLib::File *file);
     TagLib::RIFF::WAV::File *File_asWAV(TagLib::File *file);
-    // TagLib::WavPack::File *File_asWavPack(TagLib::File *file);
-    // TagLib::APE::File *File_asAPE(TagLib::File *file);
-
 } // namespace taglib_shim

musikr/src/main/jni/src/lib.rs

@@ -7,7 +7,7 @@ use jni::JNIEnv;
 mod taglib;
 mod jstream;
 
-use taglib::file::FileRef;
+use taglib::file_ref::FileRef;
 use jstream::JInputStream;
 
 type SharedEnv<'local> = Rc<RefCell<JNIEnv<'local>>>;
@@ -22,7 +22,7 @@ pub extern "C" fn Java_org_oxycblt_musikr_metadata_MetadataJNI_openFile<'local>(
     let shared_env = Rc::new(RefCell::new(env));
     let mut stream = JInputStream::new(shared_env.clone(), input);
     let file_ref = FileRef::new(stream);
-    let title = file_ref.file().and_then(|file| {
+    let title = file_ref.file().and_then(|mut file| {
         let audio_properties = file.audio_properties();
 
         if let Some(vorbis_file) = file.as_vorbis() {
@@ -39,7 +39,7 @@ pub extern "C" fn Java_org_oxycblt_musikr_metadata_MetadataJNI_openFile<'local>(
                 .and_then(|comments| comments.get("TITLE").cloned())
                 .and_then(|title| title.first().cloned())
                 .map(|s| s.to_string())
-        } else if let Some(flac_file) = file.as_flac() {
+        } else if let Some(mut flac_file) = file.as_flac() {
             flac_file
                 .xiph_comments()
                 .map(|comments| comments.field_list_map().to_hashmap())

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

@@ -0,0 +1,28 @@
+use super::bridge::CppAudioProperties;
+use std::pin::Pin;
+
+pub struct AudioProperties<'a> {
+    this: Pin<&'a CppAudioProperties>
+}
+
+impl<'a> AudioProperties<'a> {
+    pub(super) fn new(this: Pin<&'a CppAudioProperties>) -> Self {
+        Self { this }
+    }
+
+    pub fn length_in_milliseconds(&self) -> i32 {
+        self.this.as_ref().lengthInMilliseconds()
+    }
+
+    pub fn bitrate(&self) -> i32 {
+        self.this.as_ref().bitrate()
+    }
+
+    pub fn sample_rate(&self) -> i32 {
+        self.this.as_ref().sampleRate()
+    }
+
+    pub fn channels(&self) -> i32 {
+        self.this.as_ref().channels()
+    }   
+}

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

@@ -32,75 +32,73 @@ mod bridge_impl {
         #[namespace = "TagLib"]
         #[cxx_name = "IOStream"]
         type CPPIOStream;
+        // Create a RustIOStream from a BridgeStream
+        unsafe fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr<CPPIOStream>;
 
         #[namespace = "TagLib"]
         #[cxx_name = "FileRef"]
-        type TFileRef;
+        type CPPFileRef;
         #[cxx_name = "isNull"]
-        fn thisIsNull(self: Pin<&TFileRef>) -> bool;
+        fn isNull(self: Pin<&CPPFileRef>) -> bool;
         #[cxx_name = "file"]
-        fn thisFile(self: Pin<&TFileRef>) -> *mut BaseFile;
+        fn file(self: Pin<&CPPFileRef>) -> *mut CPPFile;
 
-        // Create a RustIOStream from a BridgeStream
-        unsafe fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr<CPPIOStream>;
         // Create a FileRef from an iostream
-        unsafe fn new_FileRef(stream: *mut CPPIOStream) -> UniquePtr<TFileRef>;
+        unsafe fn new_FileRef(stream: *mut CPPIOStream) -> UniquePtr<CPPFileRef>;
 
         #[namespace = "TagLib"]
         #[cxx_name = "File"]
-        type BaseFile;
+        type CPPFile;
         #[cxx_name = "audioProperties"]
-        fn thisAudioProperties(self: Pin<&BaseFile>) -> *mut AudioProperties;
+        fn audioProperties(self: Pin<&CPPFile>) -> *mut CppAudioProperties;
 
         #[namespace = "TagLib"]
-        type AudioProperties;
+        #[cxx_name = "AudioProperties"]
+        type CppAudioProperties;
         #[cxx_name = "lengthInMilliseconds"]
-        fn thisLengthInMilliseconds(self: Pin<&AudioProperties>) -> i32;
+        fn lengthInMilliseconds(self: Pin<&CppAudioProperties>) -> i32;
         #[cxx_name = "bitrate"]
-        fn thisBitrate(self: Pin<&AudioProperties>) -> i32;
+        fn bitrate(self: Pin<&CppAudioProperties>) -> i32;
         #[cxx_name = "sampleRate"]
-        fn thisSampleRate(self: Pin<&AudioProperties>) -> i32;
+        fn sampleRate(self: Pin<&CppAudioProperties>) -> i32;
         #[cxx_name = "channels"]
-        fn thisChannels(self: Pin<&AudioProperties>) -> i32;
+        fn channels(self: Pin<&CppAudioProperties>) -> i32;
 
         #[namespace = "TagLib::Ogg"]
-        #[cxx_name = "File"]
+        #[cxx_name = "XiphComment"]
-        type OggFile;
+        type CPPXiphComment;
-
-        #[namespace = "TagLib::Ogg"]
-        type XiphComment;
         #[cxx_name = "fieldListMap"]
-        unsafe fn thisFieldListMap(self: Pin<&XiphComment>) -> &SimplePropertyMap;
+        fn fieldListMap(self: Pin<&CPPXiphComment>) -> &CPPSimplePropertyMap;
 
         #[namespace = "TagLib::Ogg::Vorbis"]
         #[cxx_name = "File"]
-        type VorbisFile;
+        type CPPVorbisFile;
         #[cxx_name = "tag"]
-        unsafe fn vorbisThisTag(self: Pin<&VorbisFile>) -> *mut XiphComment;
+        unsafe fn vorbisTag(self: Pin<&CPPVorbisFile>) -> *mut CPPXiphComment;
 
         #[namespace = "TagLib::Ogg::Opus"]
         #[cxx_name = "File"]
-        type OpusFile;
+        type CPPOpusFile;
         #[cxx_name = "tag"]
-        unsafe fn opusThisTag(self: Pin<&OpusFile>) -> *mut XiphComment;
+        unsafe fn opusTag(self: Pin<&CPPOpusFile>) -> *mut CPPXiphComment;
 
         #[namespace = "TagLib::FLAC"]
         #[cxx_name = "File"]
-        type FLACFile;
+        type CPPFLACFile;
         #[cxx_name = "xiphComment"]
-        unsafe fn flacThisXiphComment(self: Pin<&mut FLACFile>, create: bool) -> *mut XiphComment;
+        unsafe fn xiphComment(self: Pin<&mut CPPFLACFile>, create: bool) -> *mut CPPXiphComment;
 
         #[namespace = "TagLib::MPEG"]
         #[cxx_name = "File"]
-        type MPEGFile;
+        type CPPMPEGFile;
 
         #[namespace = "TagLib::MP4"]
         #[cxx_name = "File"]
-        type MP4File;
+        type CPPMP4File;
 
         #[namespace = "TagLib::RIFF::WAV"]
         #[cxx_name = "File"]
-        type WAVFile;
+        type CPPWAVFile;
 
         // #[namespace = "TagLib::WavPack"]
         // #[cxx_name = "File"]
@@ -111,47 +109,45 @@ mod bridge_impl {
         // type APEFile;
 
         #[namespace = "taglib_shim"]
-        unsafe fn File_asVorbis(file: *mut BaseFile) -> *mut VorbisFile;
+        unsafe fn File_asVorbis(file: *mut CPPFile) -> *mut CPPVorbisFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asOpus(file: *mut BaseFile) -> *mut OpusFile;
+        unsafe fn File_asOpus(file: *mut CPPFile) -> *mut CPPOpusFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asMPEG(file: *mut BaseFile) -> *mut MPEGFile;
+        unsafe fn File_asMPEG(file: *mut CPPFile) -> *mut CPPMPEGFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asFLAC(file: *mut BaseFile) -> *mut FLACFile;
+        unsafe fn File_asFLAC(file: *mut CPPFile) -> *mut CPPFLACFile;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asMP4(file: *mut BaseFile) -> *mut MP4File;
+        unsafe fn File_asMP4(file: *mut CPPFile) -> *mut CPPMP4File;
         #[namespace = "taglib_shim"]
-        unsafe fn File_asWAV(file: *mut BaseFile) -> *mut WAVFile;
+        unsafe fn File_asWAV(file: *mut CPPFile) -> *mut CPPWAVFile;
-        // #[namespace = "taglib_shim"]
-        // unsafe fn File_asWavPack(file: *mut BaseFile) -> *mut WavPackFile;
-        // #[namespace = "taglib_shim"]
-        // unsafe fn File_asAPE(file: *mut BaseFile) -> *mut APEFile;
 
         #[namespace = "TagLib"]
-        type SimplePropertyMap;
+        #[cxx_name = "SimplePropertyMap"]
+        type CPPSimplePropertyMap;
         #[namespace = "taglib_shim"]
         fn SimplePropertyMap_to_vector(
-            field_list_map: Pin<&SimplePropertyMap>,
+            field_list_map: Pin<&CPPSimplePropertyMap>,
-        ) -> UniquePtr<CxxVector<Property>>;
+        ) -> UniquePtr<CxxVector<CPPProperty>>;
 
         #[namespace = "taglib_shim"]
-        type Property;
+        #[cxx_name = "Property"]
+        type CPPProperty;
         #[cxx_name = "key"]
-        fn thisKey(self: Pin<&Property>) -> &TString;
+        fn key(self: Pin<&CPPProperty>) -> &CPPString;
         #[cxx_name = "value"]
-        unsafe fn thisValue(self: Pin<&Property>) -> &TStringList;
+        unsafe fn value(self: Pin<&CPPProperty>) -> &CPPStringList;
 
         #[namespace = "TagLib"]
         #[cxx_name = "String"]
-        type TString;
+        type CPPString;
         #[cxx_name = "toCString"]
-        unsafe fn thisToCString(self: Pin<&TString>, unicode: bool) -> *const c_char;
+        unsafe fn thisToCString(self: Pin<&CPPString>, unicode: bool) -> *const c_char;
 
         #[namespace = "TagLib"]
         #[cxx_name = "StringList"]
-        type TStringList;
+        type CPPStringList;
         #[namespace = "taglib_shim"]
-        fn StringList_to_vector(string_list: Pin<&TStringList>) -> UniquePtr<CxxVector<TString>>;
+        fn StringList_to_vector(string_list: Pin<&CPPStringList>) -> UniquePtr<CxxVector<CPPString>>;
     }
 }
 

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

@@ -1,89 +1,44 @@
-use cxx::UniquePtr;
-use super::bridge::{self, TFileRef};
-use super::iostream::{IOStream, BridgedIOStream};
-pub use super::bridge::{BaseFile as File, AudioProperties};
-use super::xiph::{OpusFile, VorbisFile, FLACFile};
 use std::pin::Pin;
+use super::bridge::{self, CPPFile};
+use super::audioproperties::AudioProperties;
+use super::ogg::OpusFile;
+use super::ogg::VorbisFile;
+use super::flac::FLACFile;
 
-pub struct FileRef<'a> {
+pub struct File<'a> {
-    stream: BridgedIOStream<'a>,
+    this: Pin<&'a mut CPPFile>
-    file_ref: UniquePtr<TFileRef>
 }
 
-impl <'a> FileRef<'a> {
+
-    pub fn new<T : IOStream + 'a>(stream: T) -> FileRef<'a> {
+impl<'a> File<'a> {
-        let stream  = BridgedIOStream::new(stream);
+    pub(super) fn new(this: Pin<&'a mut CPPFile>) -> Self {
-        let cpp_stream = stream.cpp_stream().as_mut_ptr();
+        Self { this }
-        let file_ref = unsafe { bridge::new_FileRef(cpp_stream) };
-        FileRef {
-            stream,
-            file_ref
-        }
     }
 
-    pub fn file(&self) -> Option<&mut File> {
+    pub fn audio_properties(&self) -> Option<AudioProperties<'a>> {
-        let file = unsafe {
+        let props_ptr =  self.this.as_ref().audioProperties();
-            // SAFETY:
+        let props_ref = unsafe {
-            // - 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 file data is a pointer that does not depend on the
-            //   address of self.
-            let this = Pin::new_unchecked(&*self.file_ref);
-            // 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 {
-                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_mut()
+            props_ptr.as_ref()
-        })
-    }
-}
-
-impl File {
-    pub fn audio_properties(&self) -> Option<&AudioProperties> {
-        let props = unsafe {
-            // SAFETY:
-            // - 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<&File> = Pin::new_unchecked(self);
-            this.thisAudioProperties()
         };
-        unsafe {
+        let props_pin = props_ref.map(|props| unsafe { Pin::new_unchecked(props) });
-            // SAFETY:
+        props_pin.map(|props| AudioProperties::new(props))
-            // - 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()
-        }
     }
 
-    pub fn as_opus(&mut self) -> Option<&mut OpusFile> {
+    pub fn as_opus(&mut self) -> Option<OpusFile<'a>> {
-        let ptr_self = self as *mut Self;
         let opus_file = unsafe {
             // 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.
-            bridge::File_asOpus(ptr_self)
+            bridge::File_asOpus(self.this.as_mut().get_unchecked_mut() as *mut CPPFile)
         };
-        unsafe {
+        let opus_ref = unsafe {
             // SAFETY:
             // - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
             // - The null-safe version is being used.
@@ -91,39 +46,41 @@ impl File {
             // - 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_mut()
-        }
+        };  
+        let opus_pin = opus_ref.map(|opus| unsafe { Pin::new_unchecked(opus) });
+        opus_pin.map(|opus| OpusFile::new(opus))
     }
 
-    pub fn as_vorbis(&mut self) -> Option<&VorbisFile> {
+    pub fn as_vorbis(&mut self) -> Option<VorbisFile<'a>> {
-        let ptr_self = self as *mut Self;
         let vorbis_file = unsafe {
             // 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.
-            bridge::File_asVorbis(ptr_self)
+            bridge::File_asVorbis(self.this.as_mut().get_unchecked_mut() as *mut CPPFile)
         };
-        unsafe {
+        let vorbis_ref = 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_mut()
-        }
+        };
+        let vorbis_pin = vorbis_ref.map(|vorbis| unsafe { Pin::new_unchecked(vorbis) });
+        vorbis_pin.map(|vorbis| VorbisFile::new(vorbis))
     }
 
-    pub fn as_flac(&mut self) -> Option<&mut FLACFile> {
+    pub fn as_flac(&mut self) -> Option<FLACFile<'a>> {
-        let ptr_self = self as *mut Self;
         let flac_file = unsafe {
             // 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.
-            bridge::File_asFLAC(ptr_self)
+            bridge::File_asFLAC(self.this.as_mut().get_unchecked_mut() as *mut CPPFile)
         };
-        unsafe {
+        let flac_ref = unsafe {
             // SAFETY:
             // - This points to a C++ FFI type ensured to be aligned by cxx's codegen.
             // - The null-safe version is being used.
@@ -131,67 +88,16 @@ impl File {
             // - 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.
             flac_file.as_mut()
-        }
+        };
+        let flac_pin = flac_ref.map(|flac| unsafe { Pin::new_unchecked(flac) });
+        flac_pin.map(|flac| FLACFile::new(flac))
     }
 }
 
-impl AudioProperties {
+impl<'a> Drop for File<'a> {
-    pub fn length_in_milliseconds(&self) -> i32 {
-        unsafe {
-            // SAFETY:
-            // - 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(&self) -> i32 {
-        unsafe {
-            // SAFETY:
-            // - 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(&self) -> i32 {
-        unsafe {
-            // SAFETY:
-            // - 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 channels(&self) -> i32 {
-        unsafe {
-            // SAFETY:
-            // - 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()
-        }
-    }
-}
-
-impl <'a> Drop for FileRef<'a> {
     fn drop(&mut self) {
-        // First drop the file, since it has a pointer to the stream.
-        // Then drop the stream
         unsafe {
-            std::ptr::drop_in_place(&mut self.file_ref);
+            std::ptr::drop_in_place(&mut self.this);
-            std::ptr::drop_in_place(&mut self.stream);
         }
     }
-}
+}

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

@@ -0,0 +1,64 @@
+use super::bridge::{self, CPPFileRef};
+use super::file::File;
+use super::iostream::{BridgedIOStream, IOStream};
+use cxx::UniquePtr;
+use std::pin::Pin;
+
+pub struct FileRef<'a> {
+    stream: BridgedIOStream<'a>,
+    this: UniquePtr<CPPFileRef>,
+}
+
+impl<'a> FileRef<'a> {
+    pub fn new<T: IOStream + 'a>(stream: T) -> FileRef<'a> {
+        let stream = BridgedIOStream::new(stream);
+        let cpp_stream = stream.cpp_stream().as_mut_ptr();
+        let file_ref = unsafe { bridge::new_FileRef(cpp_stream) };
+        FileRef {
+            stream,
+            this: file_ref,
+        }
+    }
+
+    pub fn file(&self) -> Option<File<'a>> {
+        let file_ptr = unsafe {
+            // SAFETY:
+            // - 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 file data is a pointer that does not depend on the
+            //   address of self.
+            let this = Pin::new_unchecked(&*self.this);
+            // 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.isNull() {
+                Some(this.file())
+            } else {
+                None
+            }
+        };
+        let file_ref = file_ptr.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_mut()
+        });
+        let file_pin = file_ref.map(|file| unsafe { Pin::new_unchecked(file) });
+        file_pin.map(|file| File::new(file))
+    }
+}
+
+impl<'a> Drop for FileRef<'a> {
+    fn drop(&mut self) {
+        // First drop the file, since it has a pointer to the stream.
+        // Then drop the stream
+        unsafe {
+            std::ptr::drop_in_place(&mut self.this);
+            std::ptr::drop_in_place(&mut self.stream);
+        }
+    }
+}

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

@@ -0,0 +1,33 @@
+pub use super::bridge::CPPFLACFile;
+pub use super::xiph::XiphComment;
+use std::pin::Pin;
+
+pub struct FLACFile<'a> {
+    this: Pin<&'a mut CPPFLACFile>
+}
+
+impl<'a> FLACFile<'a> {
+    pub(super) fn new(this: Pin<&'a mut CPPFLACFile>) -> Self {
+        Self { this }
+    }
+
+    pub fn xiph_comments(&mut self) -> Option<XiphComment> {
+        let this = self.this.as_mut();
+        let tag = unsafe {
+            // SAFETY:
+            // - 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 a pointer that does not depend on the address of self.
+            // SAFETY: This is a C++ FFI function ensured to call correctly.
+            this.xiphComment(false)
+        };
+        let tag_ref = 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()
+        };
+        let tag_pin = tag_ref.map(|tag| unsafe { Pin::new_unchecked(tag) });
+        tag_pin.map(|tag| XiphComment::new(tag))
+    }
+}

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

@@ -1,6 +1,10 @@
 mod bridge;
 
+pub mod file_ref;
+pub mod iostream;
 pub mod file;
-pub mod tk;
+pub mod audioproperties;
+pub mod ogg;
+pub mod flac;
 pub mod xiph;
-pub mod iostream;
+pub mod tk;

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

@@ -0,0 +1,61 @@
+pub use super::bridge::{CPPOpusFile, CPPVorbisFile};
+use super::xiph::XiphComment;
+use std::pin::Pin;
+
+pub struct VorbisFile<'a> {
+    this: Pin<&'a mut CPPVorbisFile>
+}
+
+impl<'a> VorbisFile<'a> {
+    pub(super) fn new(this: Pin<&'a mut CPPVorbisFile>) -> Self {
+        Self { this }
+    }
+
+    pub fn xiph_comments(&self) -> Option<XiphComment> {
+        let this = self.this.as_ref();
+        let tag = unsafe {
+            // SAFETY:
+            // - 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 a pointer that does not depend on the address of self.
+            this.vorbisTag()
+        };
+        let tag_ref = 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()
+        };
+        let tag_pin = tag_ref.map(|tag| unsafe { Pin::new_unchecked(tag) });
+        tag_pin.map(|tag| XiphComment::new(tag))
+    }
+}
+
+pub struct OpusFile<'a> {
+    this: Pin<&'a mut CPPOpusFile>
+}
+
+impl <'a> OpusFile<'a> {
+    pub(super) fn new(this: Pin<&'a mut CPPOpusFile>) -> Self {
+        Self { this }
+    }
+
+    pub fn xiph_comments(&self) -> Option<XiphComment<'a>> {
+        let this = self.this.as_ref();
+        let tag = unsafe {
+            // SAFETY:
+            // - 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 a pointer that does not depend on the address of self.
+            this.opusTag()
+        };
+        let tag_ref = 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()
+        };
+        let tag_pin = tag_ref.map(|tag| unsafe { Pin::new_unchecked(tag) });
+        tag_pin.map(|tag| XiphComment::new(tag))
+    }
+}

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

@@ -1,44 +1,40 @@
-use std::{ffi::CStr, string::ToString};
-use std::pin::Pin;
 use std::collections::HashMap;
+use std::pin::Pin;
+use std::{ffi::CStr, string::ToString};
+use super::bridge::{self, CPPSimplePropertyMap, CPPString, CPPStringList};
 
-use super::bridge::{self, Property};
+pub struct SimplePropertyMap<'a> {
-pub use super::bridge::{TString, TStringList, SimplePropertyMap}; 
+    this: Pin<&'a CPPSimplePropertyMap>,
+}
 
-impl SimplePropertyMap {
+impl<'a> SimplePropertyMap<'a> {
+    pub(super) fn new(this: Pin<&'a CPPSimplePropertyMap>) -> Self {
+        Self { this }
+    }
+}
+
+impl<'a> SimplePropertyMap<'a> {
     pub fn to_hashmap(&self) -> HashMap<String, Vec<String>> {
-        let cxx_vec = unsafe {
+        let cxx_vec = bridge::SimplePropertyMap_to_vector(self.this);
-            // SAFETY:
+        cxx_vec
-            // - This pin is only used in this unsafe scope.
+            .iter()
-            // - The pin is used as a C++ this pointer in the ffi call, which does
+            .map(|property| unsafe {
-            //   not change address by C++ semantics.
+                // SAFETY:
-            // - The value is a unique_ptr to a copied vector that is not dependent
+                // - This pin is only used in this unsafe scope.
-            //   on the address of self.
+                // - 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.
-            bridge::SimplePropertyMap_to_vector(this)
+                // - The values returned are copied and thus not dependent on the address
-        };
+                //   of self.
-        cxx_vec.iter().map(|property| property.to_tuple()).collect()
+                let property_pin = Pin::new_unchecked(property);
+                let key = property_pin.key().to_string();
+                let value = property_pin.value().to_vec();
+                (key, value)
+            })
+            .collect()
     }
 }
 
-impl Property {
+impl ToString for CPPString {
-    pub fn to_tuple(&self) -> (String, Vec<String>) {
-        unsafe {
-            // SAFETY:
-            // - 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 values returned are copied and thus not dependent on the address
-            //   of self.
-            let this = Pin::new_unchecked(self);
-            let key = this.thisKey().to_string();
-            let value = this.thisValue().to_vec();
-            (key, value)
-        }
-    }
-}
-
-impl ToString for TString {
     fn to_string(&self) -> String {
         let c_str = unsafe {
             // SAFETY:
@@ -47,7 +43,7 @@ impl ToString for TString {
             //   not change address by C++ semantics.
             // - The value returned are pointers and thus not dependent on the address
             //   of self.
-            let this: Pin<&TString> = Pin::new_unchecked(self);
+            let this: Pin<&CPPString> = Pin::new_unchecked(self);
             this.thisToCString(true)
         };
         unsafe {
@@ -67,7 +63,7 @@ impl ToString for TString {
     }
 }
 
-impl TStringList {
+impl CPPStringList {
     pub fn to_vec(&self) -> Vec<String> {
         let cxx_values = unsafe {
             // SAFETY:

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

@@ -1,75 +1,19 @@
-pub use super::bridge::{OpusFile, VorbisFile, FLACFile, XiphComment};
+pub use super::bridge::CPPXiphComment;
 use super::tk::SimplePropertyMap;
 use std::pin::Pin;
 
-impl OpusFile {
+pub struct XiphComment<'a> {
-    pub fn xiph_comments(&self) -> Option<&XiphComment> {
+    this: Pin<&'a CPPXiphComment>
-        let tag = unsafe {
-            // SAFETY:
-            // - 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 a pointer that does not depend on the address of self.
-            let this = Pin::new_unchecked(self);
-            this.opusThisTag()
-        };
-        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 VorbisFile {
+impl<'a> XiphComment<'a> {
-    pub fn xiph_comments(&self) -> Option<&XiphComment> {
+    pub(super) fn new(this: Pin<&'a CPPXiphComment>) -> Self {
-        let tag = unsafe {
+        Self { this }
-            // SAFETY:
-            // - 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 a pointer that does not depend on the address of self.
-            let this = Pin::new_unchecked(self);
-            this.vorbisThisTag()
-        };
-        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 FLACFile {
+    pub fn field_list_map(&self) -> SimplePropertyMap<'a> {
-    pub fn xiph_comments(&mut self) -> Option<&XiphComment> {
+        let map = self.this.fieldListMap();
-        let tag = unsafe {
+        let map_pin = unsafe { Pin::new_unchecked(map) };
-            // SAFETY:
+        SimplePropertyMap::new(map_pin)
-            // - 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 a pointer that does not depend on the address of self.
-            let this = Pin::new_unchecked(self);
-            // SAFETY: This is a C++ FFI function ensured to call correctly.
-            this.flacThisXiphComment(false)
-        };
-        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 XiphComment {
-    pub fn field_list_map(&self) -> &SimplePropertyMap {
-        unsafe {
-            // SAFETY:
-            // - 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 a reference that does not depend on the address of self.
-            let this = Pin::new_unchecked(self);
-            this.thisFieldListMap()
-        }
     }
 }