From f5de03dfeec3492574a4e9fad315a62589bdeb42 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sat, 15 Feb 2025 11:34:47 -0700 Subject: [PATCH] musikr: refined flac pic support --- musikr/src/main/jni/build.rs | 10 +- musikr/src/main/jni/shim/picture_shim.cpp | 17 ++++ musikr/src/main/jni/shim/picture_shim.hpp | 18 +++- musikr/src/main/jni/shim/tk_shim.cpp | 38 ------- musikr/src/main/jni/shim/tk_shim.hpp | 10 -- musikr/src/main/jni/shim/xiph_shim.cpp | 26 +++++ musikr/src/main/jni/shim/xiph_shim.hpp | 22 ++++ musikr/src/main/jni/src/taglib/bridge.rs | 108 +++++++++----------- musikr/src/main/jni/src/taglib/flac.rs | 74 +++++++------- musikr/src/main/jni/src/taglib/iostream.rs | 2 +- musikr/src/main/jni/src/taglib/ogg.rs | 9 +- musikr/src/main/jni/src/taglib/tk.rs | 111 ++++++++++++--------- musikr/src/main/jni/src/taglib/xiph.rs | 63 ++++++++---- 13 files changed, 275 insertions(+), 233 deletions(-) create mode 100644 musikr/src/main/jni/shim/xiph_shim.cpp create mode 100644 musikr/src/main/jni/shim/xiph_shim.hpp diff --git a/musikr/src/main/jni/build.rs b/musikr/src/main/jni/build.rs index e46709a70..49edd5c79 100644 --- a/musikr/src/main/jni/build.rs +++ b/musikr/src/main/jni/build.rs @@ -119,6 +119,8 @@ fn main() { .file("shim/iostream_shim.cpp") .file("shim/file_shim.cpp") .file("shim/tk_shim.cpp") + .file("shim/picture_shim.cpp") + .file("shim/xiph_shim.cpp") .include(format!("taglib/pkg/{}/include", target)) .include(".") // Add the current directory to include path .flag_if_supported("-std=c++14"); @@ -129,12 +131,6 @@ fn main() { builder.compile("taglib_cxx_bindings"); // Rebuild if shim files change - println!("cargo:rerun-if-changed=shim/iostream_shim.hpp"); - println!("cargo:rerun-if-changed=shim/iostream_shim.cpp"); - println!("cargo:rerun-if-changed=shim/file_shim.hpp"); - println!("cargo:rerun-if-changed=shim/file_shim.cpp"); - println!("cargo:rerun-if-changed=shim/tk_shim.hpp"); - println!("cargo:rerun-if-changed=shim/tk_shim.cpp"); - println!("cargo:rerun-if-changed=src/taglib/ffi.rs"); + println!("cargo:rerun-if-changed=shim/"); println!("cargo:rerun-if-changed=taglib/"); } diff --git a/musikr/src/main/jni/shim/picture_shim.cpp b/musikr/src/main/jni/shim/picture_shim.cpp index 781877935..f30d8cfb3 100644 --- a/musikr/src/main/jni/shim/picture_shim.cpp +++ b/musikr/src/main/jni/shim/picture_shim.cpp @@ -1,6 +1,23 @@ #include "picture_shim.hpp" +#include "taglib/flacfile.h" namespace taglib_shim { + std::unique_ptr XiphComment_pictureList(TagLib::Ogg::XiphComment& comment) { + return std::make_unique(comment.pictureList()); + } + + std::unique_ptr FLACFile_pictureList(TagLib::FLAC::File& file) { + return std::make_unique(file.pictureList()); + } + + std::unique_ptr> PictureList_to_vector(const PictureList& list) { + auto result = std::make_unique>(); + for (const auto* picture : list) { + result->emplace_back(picture); + } + return result; + } + std::unique_ptr Picture_mimeType(const TagLib::FLAC::Picture& picture) { return std::make_unique(picture.mimeType()); } diff --git a/musikr/src/main/jni/shim/picture_shim.hpp b/musikr/src/main/jni/shim/picture_shim.hpp index 72a5c85bf..f5fbef6a1 100644 --- a/musikr/src/main/jni/shim/picture_shim.hpp +++ b/musikr/src/main/jni/shim/picture_shim.hpp @@ -3,10 +3,24 @@ #include "taglib/flacpicture.h" #include "taglib/tstring.h" #include "taglib/tbytevector.h" +#include "tk_shim.hpp" #include +#include namespace taglib_shim { - std::unique_ptr Picture_mimeType(const TagLib::FLAC::Picture& picture); - std::unique_ptr Picture_description(const TagLib::FLAC::Picture& picture); + using PictureList = TagLib::List; + + class WrappedPicture { + public: + WrappedPicture(const TagLib::FLAC::Picture* picture) : picture(picture) {} + const TagLib::FLAC::Picture* inner() const { return picture; } + private: + const TagLib::FLAC::Picture* picture; + }; + std::unique_ptr FLACFile_pictureList(TagLib::FLAC::File& file); + std::unique_ptr XiphComment_pictureList(TagLib::Ogg::XiphComment& comment); + + std::unique_ptr> PictureList_to_vector(const PictureList& list); + std::unique_ptr Picture_data(const TagLib::FLAC::Picture& picture); } \ No newline at end of file diff --git a/musikr/src/main/jni/shim/tk_shim.cpp b/musikr/src/main/jni/shim/tk_shim.cpp index b07992c7f..e5ec3d389 100644 --- a/musikr/src/main/jni/shim/tk_shim.cpp +++ b/musikr/src/main/jni/shim/tk_shim.cpp @@ -32,42 +32,4 @@ namespace taglib_shim } return result; } - - std::unique_ptr> FLACFile_pictureList_to_vector(TagLib::FLAC::File &file) - { - std::unique_ptr> result = std::make_unique>(); - const auto pictures = file.pictureList(); - for (const auto &picture : pictures) - { - result->push_back(PictureRef(picture)); - } - return result; - } - - std::unique_ptr> XiphComment_pictureList_to_vector(TagLib::Ogg::XiphComment &comment) - { - std::unique_ptr> result = std::make_unique>(); - const auto pictures = comment.pictureList(); - for (const auto &picture : pictures) - { - result->push_back(PictureRef(picture)); - } - return result; - } - - rust::String String_to_string(const TagLib::String &str) - { - return rust::String(str.to8Bit()); - } - - std::unique_ptr> ByteVector_to_bytes(const TagLib::ByteVector &data) - { - auto result = std::make_unique>(); - result->reserve(data.size()); - for (size_t i = 0; i < data.size(); i++) - { - result->push_back(static_cast(data[i])); - } - return result; - } } diff --git a/musikr/src/main/jni/shim/tk_shim.hpp b/musikr/src/main/jni/shim/tk_shim.hpp index 7b10351b7..5f59ccd0a 100644 --- a/musikr/src/main/jni/shim/tk_shim.hpp +++ b/musikr/src/main/jni/shim/tk_shim.hpp @@ -27,17 +27,7 @@ namespace taglib_shim TagLib::StringList value_; }; - struct PictureRef { - PictureRef(const TagLib::FLAC::Picture* picture) : picture_(picture) {} - const TagLib::FLAC::Picture* get() const { return picture_; } - private: - const TagLib::FLAC::Picture* picture_; - }; std::unique_ptr> SimplePropertyMap_to_vector(const TagLib::SimplePropertyMap &map); std::unique_ptr> StringList_to_vector(const TagLib::StringList &list); - std::unique_ptr> FLACFile_pictureList_to_vector(TagLib::FLAC::File &file); - std::unique_ptr> XiphComment_pictureList_to_vector(TagLib::Ogg::XiphComment &comment); - rust::String String_to_string(const TagLib::String &str); - std::unique_ptr> ByteVector_to_bytes(const TagLib::ByteVector &data); } \ No newline at end of file diff --git a/musikr/src/main/jni/shim/xiph_shim.cpp b/musikr/src/main/jni/shim/xiph_shim.cpp new file mode 100644 index 000000000..d15f4c8ae --- /dev/null +++ b/musikr/src/main/jni/shim/xiph_shim.cpp @@ -0,0 +1,26 @@ +#include "xiph_shim.hpp" + +namespace taglib_shim +{ + FieldListEntry::FieldListEntry(TagLib::String key, TagLib::StringList value) : key_(key), value_(value) {} + + const TagLib::String &FieldListEntry::key() const + { + return key_; + } + + const TagLib::StringList &FieldListEntry::value() const + { + return value_; + } + + std::unique_ptr> FieldListMap_to_entries(const TagLib::SimplePropertyMap &map) + { + std::unique_ptr> result = std::make_unique>(); + for (const auto &pair : map) + { + result->push_back(FieldListEntry(pair.first, pair.second)); + } + return result; + } +} diff --git a/musikr/src/main/jni/shim/xiph_shim.hpp b/musikr/src/main/jni/shim/xiph_shim.hpp new file mode 100644 index 000000000..0b8a60752 --- /dev/null +++ b/musikr/src/main/jni/shim/xiph_shim.hpp @@ -0,0 +1,22 @@ +#include "taglib/tpropertymap.h" +#include "taglib/xiphcomment.h" + +namespace taglib_shim +{ + using FieldListMap = TagLib::SimplePropertyMap; + + struct FieldListEntry + { + FieldListEntry(TagLib::String key, TagLib::StringList value); + const TagLib::String &key() const; + const TagLib::StringList &value() const; + + private: + TagLib::String key_; + TagLib::StringList value_; + }; + + + std::unique_ptr> FieldListMap_to_entries(const FieldListMap &map); + std::unique_ptr> StringList_to_vector(const TagLib::StringList &list); +} \ No newline at end of file diff --git a/musikr/src/main/jni/src/taglib/bridge.rs b/musikr/src/main/jni/src/taglib/bridge.rs index 400b2bd1b..e95ce224f 100644 --- a/musikr/src/main/jni/src/taglib/bridge.rs +++ b/musikr/src/main/jni/src/taglib/bridge.rs @@ -31,12 +31,13 @@ mod bridge_impl { include!("shim/file_shim.hpp"); include!("shim/tk_shim.hpp"); include!("shim/picture_shim.hpp"); + include!("shim/xiph_shim.hpp"); #[namespace = "TagLib"] #[cxx_name = "IOStream"] type CPPIOStream; // Create a RustIOStream from a BridgeStream - unsafe fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr; + fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr; #[namespace = "TagLib"] #[cxx_name = "FileRef"] @@ -65,53 +66,68 @@ mod bridge_impl { #[cxx_name = "sampleRate"] fn sampleRate(self: Pin<&CppAudioProperties>) -> i32; #[cxx_name = "channels"] - fn channels(self: Pin<&CppAudioProperties>) -> i32; + fn channels(self: Pin<&CppAudioProperties>) -> i32; #[namespace = "TagLib::FLAC"] #[cxx_name = "Picture"] type CPPFLACPicture; #[namespace = "taglib_shim"] - fn Picture_mimeType(picture: &CPPFLACPicture) -> UniquePtr; - #[namespace = "taglib_shim"] - fn Picture_description(picture: &CPPFLACPicture) -> UniquePtr; - #[cxx_name = "width"] - fn width(self: Pin<&CPPFLACPicture>) -> i32; - #[cxx_name = "height"] - fn height(self: Pin<&CPPFLACPicture>) -> i32; - #[cxx_name = "colorDepth"] - fn colorDepth(self: Pin<&CPPFLACPicture>) -> i32; - #[cxx_name = "numColors"] - fn numColors(self: Pin<&CPPFLACPicture>) -> i32; - #[namespace = "taglib_shim"] - fn Picture_data(picture: &CPPFLACPicture) -> UniquePtr; - - #[namespace = "TagLib"] - #[cxx_name = "ByteVector"] - type CPPByteVector; + fn Picture_data(picture: Pin<&CPPFLACPicture>) -> UniquePtr; #[namespace = "TagLib::Ogg"] #[cxx_name = "XiphComment"] type CPPXiphComment; #[cxx_name = "fieldListMap"] - fn fieldListMap(self: Pin<&CPPXiphComment>) -> &CPPSimplePropertyMap; + fn fieldListMap(self: Pin<&CPPXiphComment>) -> &CPPFieldListMap; + + #[namespace = "TagLib"] + #[cxx_name = "SimplePropertyMap"] + type CPPFieldListMap; + #[namespace = "taglib_shim"] + fn FieldListMap_to_entries( + field_list_map: Pin<&CPPFieldListMap>, + ) -> UniquePtr>; + + #[namespace = "taglib_shim"] + #[cxx_name = "FieldListEntry"] + type CPPFieldListEntry; + #[cxx_name = "key"] + fn key(self: Pin<&CPPFieldListEntry>) -> &CPPString; + #[cxx_name = "value"] + fn value(self: Pin<&CPPFieldListEntry>) -> &CPPStringList; #[namespace = "TagLib::Ogg::Vorbis"] #[cxx_name = "File"] type CPPVorbisFile; #[cxx_name = "tag"] - unsafe fn vorbisTag(self: Pin<&CPPVorbisFile>) -> *mut CPPXiphComment; + fn vorbisTag(self: Pin<&CPPVorbisFile>) -> *mut CPPXiphComment; + #[namespace = "taglib_shim"] + fn XiphComment_pictureList(comment: Pin<&mut CPPXiphComment>) -> UniquePtr; #[namespace = "TagLib::Ogg::Opus"] #[cxx_name = "File"] type CPPOpusFile; #[cxx_name = "tag"] - unsafe fn opusTag(self: Pin<&CPPOpusFile>) -> *mut CPPXiphComment; + fn opusTag(self: Pin<&CPPOpusFile>) -> *mut CPPXiphComment; #[namespace = "TagLib::FLAC"] #[cxx_name = "File"] type CPPFLACFile; #[cxx_name = "xiphComment"] - unsafe fn xiphComment(self: Pin<&mut CPPFLACFile>, create: bool) -> *mut CPPXiphComment; + fn xiphComment(self: Pin<&mut CPPFLACFile>, create: bool) -> *mut CPPXiphComment; + #[namespace = "taglib_shim"] + fn FLACFile_pictureList(file: Pin<&mut CPPFLACFile>) -> UniquePtr; + + #[namespace = "taglib_shim"] + #[cxx_name = "PictureList"] + type CPPPictureList; + #[namespace = "taglib_shim"] + fn PictureList_to_vector(list: Pin<&CPPPictureList>) -> UniquePtr>; + + #[namespace = "taglib_shim"] + #[cxx_name = "WrappedPicture"] + type CPPWrappedPicture; + fn inner(self: &CPPWrappedPicture) -> *const CPPFLACPicture; #[namespace = "TagLib::MPEG"] #[cxx_name = "File"] @@ -125,14 +141,6 @@ mod bridge_impl { #[cxx_name = "File"] type CPPWAVFile; - // #[namespace = "TagLib::WavPack"] - // #[cxx_name = "File"] - // type WavPackFile; - - // #[namespace = "TagLib::APE"] - // #[cxx_name = "File"] - // type APEFile; - #[namespace = "taglib_shim"] unsafe fn File_asVorbis(file: *mut CPPFile) -> *mut CPPVorbisFile; #[namespace = "taglib_shim"] @@ -146,47 +154,23 @@ mod bridge_impl { #[namespace = "taglib_shim"] unsafe fn File_asWAV(file: *mut CPPFile) -> *mut CPPWAVFile; - #[namespace = "TagLib"] - #[cxx_name = "SimplePropertyMap"] - type CPPSimplePropertyMap; - #[namespace = "taglib_shim"] - fn SimplePropertyMap_to_vector( - field_list_map: Pin<&CPPSimplePropertyMap>, - ) -> UniquePtr>; - - #[namespace = "taglib_shim"] - #[cxx_name = "Property"] - type CPPProperty; - #[cxx_name = "key"] - fn key(self: Pin<&CPPProperty>) -> &CPPString; - #[cxx_name = "value"] - unsafe fn value(self: Pin<&CPPProperty>) -> &CPPStringList; - #[namespace = "TagLib"] #[cxx_name = "String"] type CPPString; #[cxx_name = "toCString"] - unsafe fn thisToCString(self: Pin<&CPPString>, unicode: bool) -> *const c_char; + fn toCString(self: Pin<&CPPString>, unicode: bool) -> *const c_char; #[namespace = "TagLib"] #[cxx_name = "StringList"] type CPPStringList; #[namespace = "taglib_shim"] fn StringList_to_vector(string_list: Pin<&CPPStringList>) -> UniquePtr>; - - #[namespace = "taglib_shim"] - type PictureRef; - fn get(self: &PictureRef) -> *const CPPFLACPicture; - - #[namespace = "taglib_shim"] - fn FLACFile_pictureList_to_vector(file: Pin<&mut CPPFLACFile>) -> UniquePtr>; - #[namespace = "taglib_shim"] - fn XiphComment_pictureList_to_vector(comment: Pin<&mut CPPXiphComment>) -> UniquePtr>; - - #[namespace = "taglib_shim"] - fn String_to_string(str: &CPPString) -> String; - #[namespace = "taglib_shim"] - fn ByteVector_to_bytes(data: &CPPByteVector) -> UniquePtr>; + + #[namespace = "TagLib"] + #[cxx_name = "ByteVector"] + type CPPByteVector; + fn size(self: Pin<&CPPByteVector>) -> u32; + fn data(self: Pin<&CPPByteVector>) -> *const c_char; } } diff --git a/musikr/src/main/jni/src/taglib/flac.rs b/musikr/src/main/jni/src/taglib/flac.rs index 0759159b6..7886dce78 100644 --- a/musikr/src/main/jni/src/taglib/flac.rs +++ b/musikr/src/main/jni/src/taglib/flac.rs @@ -1,7 +1,10 @@ pub use super::bridge::CPPFLACFile; pub use super::bridge::CPPFLACPicture; pub use super::xiph::XiphComment; -use super::bridge::{FLACFile_pictureList_to_vector, String_to_string, ByteVector_to_bytes, Picture_mimeType, Picture_description, Picture_data}; +use super::bridge::{CPPPictureList, PictureList_to_vector, FLACFile_pictureList, Picture_data}; +use super::tk::ByteVector; +use std::marker::PhantomData; +use cxx::UniquePtr; use std::pin::Pin; pub struct FLACFile<'a> { @@ -15,15 +18,7 @@ impl<'a> FLACFile<'a> { pub fn xiph_comments(&mut self) -> Option { 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 = 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. @@ -33,11 +28,35 @@ impl<'a> FLACFile<'a> { tag_pin.map(|tag| XiphComment::new(tag)) } - pub fn picture_list(&mut self) -> Vec> { - let pictures = FLACFile_pictureList_to_vector(self.this.as_mut()); + pub fn picture_list(&mut self) -> PictureList { + let pictures = FLACFile_pictureList(self.this.as_mut()); + PictureList::new(pictures) + } +} + +pub struct PictureList<'a> { + // PictureList is implicitly tied to the lifetime of the parent that owns it, + // so we need to track that lifetime. + _data: PhantomData<&'a CPPFLACPicture>, + this: UniquePtr, +} + +impl<'a> PictureList<'a> { + pub(super) fn new(this: UniquePtr) -> Self { + Self { _data: PhantomData, this } + } + + pub fn to_vec(&self) -> Vec { + let pictures = PictureList_to_vector(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. + Pin::new_unchecked(self.this.as_ref().unwrap()) + }); + let mut result = Vec::new(); for picture_ref in pictures.iter() { - let picture_ptr = picture_ref.get(); + let picture_ptr = picture_ref.inner(); let picture_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. @@ -59,31 +78,8 @@ impl<'a> Picture<'a> { Self { this } } - pub fn mime_type(&self) -> String { - String_to_string(Picture_mimeType(self.this.get_ref()).as_ref().unwrap()) - } - - pub fn description(&self) -> String { - String_to_string(Picture_description(self.this.get_ref()).as_ref().unwrap()) - } - - pub fn width(&self) -> i32 { - self.this.width() - } - - pub fn height(&self) -> i32 { - self.this.height() - } - - pub fn color_depth(&self) -> i32 { - self.this.colorDepth() - } - - pub fn num_colors(&self) -> i32 { - self.this.numColors() - } - - pub fn data(&self) -> Vec { - ByteVector_to_bytes(Picture_data(self.this.get_ref()).as_ref().unwrap()).iter().map(|b| *b).collect() + pub fn data(&self) -> ByteVector<'a> { + let data = Picture_data(self.this); + ByteVector::new(data) } } diff --git a/musikr/src/main/jni/src/taglib/iostream.rs b/musikr/src/main/jni/src/taglib/iostream.rs index 1d220199d..4c7169c2b 100644 --- a/musikr/src/main/jni/src/taglib/iostream.rs +++ b/musikr/src/main/jni/src/taglib/iostream.rs @@ -18,7 +18,7 @@ pub(super) struct BridgedIOStream<'a> { impl<'a> BridgedIOStream<'a> { pub fn new(stream: T) -> Self { let mut rs_stream = Box::pin(DynIOStream(Box::new(stream))); - let cpp_stream = unsafe { bridge::wrap_RsIOStream(rs_stream.as_mut()) }; + let cpp_stream = bridge::wrap_RsIOStream(rs_stream.as_mut()); BridgedIOStream { rs_stream, cpp_stream diff --git a/musikr/src/main/jni/src/taglib/ogg.rs b/musikr/src/main/jni/src/taglib/ogg.rs index d9ea32958..df71a886b 100644 --- a/musikr/src/main/jni/src/taglib/ogg.rs +++ b/musikr/src/main/jni/src/taglib/ogg.rs @@ -13,14 +13,7 @@ impl<'a> VorbisFile<'a> { pub fn xiph_comments(&self) -> Option { 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 = 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. diff --git a/musikr/src/main/jni/src/taglib/tk.rs b/musikr/src/main/jni/src/taglib/tk.rs index ddbf38669..53585cb72 100644 --- a/musikr/src/main/jni/src/taglib/tk.rs +++ b/musikr/src/main/jni/src/taglib/tk.rs @@ -1,51 +1,22 @@ -use std::collections::HashMap; +use super::bridge::{self, CPPByteVector, CPPString, CPPStringList}; +use cxx::UniquePtr; +use std::marker::PhantomData; use std::pin::Pin; use std::{ffi::CStr, string::ToString}; -use super::bridge::{self, CPPSimplePropertyMap, CPPString, CPPStringList}; -pub struct SimplePropertyMap<'a> { - this: Pin<&'a CPPSimplePropertyMap>, +pub struct String<'a> { + this: Pin<&'a CPPString>, } -impl<'a> SimplePropertyMap<'a> { - pub(super) fn new(this: Pin<&'a CPPSimplePropertyMap>) -> Self { +impl<'a> String<'a> { + pub(super) fn new(this: Pin<&'a CPPString>) -> Self { Self { this } } } -impl<'a> SimplePropertyMap<'a> { - pub fn to_hashmap(&self) -> HashMap> { - let cxx_vec = bridge::SimplePropertyMap_to_vector(self.this); - cxx_vec - .iter() - .map(|property| 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 property_pin = Pin::new_unchecked(property); - let key = property_pin.key().to_string(); - let value = property_pin.value().to_vec(); - (key, value) - }) - .collect() - } -} - -impl ToString for CPPString { - fn to_string(&self) -> String { - let c_str = 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 returned are pointers and thus not dependent on the address - // of self. - let this: Pin<&CPPString> = Pin::new_unchecked(self); - this.thisToCString(true) - }; +impl<'a> ToString for String<'a> { + fn to_string(&self) -> std::string::String { + let c_str = self.this.toCString(true); unsafe { // SAFETY: // - This is a C-string returned by a C++ method guaranteed to have @@ -63,18 +34,64 @@ impl ToString for CPPString { } } -impl CPPStringList { - pub fn to_vec(&self) -> Vec { - let cxx_values = unsafe { +pub struct StringList<'a> { + this: Pin<&'a CPPStringList>, +} + +impl<'a> StringList<'a> { + pub(super) fn new(this: Pin<&'a CPPStringList>) -> Self { + Self { this } + } + + pub fn to_vec(&self) -> Vec { + let cxx_values = bridge::StringList_to_vector(self.this); + cxx_values + .iter() + .map(|value| { + let this = unsafe { + Pin::new_unchecked(value) + }; + String::new(this).to_string() + }) + .collect() + } +} + +pub struct ByteVector<'a> { + // ByteVector is implicitly tied to the lifetime of the parent that owns it, + // so we need to track that lifetime. Only reason why it's a UniquePtr is because + // we can't marshal over ownership of the ByteVector by itself over cxx. + _data: PhantomData<&'a CPPByteVector>, + this: UniquePtr, +} + +impl<'a> ByteVector<'a> { + pub(super) fn new(this: UniquePtr) -> Self { + Self { + _data: PhantomData, + this, + } + } + + pub fn to_vec(&self) -> Vec { + let this_ref = self.this.as_ref().unwrap(); + let this = 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 returned is a unique ptr to a copied vector that is not - // dependent on the address of self. - let this = Pin::new_unchecked(self); - bridge::StringList_to_vector(this) + Pin::new_unchecked(this_ref) }; - cxx_values.iter().map(|value| value.to_string()).collect() + let size = this.size().try_into().unwrap(); + let data = this.data(); + // Re-cast to u8 + let data: *const u8 = data as *const u8; + unsafe { + // SAFETY: + // - data points to a valid buffer of size 'size' owned by the C++ ByteVector + // - we're creating a new Vec and copying the data, not taking ownership + // - the source data won't be modified while we're reading from it + std::slice::from_raw_parts(data, size).to_vec() + } } } diff --git a/musikr/src/main/jni/src/taglib/xiph.rs b/musikr/src/main/jni/src/taglib/xiph.rs index 2f23e3788..a02ace3f0 100644 --- a/musikr/src/main/jni/src/taglib/xiph.rs +++ b/musikr/src/main/jni/src/taglib/xiph.rs @@ -1,8 +1,9 @@ pub use super::bridge::CPPXiphComment; -pub use super::flac::Picture; -use super::bridge::XiphComment_pictureList_to_vector; -use super::tk::SimplePropertyMap; +pub use super::flac::PictureList; +use super::bridge::{CPPFieldListMap, FieldListMap_to_entries, XiphComment_pictureList}; +use super::tk; use std::pin::Pin; +use std::collections::HashMap; pub struct XiphComment<'a> { this: Pin<&'a mut CPPXiphComment> @@ -13,25 +14,49 @@ impl<'a> XiphComment<'a> { Self { this } } - pub fn field_list_map(&'a self) -> SimplePropertyMap<'a> { + pub fn field_list_map(&'a self) -> FieldListMap<'a> { let map = self.this.as_ref().fieldListMap(); let map_pin = unsafe { Pin::new_unchecked(map) }; - SimplePropertyMap::new(map_pin) + FieldListMap::new(map_pin) } - pub fn picture_list(&mut self) -> Vec> { - let pictures = XiphComment_pictureList_to_vector(self.this.as_mut()); - let mut result = Vec::new(); - for picture_ref in pictures.iter() { - let picture_ptr = picture_ref.get(); - let picture_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. - picture_ptr.as_ref().unwrap() - }; - let picture_pin = unsafe { Pin::new_unchecked(picture_ref) }; - result.push(Picture::new(picture_pin)); - } - result + pub fn picture_list(&mut self) -> PictureList<'a> { + let pictures = XiphComment_pictureList(self.this.as_mut()); + PictureList::new(pictures) + } +} + +pub struct FieldListMap<'a> { + this: Pin<&'a CPPFieldListMap>, +} + +impl<'a> FieldListMap<'a> { + pub(super) fn new(this: Pin<&'a CPPFieldListMap>) -> Self { + Self { this } + } +} + +impl<'a> FieldListMap<'a> { + pub fn to_hashmap(&self) -> HashMap> { + let cxx_vec = FieldListMap_to_entries(self.this); + cxx_vec + .iter() + .map(|property| { + // 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 property_pin = unsafe { Pin::new_unchecked(property) }; + let key_ref = property_pin.key(); + let key_pin = unsafe { Pin::new_unchecked(key_ref) }; + let key = tk::String::new(key_pin).to_string(); + let value_ref = property_pin.value(); + let value_pin = unsafe { Pin::new_unchecked(value_ref) }; + let value = tk::StringList::new(value_pin).to_vec(); + (key, value) + }) + .collect() } }