From 7f84349f2e4efaf9f2fc77d16ad3aa259ea024b4 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Mon, 10 Feb 2025 13:02:02 -0700 Subject: [PATCH] musikr: use no_std in crate Doesn't really help since jni uses std excessively. --- musikr/src/main/jni/Cargo.toml | 6 +- musikr/src/main/jni/shim/iostream_shim.cpp | 18 ++-- musikr/src/main/jni/src/jni_stream.rs | 77 +++++--------- musikr/src/main/jni/src/lib.rs | 10 +- musikr/src/main/jni/src/taglib/ffi.rs | 13 ++- musikr/src/main/jni/src/taglib/mod.rs | 113 ++++++++++++--------- musikr/src/main/jni/src/taglib/stream.rs | 59 +++-------- 7 files changed, 135 insertions(+), 161 deletions(-) diff --git a/musikr/src/main/jni/Cargo.toml b/musikr/src/main/jni/Cargo.toml index 926a1e73f..f2da31def 100644 --- a/musikr/src/main/jni/Cargo.toml +++ b/musikr/src/main/jni/Cargo.toml @@ -8,8 +8,8 @@ name = "metadatajni" crate-type = ["cdylib"] [dependencies] -cxx = "1.0.137" -jni = "0.21.1" +cxx = { version = "1.0.137", default-features = false, features = ["alloc"] } +jni = { version = "0.21.1", default-features = false } [build-dependencies] -cxx-build = "1.0.137" \ No newline at end of file +cxx-build = "1.0.137" diff --git a/musikr/src/main/jni/shim/iostream_shim.cpp b/musikr/src/main/jni/shim/iostream_shim.cpp index c4bcd392c..9d0051d9b 100644 --- a/musikr/src/main/jni/shim/iostream_shim.cpp +++ b/musikr/src/main/jni/shim/iostream_shim.cpp @@ -8,12 +8,12 @@ extern "C" { const char *rust_stream_name(const void *stream); size_t rust_stream_read(void *stream, uint8_t *buffer, size_t length); - void rust_stream_write(void *stream, const uint8_t *data, size_t length); + // void rust_stream_write(void *stream, const uint8_t *data, size_t length); void rust_stream_seek(void *stream, int64_t offset, int32_t whence); - void rust_stream_truncate(void *stream, int64_t length); + // void rust_stream_truncate(void *stream, int64_t length); int64_t rust_stream_tell(const void *stream); int64_t rust_stream_length(const void *stream); - bool rust_stream_is_readonly(const void *stream); + // bool rust_stream_is_readonly(const void *stream); } namespace taglib_shim @@ -47,11 +47,9 @@ namespace taglib_shim return TagLib::ByteVector(reinterpret_cast(buffer.data()), bytes_read); } - void RustIOStream::writeBlock(const TagLib::ByteVector &data) + void RustIOStream::writeBlock(const TagLib::ByteVector &_) { - rust_stream_write(rust_stream, - reinterpret_cast(data.data()), - data.size()); + throw std::runtime_error("Stream is read-only"); } void RustIOStream::insert(const TagLib::ByteVector &data, TagLib::offset_t start, size_t replace) @@ -127,9 +125,9 @@ namespace taglib_shim seek(0); } - void RustIOStream::truncate(TagLib::offset_t length) + void RustIOStream::truncate(TagLib::offset_t _) { - rust_stream_truncate(rust_stream, length); + throw std::runtime_error("Stream is read-only"); } TagLib::offset_t RustIOStream::tell() const @@ -144,7 +142,7 @@ namespace taglib_shim bool RustIOStream::readOnly() const { - return rust_stream_is_readonly(rust_stream); + return true; } bool RustIOStream::isOpen() const diff --git a/musikr/src/main/jni/src/jni_stream.rs b/musikr/src/main/jni/src/jni_stream.rs index 5077a2bcb..e82644afe 100644 --- a/musikr/src/main/jni/src/jni_stream.rs +++ b/musikr/src/main/jni/src/jni_stream.rs @@ -1,7 +1,7 @@ -use crate::taglib::TagLibStream; +use crate::taglib::{IOStream, Whence}; use jni::objects::{JObject, JValue}; use jni::JNIEnv; -use std::io::{Read, Seek, SeekFrom, Write}; +use alloc::string::String; pub struct JInputStream<'local, 'a> { env: &'a mut JNIEnv<'local>, @@ -20,7 +20,7 @@ impl<'local, 'a> JInputStream<'local, 'a> { } } -impl<'local, 'a> TagLibStream for JInputStream<'local, 'a> { +impl<'local, 'a> IOStream for JInputStream<'local, 'a> { fn name(&mut self) -> String { // Call the Java name() method safely let name = self @@ -35,18 +35,12 @@ impl<'local, 'a> TagLibStream for JInputStream<'local, 'a> { .into() } - fn is_readonly(&self) -> bool { - true // JInputStream is always read-only - } -} - -impl<'local, 'a> Read for JInputStream<'local, 'a> { - fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + fn read(&mut self, buf: &mut [u8]) -> Result { // Create a direct ByteBuffer from the Rust slice let byte_buffer = unsafe { self.env .new_direct_byte_buffer(buf.as_mut_ptr(), buf.len()) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))? + .map_err(|_| String::from("Failed to create byte buffer"))? }; // Call readBlock safely @@ -59,38 +53,20 @@ impl<'local, 'a> Read for JInputStream<'local, 'a> { &[JValue::Object(&byte_buffer)], ) .and_then(|result| result.z()) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?; + .map_err(|_| String::from("Failed to read block"))?; if !success { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Failed to read block", - )); + return Err(String::from("Failed to read block")); } Ok(buf.len()) } -} -impl<'local, 'a> Write for JInputStream<'local, 'a> { - fn write(&mut self, _buf: &[u8]) -> std::io::Result { - Err(std::io::Error::new( - std::io::ErrorKind::PermissionDenied, - "JInputStream is read-only", - )) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) // Nothing to flush in a read-only stream - } -} - -impl<'local, 'a> Seek for JInputStream<'local, 'a> { - fn seek(&mut self, pos: SeekFrom) -> std::io::Result { + fn seek(&mut self, pos: Whence) -> Result<(), String> { let (method, offset) = match pos { - SeekFrom::Start(offset) => ("seekFromBeginning", offset as i64), - SeekFrom::Current(offset) => ("seekFromCurrent", offset), - SeekFrom::End(offset) => ("seekFromEnd", offset), + Whence::Start(offset) => ("seekFromBeginning", offset as i64), + Whence::Current(offset) => ("seekFromCurrent", offset), + Whence::End(offset) => ("seekFromEnd", offset), }; // Call the appropriate seek method safely @@ -98,29 +74,28 @@ impl<'local, 'a> Seek for JInputStream<'local, 'a> { .env .call_method(&self.input, method, "(J)Z", &[JValue::Long(offset)]) .and_then(|result| result.z()) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?; + .map_err(|e| String::from("Failed to seek"))?; if !success { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Failed to seek", - )); + return Err(String::from("Failed to seek")); } - // Return current position safely - let position = self + Ok(()) + } + + fn tell(&mut self) -> Result { + self .env .call_method(&self.input, "tell", "()J", &[]) .and_then(|result| result.j()) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?; + .map_err(|_| String::from("Failed to get position")) + } - if position == i64::MIN { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Failed to get position", - )); - } - - Ok(position as u64) + fn length(&mut self) -> Result { + self + .env + .call_method(&self.input, "length", "()J", &[]) + .and_then(|result| result.j()) + .map_err(|_| String::from("Failed to get length")) } } diff --git a/musikr/src/main/jni/src/lib.rs b/musikr/src/main/jni/src/lib.rs index f6378b13a..37028ec3e 100644 --- a/musikr/src/main/jni/src/lib.rs +++ b/musikr/src/main/jni/src/lib.rs @@ -1,3 +1,7 @@ +#![no_std] +extern crate core; +extern crate alloc; + use jni::objects::{JClass, JObject}; use jni::sys::jstring; use jni::JNIEnv; @@ -8,6 +12,10 @@ mod jni_stream; pub use taglib::*; use jni_stream::JInputStream; +use alloc::string::String; + +type Map = alloc::collections::BTreeMap; + #[no_mangle] pub extern "C" fn Java_org_oxycblt_musikr_metadata_MetadataJNI_openFile<'local>( mut env: JNIEnv<'local>, @@ -18,7 +26,7 @@ pub extern "C" fn Java_org_oxycblt_musikr_metadata_MetadataJNI_openFile<'local>( let stream = match JInputStream::new(&mut env, input) { Ok(stream) => stream, Err(e) => { - let error = format!("Failed to create input stream: {}", e); + let error = String::from("Failed to create input stream"); let error_str = env.new_string(error).expect("Couldn't create error string!"); return error_str.into_raw(); } diff --git a/musikr/src/main/jni/src/taglib/ffi.rs b/musikr/src/main/jni/src/taglib/ffi.rs index 65a649558..311339523 100644 --- a/musikr/src/main/jni/src/taglib/ffi.rs +++ b/musikr/src/main/jni/src/taglib/ffi.rs @@ -1,7 +1,9 @@ -use std::ffi::CStr; -use std::pin::Pin; -use std::string::ToString; -use std::collections::HashMap; +use core::ffi::CStr; +use core::pin::Pin; + +use alloc::collections::BTreeMap; +use alloc::string::{String, ToString}; +use alloc::vec::Vec; #[cxx::bridge] pub(crate) mod bindings { @@ -340,7 +342,7 @@ impl bindings::XiphComment { } impl bindings::SimplePropertyMap { - pub fn to_hashmap(&self) -> HashMap> { + pub fn to_hashmap(&self) -> BTreeMap> { let cxx_vec = unsafe { // SAFETY: // - This pin is only used in this unsafe scope. @@ -371,6 +373,7 @@ impl bindings::Property { } } } + impl ToString for bindings::TString { fn to_string(&self) -> String { let c_str = unsafe { diff --git a/musikr/src/main/jni/src/taglib/mod.rs b/musikr/src/main/jni/src/taglib/mod.rs index 41fe95181..e960b4928 100644 --- a/musikr/src/main/jni/src/taglib/mod.rs +++ b/musikr/src/main/jni/src/taglib/mod.rs @@ -2,52 +2,12 @@ mod ffi; mod stream; use ffi::bindings; -use std::collections::HashMap; -pub use stream::{RustStream, TagLibStream}; +use alloc::boxed::Box; +use alloc::string::String; +use alloc::collections::BTreeMap; +use alloc::vec::Vec; +pub use stream::RustStream; -type XiphComments = HashMap>; - -pub enum File { - Unknown { - audio_properties: Option, - }, - MP3 { - audio_properties: Option, - }, - FLAC { - audio_properties: Option, - xiph_comments: Option, - }, - MP4 { - audio_properties: Option, - }, - OGG { - audio_properties: Option, - xiph_comments: Option, - }, - Opus { - audio_properties: Option, - xiph_comments: Option, - }, - WAV { - audio_properties: Option, - }, - WavPack { - audio_properties: Option, - }, - APE { - audio_properties: Option, - }, -} - -/// Audio properties of a media file -#[derive(Default)] -pub struct AudioProperties { - pub length_in_milliseconds: i32, - pub bitrate_in_kilobits_per_second: i32, - pub sample_rate_in_hz: i32, - pub number_of_channels: i32, -} // Safe wrapper for FileRef that owns extracted data pub struct FileRef { @@ -56,7 +16,7 @@ pub struct FileRef { impl FileRef { /// Create a new FileRef from a stream implementing TagLibStream - pub fn from_stream<'a, T: TagLibStream + 'a>(stream: T) -> Option { + pub fn from_stream<'a, T: IOStream + 'a>(stream: T) -> Option { // Create the RustStream wrapper let rust_stream = stream::RustStream::new(stream); @@ -114,3 +74,64 @@ impl FileRef { &self.file } } + +// Trait that must be implemented by Rust streams to be used with TagLib +pub trait IOStream { + fn name(&mut self) -> String; + fn read(&mut self, buf: &mut [u8]) -> Result; + fn seek(&mut self, whence: Whence) -> Result<(), String>; + fn tell(&mut self) -> Result; + fn length(&mut self) -> Result; + // No tag editing support, only doing read-only streams for now + // fn write(&mut self, _buf: &[u8]) -> std::io::Result; +} + +pub enum Whence { + Start(i64), + Current(i64), + End(i64), +} + +/// Audio properties of a media file +#[derive(Default)] +pub struct AudioProperties { + pub length_in_milliseconds: i32, + pub bitrate_in_kilobits_per_second: i32, + pub sample_rate_in_hz: i32, + pub number_of_channels: i32, +} + +type XiphComments = BTreeMap>; + +pub enum File { + Unknown { + audio_properties: Option, + }, + MP3 { + audio_properties: Option, + }, + FLAC { + audio_properties: Option, + xiph_comments: Option, + }, + MP4 { + audio_properties: Option, + }, + OGG { + audio_properties: Option, + xiph_comments: Option, + }, + Opus { + audio_properties: Option, + xiph_comments: Option, + }, + WAV { + audio_properties: Option, + }, + WavPack { + audio_properties: Option, + }, + APE { + audio_properties: Option, + }, +} diff --git a/musikr/src/main/jni/src/taglib/stream.rs b/musikr/src/main/jni/src/taglib/stream.rs index 80b6b1366..ecd37c6b0 100644 --- a/musikr/src/main/jni/src/taglib/stream.rs +++ b/musikr/src/main/jni/src/taglib/stream.rs @@ -1,19 +1,15 @@ -use std::ffi::{c_void, CString}; -use std::io::{Read, Seek, SeekFrom, Write}; -use std::os::raw::c_char; - -// Trait that must be implemented by Rust streams to be used with TagLib -pub trait TagLibStream: Read + Write + Seek { - fn name(&mut self) -> String; - fn is_readonly(&self) -> bool; -} +use core::ffi::{c_void, c_char}; +use alloc::ffi::CString; +use alloc::boxed::Box; +use alloc::slice; +use super::{IOStream, Whence}; // Opaque type for C++ #[repr(C)] -pub struct RustStream<'a>(Box); +pub struct RustStream<'a>(Box); impl<'a> RustStream<'a> { - pub fn new(stream: T) -> Self { + pub fn new(stream: T) -> Self { RustStream(Box::new(stream)) } } @@ -34,21 +30,10 @@ pub extern "C" fn rust_stream_read( length: usize, ) -> usize { let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; - let buffer = unsafe { std::slice::from_raw_parts_mut(buffer, length) }; + let buffer = unsafe { slice::from_raw_parts_mut(buffer, length) }; stream.0.read(buffer).unwrap_or(0) } -#[no_mangle] -pub extern "C" fn rust_stream_write( - stream: *mut c_void, - data: *const u8, - length: usize, -) { - let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; - let data = unsafe { std::slice::from_raw_parts(data, length) }; - stream.0.write_all(data).unwrap(); -} - #[no_mangle] pub extern "C" fn rust_stream_seek( stream: *mut c_void, @@ -57,38 +42,22 @@ pub extern "C" fn rust_stream_seek( ) { let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; let pos = match whence { - 0 => SeekFrom::Start(offset as u64), - 1 => SeekFrom::Current(offset), - 2 => SeekFrom::End(offset), + 0 => Whence::Start(offset), + 1 => Whence::Current(offset), + 2 => Whence::End(offset), _ => panic!("Invalid seek whence"), }; stream.0.seek(pos).unwrap(); } -#[no_mangle] -pub extern "C" fn rust_stream_truncate(stream: *mut c_void, length: i64) { - let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; - stream.0.seek(SeekFrom::Start(length as u64)).unwrap(); - // TODO: Actually implement truncate once we have a better trait bound -} - #[no_mangle] pub extern "C" fn rust_stream_tell(stream: *mut c_void) -> i64 { - let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; - stream.0.seek(SeekFrom::Current(0)).unwrap() as i64 + let stream: &mut RustStream<'_> = unsafe { &mut *(stream as *mut RustStream<'_>) }; + stream.0.tell().unwrap() } #[no_mangle] pub extern "C" fn rust_stream_length(stream: *mut c_void) -> i64 { let stream = unsafe { &mut *(stream as *mut RustStream<'_>) }; - let current = stream.0.seek(SeekFrom::Current(0)).unwrap(); - let end = stream.0.seek(SeekFrom::End(0)).unwrap(); - stream.0.seek(SeekFrom::Start(current)).unwrap(); - end as i64 + stream.0.length().unwrap() } - -#[no_mangle] -pub extern "C" fn rust_stream_is_readonly(stream: *const c_void) -> bool { - let stream = unsafe { &*(stream as *const RustStream<'_>) }; - stream.0.is_readonly() -} \ No newline at end of file