From b45e41bc3bd380c895b3b1c480d013cd97764579 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Sat, 15 Feb 2025 16:02:30 -0700 Subject: [PATCH] musikr: cleanup --- musikr/src/main/jni/shim/id3_shim.cpp | 23 +++++++++--------- musikr/src/main/jni/shim/id3_shim.hpp | 7 ++++-- musikr/src/main/jni/shim/picture_shim.cpp | 4 ++-- musikr/src/main/jni/shim/picture_shim.hpp | 8 +++---- musikr/src/main/jni/src/taglib/bridge.rs | 29 +++++++++++++---------- musikr/src/main/jni/src/taglib/flac.rs | 4 ++-- musikr/src/main/jni/src/taglib/id3v2.rs | 27 +++++++++++++++++---- 7 files changed, 65 insertions(+), 37 deletions(-) diff --git a/musikr/src/main/jni/shim/id3_shim.cpp b/musikr/src/main/jni/shim/id3_shim.cpp index eb75555a2..13a3ab558 100644 --- a/musikr/src/main/jni/shim/id3_shim.cpp +++ b/musikr/src/main/jni/shim/id3_shim.cpp @@ -1,6 +1,18 @@ #include "id3_shim.hpp" namespace taglib_shim { + std::unique_ptr Tag_frameList(const TagLib::ID3v2::Tag& tag) { + return std::make_unique(tag.frameList()); + } + + std::unique_ptr> FrameList_to_vector(const TagLib::ID3v2::FrameList& list) { + auto frames = std::make_unique>(); + for (const auto& frame : list) { + frames->push_back(FramePointer{frame}); + } + return frames; + } + const TagLib::ID3v2::TextIdentificationFrame* Frame_asTextIdentification(const TagLib::ID3v2::Frame* frame) { return dynamic_cast(frame); } @@ -25,15 +37,4 @@ namespace taglib_shim { return std::make_unique(frame.fieldList()); } - TagLib::ID3v2::Tag* File_ID3v2Tag(TagLib::MPEG::File* file, bool create) { - return file->ID3v2Tag(create); - } - - std::unique_ptr> Tag_frameList(const TagLib::ID3v2::Tag& tag) { - auto frames = std::make_unique>(); - for (const auto& frame : tag.frameList()) { - frames->push_back(WrappedFrame{frame}); - } - return frames; - } } \ No newline at end of file diff --git a/musikr/src/main/jni/shim/id3_shim.hpp b/musikr/src/main/jni/shim/id3_shim.hpp index 7058ccf6f..7e6122125 100644 --- a/musikr/src/main/jni/shim/id3_shim.hpp +++ b/musikr/src/main/jni/shim/id3_shim.hpp @@ -10,11 +10,15 @@ #include "taglib/mpegfile.h" namespace taglib_shim { - struct WrappedFrame { + struct FramePointer { const TagLib::ID3v2::Frame* inner; const TagLib::ID3v2::Frame* get() const { return inner; } }; + std::unique_ptr Tag_frameList(const TagLib::ID3v2::Tag& tag); + + std::unique_ptr> FrameList_to_vector(const TagLib::ID3v2::FrameList& list); + // Frame type checking and casting const TagLib::ID3v2::TextIdentificationFrame* Frame_asTextIdentification(const TagLib::ID3v2::Frame* frame); const TagLib::ID3v2::UserTextIdentificationFrame* Frame_asUserTextIdentification(const TagLib::ID3v2::Frame* frame); @@ -27,5 +31,4 @@ namespace taglib_shim { // ID3v2 tag access TagLib::ID3v2::Tag* File_ID3v2Tag(TagLib::MPEG::File* file, bool create); - std::unique_ptr> Tag_frameList(const TagLib::ID3v2::Tag& tag); } \ No newline at end of file diff --git a/musikr/src/main/jni/shim/picture_shim.cpp b/musikr/src/main/jni/shim/picture_shim.cpp index f30d8cfb3..a3a0a1192 100644 --- a/musikr/src/main/jni/shim/picture_shim.cpp +++ b/musikr/src/main/jni/shim/picture_shim.cpp @@ -10,8 +10,8 @@ namespace taglib_shim { return std::make_unique(file.pictureList()); } - std::unique_ptr> PictureList_to_vector(const PictureList& list) { - auto result = std::make_unique>(); + std::unique_ptr> PictureList_to_vector(const PictureList& list) { + auto result = std::make_unique>(); for (const auto* picture : list) { result->emplace_back(picture); } diff --git a/musikr/src/main/jni/shim/picture_shim.hpp b/musikr/src/main/jni/shim/picture_shim.hpp index f5fbef6a1..a5c48dcee 100644 --- a/musikr/src/main/jni/shim/picture_shim.hpp +++ b/musikr/src/main/jni/shim/picture_shim.hpp @@ -10,17 +10,17 @@ namespace taglib_shim { using PictureList = TagLib::List; - class WrappedPicture { + class PicturePointer { public: - WrappedPicture(const TagLib::FLAC::Picture* picture) : picture(picture) {} - const TagLib::FLAC::Picture* inner() const { return picture; } + PicturePointer(const TagLib::FLAC::Picture* picture) : picture(picture) {} + const TagLib::FLAC::Picture* get() 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> 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/src/taglib/bridge.rs b/musikr/src/main/jni/src/taglib/bridge.rs index acdea39d4..5d68722b1 100644 --- a/musikr/src/main/jni/src/taglib/bridge.rs +++ b/musikr/src/main/jni/src/taglib/bridge.rs @@ -38,20 +38,17 @@ mod bridge_impl { #[namespace = "TagLib"] #[cxx_name = "IOStream"] type CPPIOStream; - // Create a RustIOStream from a BridgeStream fn wrap_RsIOStream(stream: Pin<&mut DynIOStream>) -> UniquePtr; #[namespace = "TagLib"] #[cxx_name = "FileRef"] type CPPFileRef; + unsafe fn new_FileRef(stream: *mut CPPIOStream) -> UniquePtr; #[cxx_name = "isNull"] fn isNull(self: Pin<&CPPFileRef>) -> bool; #[cxx_name = "file"] fn file(self: Pin<&CPPFileRef>) -> *mut CPPFile; - // Create a FileRef from an iostream - unsafe fn new_FileRef(stream: *mut CPPIOStream) -> UniquePtr; - #[namespace = "TagLib"] #[cxx_name = "File"] type CPPFile; @@ -108,12 +105,12 @@ mod bridge_impl { #[cxx_name = "PictureList"] type CPPPictureList; #[namespace = "taglib_shim"] - fn PictureList_to_vector(list: Pin<&CPPPictureList>) -> UniquePtr>; + fn PictureList_to_vector(list: Pin<&CPPPictureList>) -> UniquePtr>; #[namespace = "taglib_shim"] - #[cxx_name = "WrappedPicture"] - type CPPWrappedPicture; - fn inner(self: &CPPWrappedPicture) -> *const CPPFLACPicture; + #[cxx_name = "PicturePointer"] + type CPPFLACPicturePointer; + fn get(self: &CPPFLACPicturePointer) -> *const CPPFLACPicture; #[namespace = "TagLib::MPEG"] #[cxx_name = "File"] @@ -161,7 +158,18 @@ mod bridge_impl { #[cxx_name = "Tag"] type CPPID3v2Tag; #[namespace = "taglib_shim"] - fn Tag_frameList(tag: Pin<&CPPID3v2Tag>) -> UniquePtr>; + fn Tag_frameList(tag: Pin<&CPPID3v2Tag>) -> UniquePtr; + + #[namespace = "TagLib::ID3v2"] + #[cxx_name = "FrameList"] + type CPPID3v2FrameList; + #[namespace = "taglib_shim"] + fn FrameList_to_vector(list: Pin<&CPPID3v2FrameList>) -> UniquePtr>; + + #[namespace = "taglib_shim"] + #[cxx_name = "FramePointer"] + type CPPFramePointer; + fn get(self: &CPPFramePointer) -> *const CPPID3v2Frame; #[namespace = "TagLib::ID3v2"] #[cxx_name = "Frame"] @@ -173,9 +181,6 @@ mod bridge_impl { #[namespace = "taglib_shim"] unsafe fn Frame_asAttachedPicture(frame: *const CPPID3v2Frame) -> *const CPPID3v2AttachedPictureFrame; - #[namespace = "taglib_shim"] - type WrappedFrame; - fn get(self: &WrappedFrame) -> *const CPPID3v2Frame; #[namespace = "TagLib::ID3v2"] #[cxx_name = "TextIdentificationFrame"] diff --git a/musikr/src/main/jni/src/taglib/flac.rs b/musikr/src/main/jni/src/taglib/flac.rs index 3e3a21c99..52686e7b3 100644 --- a/musikr/src/main/jni/src/taglib/flac.rs +++ b/musikr/src/main/jni/src/taglib/flac.rs @@ -56,8 +56,8 @@ impl<'file_ref> PictureList<'file_ref> { }); let mut result = Vec::new(); - for picture_ref in pictures.iter() { - let picture_ptr = picture_ref.inner(); + for picture_ptr in pictures.iter() { + let picture_ptr = picture_ptr.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. diff --git a/musikr/src/main/jni/src/taglib/id3v2.rs b/musikr/src/main/jni/src/taglib/id3v2.rs index 4bcabd6ee..f06b4dc2c 100644 --- a/musikr/src/main/jni/src/taglib/id3v2.rs +++ b/musikr/src/main/jni/src/taglib/id3v2.rs @@ -1,9 +1,11 @@ use super::bridge::{ self, CPPID3v2Frame, CPPID3v2TextIdentificationFrame, - CPPID3v2UserTextIdentificationFrame, CPPID3v2AttachedPictureFrame, CPPID3v2Tag + CPPID3v2UserTextIdentificationFrame, CPPID3v2AttachedPictureFrame, CPPID3v2Tag, CPPID3v2FrameList }; use super::tk::{ByteVector, StringList}; use std::pin::Pin; +use std::marker::PhantomData; +use cxx::UniquePtr; pub struct ID3v2Tag<'file_ref> { this: Pin<&'file_ref CPPID3v2Tag> @@ -14,8 +16,25 @@ impl<'file_ref> ID3v2Tag<'file_ref> { Self { this } } - pub fn frames(&self) -> Vec> { + pub fn frames(&self) -> FrameList<'file_ref> { let frames = bridge::Tag_frameList(self.this.as_ref()); + FrameList::new(frames) + } +} + +pub struct FrameList<'file_ref> { + _data: PhantomData<&'file_ref CPPID3v2FrameList>, + this: UniquePtr +} + +impl<'file_ref> FrameList<'file_ref> { + pub(super) fn new(this: UniquePtr) -> Self { + Self { _data: PhantomData, this } + } + + pub fn to_vec(&self) -> Vec> { + let this = unsafe { Pin::new_unchecked(self.this.as_ref().unwrap()) }; + let frames = bridge::FrameList_to_vector(this); frames.iter().map(|frame| { let frame_ptr = frame.get(); let frame_ref = unsafe { frame_ptr.as_ref().unwrap() }; @@ -71,7 +90,7 @@ impl<'file_ref> TextIdentificationFrame<'file_ref> { Self { this } } - pub fn field_list<'slf>(&'slf self) -> StringList<'file_ref> { + pub fn field_list(&self) -> StringList<'file_ref> { let field_list = bridge::TextIdentificationFrame_fieldList(self.this); StringList::owned(field_list) } @@ -86,7 +105,7 @@ impl<'file_ref> UserTextIdentificationFrame<'file_ref> { Self { this } } - pub fn values<'slf>(&'slf self) -> StringList<'file_ref> { + pub fn values(&self) -> StringList<'file_ref> { let values = bridge::UserTextIdentificationFrame_fieldList(self.this); StringList::owned(values) }