diff --git a/musikr/src/main/jni/src/jbuilder.rs b/musikr/src/main/jni/src/jbuilder.rs index d25e87ef1..1cd16db5b 100644 --- a/musikr/src/main/jni/src/jbuilder.rs +++ b/musikr/src/main/jni/src/jbuilder.rs @@ -36,15 +36,9 @@ impl<'local, 'file_ref> JMetadataBuilder<'local, 'file_ref> { } pub fn set_id3v1(&mut self, tag: &id3v1::ID3v1Tag) { - if let Some(title) = tag.title() { - self.id3v2.add_id("TIT2", title.to_string()); - } - if let Some(artist) = tag.artist() { - self.id3v2.add_id("TPE1", artist.to_string()); - } - if let Some(album) = tag.album() { - self.id3v2.add_id("TALB", album.to_string()); - } + self.id3v2.add_id("TIT2", tag.title().to_string()); + self.id3v2.add_id("TPE1", tag.artist().to_string()); + self.id3v2.add_id("TALB", tag.album().to_string()); self.id3v2.add_id("TRCK", tag.track().to_string()); self.id3v2.add_id("TYER", tag.year().to_string()); diff --git a/musikr/src/main/jni/src/taglib/bridge.rs b/musikr/src/main/jni/src/taglib/bridge.rs index 4accba15f..ece6a7b7e 100644 --- a/musikr/src/main/jni/src/taglib/bridge.rs +++ b/musikr/src/main/jni/src/taglib/bridge.rs @@ -1,5 +1,11 @@ use super::iostream::DynIOStream; +pub trait TagLibAllocated {} + +pub trait TagLibRef {} + +pub trait TagLibShared {} + #[cxx::bridge] mod bridge_impl { // Expose Rust IOStream to C++ @@ -86,8 +92,6 @@ mod bridge_impl { type CPPVorbisFile; #[cxx_name = "tag"] fn vorbisTag(self: &CPPVorbisFile) -> *mut CPPXiphComment; - #[namespace = "taglib_shim"] - fn XiphComment_pictureList(comment: Pin<&mut CPPXiphComment>) -> UniquePtr; #[namespace = "TagLib::Ogg::Opus"] #[cxx_name = "File"] @@ -133,6 +137,8 @@ mod bridge_impl { // Explicit lifecycle definition to state while the Pin is temporary, the CPPFieldListMap // ref returned actually has the same lifetime as the CPPXiphComment. fn fieldListMap(self: &CPPXiphComment) -> &CPPFieldListMap; + #[namespace = "taglib_shim"] + fn XiphComment_pictureList(comment: Pin<&mut CPPXiphComment>) -> UniquePtr; #[namespace = "TagLib"] #[cxx_name = "SimplePropertyMap"] @@ -316,17 +322,17 @@ mod bridge_impl { string_list: &CPPStringList, ) -> UniquePtr>; - #[namespace = "TagLib"] - #[cxx_name = "ByteVectorList"] - type CPPByteVectorList; - #[namespace = "taglib_shim"] - fn ByteVectorList_to_vector(list: &CPPByteVectorList) -> UniquePtr>; - #[namespace = "TagLib"] #[cxx_name = "ByteVector"] type CPPByteVector; fn size(self: &CPPByteVector) -> u32; fn data(self: &CPPByteVector) -> *const c_char; + + #[namespace = "TagLib"] + #[cxx_name = "ByteVectorList"] + type CPPByteVectorList; + #[namespace = "taglib_shim"] + fn ByteVectorList_to_vector(list: &CPPByteVectorList) -> UniquePtr>; } } @@ -363,3 +369,72 @@ impl MP4ItemType { } pub use bridge_impl::*; + +impl TagLibAllocated for CPPFileRef {} + +impl TagLibRef for CPPFile {} + +impl TagLibRef for CppAudioProperties {} + +// All of the File implementations are also TagLibRef and TagLibAllocated + +impl TagLibRef for CPPVorbisFile {} + +impl TagLibRef for CPPOpusFile {} + +impl TagLibRef for CPPFLACFile {} + +impl TagLibShared for CPPPictureList {} + +impl TagLibShared for CPPFLACPicture {} + +impl TagLibRef for CPPMPEGFile {} + +impl TagLibRef for CPPMP4File {} + +impl TagLibRef for CPPMP4Tag {} + +impl TagLibRef for CPPItemMap {} + +impl TagLibRef for CPPItemMapEntry {} + +impl TagLibRef for CPPFieldListMap {} + +impl TagLibRef for CPPFieldListEntry {} + +impl TagLibRef for CPPWAVFile {} + +impl TagLibRef for CPPXiphComment {} + +impl TagLibRef for CPPID3v1Tag {} + +impl TagLibRef for CPPID3v2Tag {} + +impl TagLibShared for CPPID3v2FrameList {} + +impl TagLibRef for CPPID3v2Frame {} + +impl TagLibRef for CPPID3v2TextIdentificationFrame {} + +impl TagLibRef for CPPID3v2UserTextIdentificationFrame {} + +impl TagLibRef for CPPID3v2AttachedPictureFrame {} + +impl TagLibShared for CPPMP4Item {} + +impl TagLibShared for CPPCoverArt {} + +impl TagLibShared for CPPIntPair {} + +impl TagLibShared for CPPString {} + +impl TagLibShared for CPPStringList {} + +impl TagLibShared for CPPByteVector {} + +impl TagLibShared for CPPByteVectorList {} + +impl TagLibShared for CPPCoverArtList {} + +impl TagLibRef for T {} +impl TagLibAllocated for T {} \ No newline at end of file diff --git a/musikr/src/main/jni/src/taglib/file.rs b/musikr/src/main/jni/src/taglib/file.rs index fac9a9be2..0670c3067 100644 --- a/musikr/src/main/jni/src/taglib/file.rs +++ b/musikr/src/main/jni/src/taglib/file.rs @@ -29,7 +29,7 @@ impl<'file_ref> File<'file_ref> { // to this, ensuring that it will not be mutated as per the aliasing rules. props_ptr.as_ref() }; - let props_this = props_ref.map(|props| unsafe { RefThis::new(props) }); + let props_this = props_ref.map(|props| RefThis::new(props)); props_this.map(|this| AudioProperties::new(this)) } @@ -50,7 +50,7 @@ impl<'file_ref> File<'file_ref> { // to this, ensuring that it will not be mutated as per the aliasing rules. opus_file.as_mut() }; - let opus_this = opus_ref.map(|opus| unsafe { RefThisMut::new(opus) }); + let opus_this = opus_ref.map(|opus| RefThisMut::new(opus)); opus_this.map(|this| OpusFile::new(this)) } @@ -71,7 +71,7 @@ impl<'file_ref> File<'file_ref> { // to this, ensuring that it will not be mutated as per the aliasing rules. vorbis_file.as_mut() }; - let vorbis_this = vorbis_ref.map(|vorbis| unsafe { RefThisMut::new(vorbis) }); + let vorbis_this = vorbis_ref.map(|vorbis| RefThisMut::new(vorbis)); vorbis_this.map(|this| VorbisFile::new(this)) } @@ -92,7 +92,7 @@ impl<'file_ref> File<'file_ref> { // to this, ensuring that it will not be mutated as per the aliasing rules. flac_file.as_mut() }; - let flac_this = flac_ref.map(|flac| unsafe { RefThisMut::new(flac) }); + let flac_this = flac_ref.map(|flac| RefThisMut::new(flac)); flac_this.map(|this| FLACFile::new(this)) } @@ -113,7 +113,7 @@ impl<'file_ref> File<'file_ref> { // to this, ensuring that it will not be mutated as per the aliasing rules. mpeg_file.as_mut() }; - let mpeg_this = mpeg_ref.map(|mpeg| unsafe { RefThisMut::new(mpeg) }); + let mpeg_this = mpeg_ref.map(|mpeg| RefThisMut::new(mpeg)); mpeg_this.map(|this| MPEGFile::new(this)) } @@ -122,7 +122,7 @@ impl<'file_ref> File<'file_ref> { bridge::File_asMP4(self.this.ptr_mut()) }; let mp4_ref = unsafe { mp4_file.as_mut() }; - let mp4_this = mp4_ref.map(|mp4| unsafe { RefThisMut::new(mp4) }); + let mp4_this = mp4_ref.map(|mp4| RefThisMut::new(mp4)); mp4_this.map(|this| MP4File::new(this)) } diff --git a/musikr/src/main/jni/src/taglib/file_ref.rs b/musikr/src/main/jni/src/taglib/file_ref.rs index 9c93b6083..1b7edc225 100644 --- a/musikr/src/main/jni/src/taglib/file_ref.rs +++ b/musikr/src/main/jni/src/taglib/file_ref.rs @@ -48,7 +48,7 @@ impl<'io> FileRef<'io> { // to this, ensuring that it will not be mutated as per the aliasing rules. file.as_mut() }); - let file_this = file_ref.map(|file| unsafe { RefThisMut::new(file) }); + let file_this = file_ref.map(|file| RefThisMut::new(file)); file_this.map(|this| File::new(this)) } } diff --git a/musikr/src/main/jni/src/taglib/flac.rs b/musikr/src/main/jni/src/taglib/flac.rs index 3f417ecea..860817182 100644 --- a/musikr/src/main/jni/src/taglib/flac.rs +++ b/musikr/src/main/jni/src/taglib/flac.rs @@ -27,27 +27,27 @@ impl<'file_ref> FLACFile<'file_ref> { // via this function and thus cannot be mutated, satisfying the aliasing rules. tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| XiphComment::new(this)) } - pub fn picture_list(&mut self) -> Option> { + pub fn picture_list(&mut self) -> PictureList<'file_ref> { let pictures = FLACFile_pictureList(self.this.pin_mut()); - let this = unsafe { OwnedThis::new(pictures) }; - this.map(|this| PictureList::new(this)) + let this = OwnedThis::new(pictures).unwrap(); + PictureList::new(this) } pub fn id3v1_tag(&mut self) -> Option> { let tag = self.this.pin_mut().FLACID3v1Tag(false); let tag_ref = unsafe { tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| ID3v1Tag::new(this)) } pub fn id3v2_tag(&mut self) -> Option> { let tag = self.this.pin_mut().FLACID3v2Tag(false); let tag_ref = unsafe { tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| ID3v2Tag::new(this)) } } @@ -71,7 +71,7 @@ impl<'file_ref> PictureList<'file_ref> { // via this function and thus cannot be mutated, satisfying the aliasing rules. picture_ptr.as_ref().unwrap() }; - let picture_this = unsafe { RefThis::new(picture_ref) }; + let picture_this = RefThis::new(picture_ref); result.push(Picture::new(picture_this)); } result @@ -87,9 +87,9 @@ impl<'file_ref> Picture<'file_ref> { Self { this } } - pub fn data(&self) -> Option> { + pub fn data(&self) -> OwnedByteVector<'file_ref> { let data = Picture_data(self.this.as_ref()); - let this = unsafe { OwnedThis::new(data) }; - this.map(|this| ByteVector::new(this)) + let this = OwnedThis::new(data).unwrap(); + ByteVector::new(this) } } diff --git a/musikr/src/main/jni/src/taglib/id3v1.rs b/musikr/src/main/jni/src/taglib/id3v1.rs index 8f21cf4f2..91cf0b068 100644 --- a/musikr/src/main/jni/src/taglib/id3v1.rs +++ b/musikr/src/main/jni/src/taglib/id3v1.rs @@ -11,28 +11,28 @@ impl<'file_ref> ID3v1Tag<'file_ref> { Self { this } } - pub fn title(&self) -> Option> { + pub fn title(&self) -> OwnedString<'file_ref> { let title = bridge::ID3v1Tag_title(self.this.as_ref()); - let string_this = unsafe { OwnedThis::new(title) }; - string_this.map(|this| String::new(this)) + let this = OwnedThis::new(title).unwrap(); + String::new(this) } - pub fn artist(&self) -> Option> { + pub fn artist(&self) -> OwnedString<'file_ref> { let artist = bridge::ID3v1Tag_artist(self.this.as_ref()); - let string_this = unsafe { OwnedThis::new(artist) }; - string_this.map(|this| String::new(this)) + let this = OwnedThis::new(artist).unwrap(); + String::new(this) } - pub fn album(&self) -> Option> { + pub fn album(&self) -> OwnedString<'file_ref> { let album = bridge::ID3v1Tag_album(self.this.as_ref()); - let string_this = unsafe { OwnedThis::new(album) }; - string_this.map(|this| String::new(this)) + let this = OwnedThis::new(album).unwrap(); + String::new(this) } - pub fn comment(&self) -> Option> { + pub fn comment(&self) -> OwnedString<'file_ref> { let comment = bridge::ID3v1Tag_comment(self.this.as_ref()); - let string_this = unsafe { OwnedThis::new(comment) }; - string_this.map(|this| String::new(this)) + let this = OwnedThis::new(comment).unwrap(); + String::new(this) } pub fn genre_index(&self) -> u32 { diff --git a/musikr/src/main/jni/src/taglib/id3v2.rs b/musikr/src/main/jni/src/taglib/id3v2.rs index a398f60d2..97d554a69 100644 --- a/musikr/src/main/jni/src/taglib/id3v2.rs +++ b/musikr/src/main/jni/src/taglib/id3v2.rs @@ -55,14 +55,14 @@ impl<'file_ref> Frame<'file_ref> { pub fn id(&self) -> tk::OwnedByteVector<'file_ref> { let id = bridge::Frame_id(self.this.as_ref()); - let this = unsafe { OwnedThis::new(id).unwrap() }; + let this = OwnedThis::new(id).unwrap(); ByteVector::new(this) } pub fn as_text_identification(&mut self) -> Option> { let frame = unsafe { bridge::Frame_asTextIdentification(self.this.ptr()) }; let frame_ref = unsafe { frame.as_ref() }; - let frame_this = frame_ref.map(|frame| unsafe { RefThis::new(frame) }); + let frame_this = frame_ref.map(|frame| RefThis::new(frame)); frame_this.map(|this| TextIdentificationFrame::new(this)) } @@ -71,14 +71,14 @@ impl<'file_ref> Frame<'file_ref> { ) -> Option> { let frame = unsafe { bridge::Frame_asUserTextIdentification(self.this.ptr()) }; let frame_ref = unsafe { frame.as_ref() }; - let frame_this = frame_ref.map(|frame| unsafe { RefThis::new(frame) }); + let frame_this = frame_ref.map(|frame| RefThis::new(frame)); frame_this.map(|this| UserTextIdentificationFrame::new(this)) } pub fn as_attached_picture(&mut self) -> Option> { let frame = unsafe { bridge::Frame_asAttachedPicture(self.this.ptr()) }; let frame_ref = unsafe { frame.as_ref() }; - let frame_this = frame_ref.map(|frame| unsafe { RefThis::new(frame) }); + let frame_this = frame_ref.map(|frame| RefThis::new(frame)); frame_this.map(|this| AttachedPictureFrame::new(this)) } } diff --git a/musikr/src/main/jni/src/taglib/mp4.rs b/musikr/src/main/jni/src/taglib/mp4.rs index b7062fae2..377c110d4 100644 --- a/musikr/src/main/jni/src/taglib/mp4.rs +++ b/musikr/src/main/jni/src/taglib/mp4.rs @@ -16,11 +16,11 @@ impl<'file_ref> MP4File<'file_ref> { pub fn tag(&self) -> Option> { let this = self.this.as_ref(); - let tag = unsafe { this.MP4Tag() }; + let tag = this.MP4Tag(); let tag_ref = unsafe { tag.as_ref() }; tag_ref.map(|tag| { // SAFETY: The tag pointer is guaranteed to be valid for the lifetime of self - let tag_this = unsafe { RefThis::new(tag) }; + let tag_this = RefThis::new(tag); MP4Tag::new(tag_this) }) } @@ -37,7 +37,7 @@ impl<'file_ref> MP4Tag<'file_ref> { pub fn item_map(&'file_ref self) -> ItemMap<'file_ref> { let map: &'file_ref CPPItemMap = self.this.as_ref().itemMap(); - let map_this = unsafe { RefThis::new(map) }; + let map_this = RefThis::new(map); ItemMap::new(map_this) } } @@ -63,11 +63,11 @@ impl<'file_ref> ItemMap<'file_ref> { // - The values returned are copied and thus not dependent on the address // of self. let key_ref = property.key(); - let key_this = unsafe { OwnedThis::new(key_ref) }.unwrap(); + let key_this = OwnedThis::new(key_ref).unwrap(); let key = tk::String::new(key_this).to_string(); let value_ref = property.value(); - let value_this = unsafe { OwnedThis::new(value_ref) }.unwrap(); + let value_this = OwnedThis::new(value_ref).unwrap(); let value = MP4Item::new(value_this); (key, value) @@ -99,26 +99,26 @@ impl<'file_ref> MP4Item<'file_ref> { MP4ItemType::Int => Some(MP4Data::Int(self.this.as_ref().toInt())), MP4ItemType::IntPair => { let pair = super::bridge::Item_toIntPair(self.this.as_ref()); - let pair_this = unsafe { OwnedThis::new(pair) }; - pair_this.map(|this| MP4Data::IntPair(IntPair::new(this))) + let pair_this = OwnedThis::new(pair).unwrap(); + Some(MP4Data::IntPair(IntPair::new(pair_this))) }, MP4ItemType::Byte => Some(MP4Data::Byte(self.this.as_ref().toByte())), MP4ItemType::UInt => Some(MP4Data::UInt(self.this.as_ref().toUInt())), MP4ItemType::LongLong => Some(MP4Data::LongLong(super::bridge::Item_toLongLong(self.this.as_ref()))), MP4ItemType::StringList => { let string_list = super::bridge::Item_toStringList(self.this.as_ref()); - let string_list_this = unsafe { OwnedThis::new(string_list) }; - string_list_this.map(|this| MP4Data::StringList(tk::StringList::new(this))) + let string_list_this = OwnedThis::new(string_list).unwrap(); + Some(MP4Data::StringList(tk::StringList::new(string_list_this))) }, MP4ItemType::ByteVectorList => { let byte_vector_list = super::bridge::Item_toByteVectorList(self.this.as_ref()); - let byte_vector_list_this = unsafe { OwnedThis::new(byte_vector_list) }; - byte_vector_list_this.map(|this| MP4Data::ByteVectorList(tk::ByteVectorList::new(this))) + let byte_vector_list_this = OwnedThis::new(byte_vector_list).unwrap(); + Some(MP4Data::ByteVectorList(tk::ByteVectorList::new(byte_vector_list_this))) }, MP4ItemType::CoverArtList => { let cover_art_list = super::bridge::Item_toCoverArtList(self.this.as_ref()); - let cover_art_list_this = unsafe { OwnedThis::new(cover_art_list) }; - cover_art_list_this.map(|this| MP4Data::CoverArtList(CoverArtList::new(this))) + let cover_art_list_this = OwnedThis::new(cover_art_list).unwrap(); + Some(MP4Data::CoverArtList(CoverArtList::new(cover_art_list_this))) } }) } diff --git a/musikr/src/main/jni/src/taglib/mpeg.rs b/musikr/src/main/jni/src/taglib/mpeg.rs index ea0029455..c17cebd90 100644 --- a/musikr/src/main/jni/src/taglib/mpeg.rs +++ b/musikr/src/main/jni/src/taglib/mpeg.rs @@ -16,14 +16,14 @@ impl<'file_ref> MPEGFile<'file_ref> { pub fn id3v1_tag(&mut self) -> Option> { let tag = self.this.pin_mut().MPEGID3v1Tag(false); let tag_ref = unsafe { tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| ID3v1Tag::new(this)) } pub fn id3v2_tag(&mut self) -> Option> { let tag = self.this.pin_mut().MPEGID3v2Tag(false); let tag_ref = unsafe { tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| ID3v2Tag::new(this)) } } diff --git a/musikr/src/main/jni/src/taglib/ogg.rs b/musikr/src/main/jni/src/taglib/ogg.rs index 1b4b3b8b1..4d9ee5262 100644 --- a/musikr/src/main/jni/src/taglib/ogg.rs +++ b/musikr/src/main/jni/src/taglib/ogg.rs @@ -19,7 +19,7 @@ impl<'file_ref> VorbisFile<'file_ref> { // via this function and thus cannot be mutated, satisfying the aliasing rules. tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| XiphComment::new(this)) } } @@ -40,7 +40,7 @@ impl<'file_ref> OpusFile<'file_ref> { // via this function and thus cannot be mutated, satisfying the aliasing rules. tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| XiphComment::new(this)) } } diff --git a/musikr/src/main/jni/src/taglib/riff.rs b/musikr/src/main/jni/src/taglib/riff.rs index f33e51550..d2640d683 100644 --- a/musikr/src/main/jni/src/taglib/riff.rs +++ b/musikr/src/main/jni/src/taglib/riff.rs @@ -15,7 +15,7 @@ impl<'file_ref> WAVFile<'file_ref> { pub fn id3v2_tag(&mut self) -> Option> { let tag = self.this.as_ref().WAVID3v2Tag(); let tag_ref = unsafe { tag.as_mut() }; - let tag_this = tag_ref.map(|tag| unsafe { RefThisMut::new(tag) }); + let tag_this = tag_ref.map(|tag| RefThisMut::new(tag)); tag_this.map(|this| ID3v2Tag::new(this)) } } \ No newline at end of file diff --git a/musikr/src/main/jni/src/taglib/this.rs b/musikr/src/main/jni/src/taglib/this.rs index e937bafca..d7c4ffc13 100644 --- a/musikr/src/main/jni/src/taglib/this.rs +++ b/musikr/src/main/jni/src/taglib/this.rs @@ -1,6 +1,7 @@ use std::marker::PhantomData; use std::pin::Pin; use cxx::{UniquePtr, memory::UniquePtrTarget}; +use super::bridge::{TagLibAllocated, TagLibRef, TagLibShared}; /// A taglib-FFI-specific trait representing a C++ object returned by the library. /// @@ -9,7 +10,7 @@ use cxx::{UniquePtr, memory::UniquePtrTarget}; /// and will be dropped when the FileRef is dropped. /// - This object will not move or be mutated over the FileRef's lifetime, this way /// it can be temporarily pinned for use as a `this` pointer. -pub trait This<'file_ref, T> : AsRef {} +pub trait This<'file_ref, T: TagLibAllocated> : AsRef {} /// A taglib-FFI-specific trait representing a C++ object returned by the library. /// @@ -21,16 +22,16 @@ pub trait This<'file_ref, T> : AsRef {} /// and will be dropped when the FileRef is dropped. /// - This object will not move over the FileRef's lifetime, this way it can be /// temporarily pinned for use as a `this` pointer. -pub trait ThisMut<'file_ref, T> : This<'file_ref, T> { +pub trait ThisMut<'file_ref, T: TagLibAllocated> : This<'file_ref, T> { fn pin_mut(&mut self) -> Pin<&mut T>; } /// A [This] instance that is a reference to a C++ object. -pub struct RefThis<'file_ref, T> { +pub struct RefThis<'file_ref, T: TagLibRef> { this: &'file_ref T } -impl<'file_ref, T> RefThis<'file_ref, T> { +impl<'file_ref, T: TagLibRef> RefThis<'file_ref, T> { /// Create a new [RefThis] from a reference to a C++ object. /// /// This is safe to call assuming the contract of [This] is upheld. Since this @@ -38,7 +39,7 @@ impl<'file_ref, T> RefThis<'file_ref, T> { /// responsibility to ensure that the reference is valid for the lifetime of /// the `'file_ref` parameter. More or less, if it comes from the TagLib FFI /// interface, it is safe to use this. - pub unsafe fn new(this: &'file_ref T) -> Self { + pub fn new(this: &'file_ref T) -> Self { // Rough informal contact is that the reference points to a C++ object // that will live and not move for as long as 'file_ref. Self { this } @@ -54,22 +55,22 @@ impl<'file_ref, T> RefThis<'file_ref, T> { } } -impl<'file_ref, T> AsRef for RefThis<'file_ref, T> { +impl<'file_ref, T: TagLibRef> AsRef for RefThis<'file_ref, T> { fn as_ref(&self) -> &T { self.this } } -impl<'file_ref, T> This<'file_ref, T> for RefThis<'file_ref, T> {} +impl<'file_ref, T: TagLibRef> This<'file_ref, T> for RefThis<'file_ref, T> {} /// A [ThisMut] instance that is a reference to a C++ object. /// /// This is similar to [RefThis], but allows mutating the object. -pub struct RefThisMut<'file_ref, T> { +pub struct RefThisMut<'file_ref, T: TagLibRef> { this: &'file_ref mut T, } -impl<'file_ref, T> RefThisMut<'file_ref, T> { +impl<'file_ref, T: TagLibRef> RefThisMut<'file_ref, T> { /// Create a new [RefThisMut] from a reference to a C++ object. /// /// This is safe to call assuming the contract of [ThisMut] is upheld. Since @@ -77,7 +78,7 @@ impl<'file_ref, T> RefThisMut<'file_ref, T> { /// responsibility to ensure that the reference is valid for the lifetime of /// the `'file_ref` parameter. More or less, if it comes from the TagLib FFI /// interface, it is safe to use this. - pub unsafe fn new(this: &'file_ref mut T) -> Self { + pub fn new(this: &'file_ref mut T) -> Self { Self { this } } @@ -100,15 +101,15 @@ impl<'file_ref, T> RefThisMut<'file_ref, T> { } } -impl<'file_ref, T> AsRef for RefThisMut<'file_ref, T> { +impl<'file_ref, T: TagLibRef> AsRef for RefThisMut<'file_ref, T> { fn as_ref(&self) -> &T { self.this } } -impl<'file_ref, T> This<'file_ref, T> for RefThisMut<'file_ref, T> {} +impl<'file_ref, T: TagLibRef> This<'file_ref, T> for RefThisMut<'file_ref, T> {} -impl<'file_ref, T> ThisMut<'file_ref, T> for RefThisMut<'file_ref, T> { +impl<'file_ref, T: TagLibRef> ThisMut<'file_ref, T> for RefThisMut<'file_ref, T> { fn pin_mut(&mut self) -> Pin<&mut T> { unsafe { Pin::new_unchecked(self.this) } } @@ -119,12 +120,12 @@ impl<'file_ref, T> ThisMut<'file_ref, T> for RefThisMut<'file_ref, T> { /// "Owned" in this context only really means that the object is not a rust reference. /// In practice, all "owned" taglib objects are actually shared references, and are /// thus tied to the lifetime of the `'file_ref` parameter. -pub struct OwnedThis<'file_ref, T : UniquePtrTarget> { +pub struct OwnedThis<'file_ref, T: TagLibShared + UniquePtrTarget> { _data: PhantomData<&'file_ref ()>, this: UniquePtr, } -impl<'file_ref, T : UniquePtrTarget> OwnedThis<'file_ref, T> { +impl<'file_ref, T: TagLibShared + UniquePtrTarget> OwnedThis<'file_ref, T> { /// Create a new [OwnedThis] from a [UniquePtr]. /// /// This is safe to call assuming the contract of [This] is upheld. Since this @@ -134,7 +135,7 @@ impl<'file_ref, T : UniquePtrTarget> OwnedThis<'file_ref, T> { /// interface, it is safe to use this. /// /// This will return `None` if the `UniquePtr` is `null`. - pub unsafe fn new(this: UniquePtr) -> Option { + pub fn new(this: UniquePtr) -> Option { if !this.is_null() { Some(Self { _data: PhantomData, @@ -146,15 +147,15 @@ impl<'file_ref, T : UniquePtrTarget> OwnedThis<'file_ref, T> { } } -impl<'file_ref, T : UniquePtrTarget> AsRef for OwnedThis<'file_ref, T> { +impl<'file_ref, T: TagLibShared + UniquePtrTarget> AsRef for OwnedThis<'file_ref, T> { fn as_ref(&self) -> &T { self.this.as_ref().unwrap() } } -impl<'file_ref, T : UniquePtrTarget> This<'file_ref, T> for OwnedThis<'file_ref, T> {} +impl<'file_ref, T: TagLibShared + UniquePtrTarget> This<'file_ref, T> for OwnedThis<'file_ref, T> {} -impl<'file_ref, T : UniquePtrTarget> ThisMut<'file_ref, T> for OwnedThis<'file_ref, T> { +impl<'file_ref, T: TagLibShared + UniquePtrTarget> ThisMut<'file_ref, T> for OwnedThis<'file_ref, T> { fn pin_mut(&mut self) -> Pin<&mut T> { self.this.as_mut().unwrap() } diff --git a/musikr/src/main/jni/src/taglib/tk.rs b/musikr/src/main/jni/src/taglib/tk.rs index e16284d4b..73ed14683 100644 --- a/musikr/src/main/jni/src/taglib/tk.rs +++ b/musikr/src/main/jni/src/taglib/tk.rs @@ -63,7 +63,7 @@ impl<'file_ref, T: This<'file_ref, InnerStringList>> StringList<'file_ref, T> { cxx_values .iter() .map(|value| { - let this = unsafe { RefThis::new(value) }; + let this = RefThis::new(value); String::new(this).to_string() }) .collect() diff --git a/musikr/src/main/jni/src/taglib/xiph.rs b/musikr/src/main/jni/src/taglib/xiph.rs index 034091b2c..a9b6b0d06 100644 --- a/musikr/src/main/jni/src/taglib/xiph.rs +++ b/musikr/src/main/jni/src/taglib/xiph.rs @@ -17,14 +17,14 @@ impl<'file_ref> XiphComment<'file_ref> { pub fn field_list_map(&'file_ref self) -> FieldListMap<'file_ref> { let map: &'file_ref CPPFieldListMap = self.this.as_ref().fieldListMap(); - let map_this = unsafe { RefThis::new(map) }; + let map_this = RefThis::new(map); FieldListMap::new(map_this) } - pub fn picture_list(&mut self) -> Option> { + pub fn picture_list(&mut self) -> PictureList<'file_ref> { let pictures = XiphComment_pictureList(self.this.pin_mut()); - let pictures_this = unsafe { OwnedThis::new(pictures) }; - pictures_this.map(|this| PictureList::new(this)) + let pictures_this = OwnedThis::new(pictures).unwrap(); + PictureList::new(pictures_this) } } @@ -51,10 +51,10 @@ impl<'file_ref> FieldListMap<'file_ref> { // - The values returned are copied and thus not dependent on the address // of self. let key_ref = property.key(); - let key_this = unsafe { OwnedThis::new(key_ref) }.unwrap(); + let key_this = OwnedThis::new(key_ref).unwrap(); let key = tk::String::new(key_this).to_string(); let value_ref = property.value(); - let value_this = unsafe { OwnedThis::new(value_ref) }.unwrap(); + let value_this = OwnedThis::new(value_ref).unwrap(); let value = tk::StringList::new(value_this); (key, value) })