From 2ebea695b0f2406612d2eaee812a7b33c24decd9 Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 09:28:17 -0600 Subject: [PATCH 1/8] fix: skip ID3v2 tags in Mp3Decoder to fix 0x807f00fd error Set mp3_stream_start to after the ID3v2 tag header so the PSP's hardware MP3 decoder sees raw frames instead of tag metadata. Without this, sceMp3Init returns 0x807f00fd on files with ID3v2 tags. Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index e897ccc..f62a4a5 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -69,12 +69,15 @@ impl Mp3Decoder { return Err(Mp3Error(ret)); } + // Skip ID3v2 tag so the decoder sees raw MP3 frames. + let start_offset = skip_id3v2(data); + let owned_data = Vec::from(data); let mut mp3_buf = alloc::vec![0u8; MP3_BUF_SIZE]; let mut pcm_buf = alloc::vec![0i16; PCM_BUF_SIZE]; let mut init_arg = sys::SceMp3InitArg { - mp3_stream_start: 0, + mp3_stream_start: start_offset as u32, unk1: 0, mp3_stream_end: owned_data.len() as u32, unk2: 0, @@ -100,7 +103,13 @@ impl Mp3Decoder { }; // Feed initial data. - decoder.feed_data()?; + if let Err(e) = decoder.feed_data() { + unsafe { + sys::sceMp3ReleaseMp3Handle(handle); + sys::sceMp3TermResource(); + } + return Err(e); + } // Initialize the decoder. let ret = unsafe { sys::sceMp3Init(handle) }; @@ -203,8 +212,13 @@ impl Mp3Decoder { return Ok(()); } + // SAFETY: src_offset and copy_len are bounds-checked above. unsafe { - core::ptr::copy_nonoverlapping(self._data.as_ptr().add(src_offset), dst_ptr, copy_len); + core::ptr::copy_nonoverlapping( + self._data.as_ptr().add(src_offset), + dst_ptr, + copy_len, + ); } let ret = unsafe { sys::sceMp3NotifyAddStreamData(self.handle, copy_len as i32) }; From dafac66c158d0db49dae90a4ac7c322b60fb5784 Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 10:17:45 -0600 Subject: [PATCH 2/8] feat: add Mp3Decoder::release() to avoid TermResource between songs The PSP crashes when sceMp3TermResource() + sceMp3InitResource() are called in sequence (e.g. stopping one song and starting another). release() frees the handle via sceMp3ReleaseMp3Handle but skips the global TermResource, keeping the MP3 subsystem alive for reuse. Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index f62a4a5..33b9be2 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -179,6 +179,17 @@ impl Mp3Decoder { if ret < 0 { Err(Mp3Error(ret)) } else { Ok(()) } } + /// Release the MP3 handle without terminating the global resource. + /// + /// Use this instead of dropping when another decoder will be created + /// afterward (e.g. switching songs). The global MP3 resource subsystem + /// stays initialized so the next `Mp3Decoder::new()` succeeds without + /// a full Init→Term→Init cycle (which crashes on real PSP hardware). + pub fn release(self) { + unsafe { sys::sceMp3ReleaseMp3Handle(self.handle) }; + core::mem::forget(self); + } + /// Feed data from the source buffer into the decoder's stream buffer. fn feed_data(&mut self) -> Result<(), Mp3Error> { let mut dst_ptr: *mut u8 = core::ptr::null_mut(); From b4eced62920f108de2c897add533e192a4bc007b Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 10:30:28 -0600 Subject: [PATCH 3/8] fix: free heap buffers in Mp3Decoder::release() instead of leaking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit core::mem::forget(self) prevented TermResource but also leaked the Vec data/mp3_buf/pcm_buf — exhausting PSP RAM after a few songs. Use ManuallyDrop + drop_in_place to free the Vecs while still skipping the Drop impl (which calls sceMp3TermResource). Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index 33b9be2..a6d408b 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -185,9 +185,18 @@ impl Mp3Decoder { /// afterward (e.g. switching songs). The global MP3 resource subsystem /// stays initialized so the next `Mp3Decoder::new()` succeeds without /// a full Init→Term→Init cycle (which crashes on real PSP hardware). + /// + /// Heap buffers are freed normally — only `sceMp3TermResource` is skipped. pub fn release(self) { - unsafe { sys::sceMp3ReleaseMp3Handle(self.handle) }; - core::mem::forget(self); + let mut this = core::mem::ManuallyDrop::new(self); + unsafe { sys::sceMp3ReleaseMp3Handle(this.handle) }; + // Free heap buffers without running Drop (which calls TermResource). + // SAFETY: Each field is valid and only dropped once. + unsafe { + core::ptr::drop_in_place(&mut this._data); + core::ptr::drop_in_place(&mut this.mp3_buf); + core::ptr::drop_in_place(&mut this.pcm_buf); + } } /// Feed data from the source buffer into the decoder's stream buffer. From c5a61b8b32c11257f21d8df5ea221b81d5c5a2fd Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 10:39:48 -0600 Subject: [PATCH 4/8] refactor: split Mp3Decoder into new() and new_reuse() constructors new() calls sceMp3InitResource (first-time init). new_reuse() skips it (resource already initialized from prior session). This lets callers do init-once + release/reuse without stacking InitResource calls, which crashes on real PSP after ~2 calls. Also adds suppress_drop() for clean error-path cleanup without triggering the Drop impl's TermResource call. Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 53 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index a6d408b..928ded4 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -61,15 +61,32 @@ const PCM_BUF_SIZE: usize = 4608; // 1152 samples * 2 channels * 2 bytes (as i16 impl Mp3Decoder { /// Create a decoder from in-memory MP3 data. /// - /// Initializes the MP3 resource subsystem, reserves a handle, and - /// feeds the initial data to the decoder. + /// Calls `sceMp3InitResource` and cleans up with `sceMp3TermResource` + /// on failure. For repeated decode sessions (song switching), prefer + /// calling `sceMp3InitResource` once at startup and using + /// [`new_reuse`](Self::new_reuse) for each song. pub fn new(data: &[u8]) -> Result { let ret = unsafe { sys::sceMp3InitResource() }; if ret < 0 { return Err(Mp3Error(ret)); } + Self::create(data).map_err(|e| { + unsafe { sys::sceMp3TermResource() }; + e + }) + } + + /// Create a decoder when the MP3 resource subsystem is already initialized. + /// + /// Use after `sceMp3InitResource` has been called once (or after a + /// previous decoder was released via [`release`](Self::release)). + /// Avoids the Init→Term→Init cycle which crashes on real PSP hardware. + pub fn new_reuse(data: &[u8]) -> Result { + Self::create(data) + } - // Skip ID3v2 tag so the decoder sees raw MP3 frames. + /// Internal constructor — does not call InitResource or TermResource. + fn create(data: &[u8]) -> Result { let start_offset = skip_id3v2(data); let owned_data = Vec::from(data); @@ -89,7 +106,6 @@ impl Mp3Decoder { let handle_id = unsafe { sys::sceMp3ReserveMp3Handle(&mut init_arg) }; if handle_id < 0 { - unsafe { sys::sceMp3TermResource() }; return Err(Mp3Error(handle_id)); } let handle = sys::Mp3Handle(handle_id); @@ -104,20 +120,16 @@ impl Mp3Decoder { // Feed initial data. if let Err(e) = decoder.feed_data() { - unsafe { - sys::sceMp3ReleaseMp3Handle(handle); - sys::sceMp3TermResource(); - } + unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; + decoder.suppress_drop(); return Err(e); } // Initialize the decoder. let ret = unsafe { sys::sceMp3Init(handle) }; if ret < 0 { - unsafe { - sys::sceMp3ReleaseMp3Handle(handle); - sys::sceMp3TermResource(); - } + unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; + decoder.suppress_drop(); return Err(Mp3Error(ret)); } @@ -183,14 +195,13 @@ impl Mp3Decoder { /// /// Use this instead of dropping when another decoder will be created /// afterward (e.g. switching songs). The global MP3 resource subsystem - /// stays initialized so the next `Mp3Decoder::new()` succeeds without - /// a full Init→Term→Init cycle (which crashes on real PSP hardware). + /// stays initialized so the next [`new_reuse`](Self::new_reuse) call + /// succeeds without an Init→Term→Init cycle. /// /// Heap buffers are freed normally — only `sceMp3TermResource` is skipped. pub fn release(self) { let mut this = core::mem::ManuallyDrop::new(self); unsafe { sys::sceMp3ReleaseMp3Handle(this.handle) }; - // Free heap buffers without running Drop (which calls TermResource). // SAFETY: Each field is valid and only dropped once. unsafe { core::ptr::drop_in_place(&mut this._data); @@ -199,6 +210,18 @@ impl Mp3Decoder { } } + /// Free heap buffers without running Drop (skips both ReleaseMp3Handle + /// and TermResource). Used in error paths where the handle was already + /// released by the caller. + fn suppress_drop(self) { + let mut this = core::mem::ManuallyDrop::new(self); + unsafe { + core::ptr::drop_in_place(&mut this._data); + core::ptr::drop_in_place(&mut this.mp3_buf); + core::ptr::drop_in_place(&mut this.pcm_buf); + } + } + /// Feed data from the source buffer into the decoder's stream buffer. fn feed_data(&mut self) -> Result<(), Mp3Error> { let mut dst_ptr: *mut u8 = core::ptr::null_mut(); From 632a846cf163796f18345a75e56232685797a3c6 Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 10:53:01 -0600 Subject: [PATCH 5/8] feat: add Mp3Decoder::reload() for crash-free song switching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PSP's sceMp3 Release→Reserve cycle crashes on real hardware. reload() reuses the existing handle by resetting the play position and re-feeding new data, avoiding Release/Reserve/Term/Init entirely. Changes: - Strip ID3v2 tag before storing data (mp3_stream_start always 0) - Use large mp3_stream_end sentinel so handle works for any file size - Add reload() method: reset + replace data + re-feed - Remove release/new_reuse/suppress_drop (no longer needed) Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 89 +++++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 51 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index 928ded4..ce9cc4c 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -41,9 +41,12 @@ impl core::fmt::Display for Mp3Error { /// /// Decodes MP3 data using the PSP's hardware decoder. The MP3 data is /// provided as a byte slice and must remain valid for the decoder's lifetime. +/// +/// For song switching, use [`reload`](Self::reload) to swap in new data +/// without releasing the handle (which can crash on real PSP hardware). pub struct Mp3Decoder { handle: sys::Mp3Handle, - /// MP3 source data (kept alive for the duration of decoding). + /// MP3 source data (ID3v2 tag already stripped). _data: Vec, /// Internal stream buffer used by the MP3 decoder. mp3_buf: Vec, @@ -58,13 +61,16 @@ const MP3_BUF_SIZE: usize = 8 * 1024; /// Size of the internal PCM output buffer (max output per decode call). const PCM_BUF_SIZE: usize = 4608; // 1152 samples * 2 channels * 2 bytes (as i16 count) +/// Large sentinel for `mp3_stream_end` so a single handle can be reused +/// for files of any size. EOF is signalled by `feed_data` when the +/// actual source data runs out. +const STREAM_END_MAX: u32 = 0x0FFF_FFFF; // 256 MiB + impl Mp3Decoder { /// Create a decoder from in-memory MP3 data. /// - /// Calls `sceMp3InitResource` and cleans up with `sceMp3TermResource` - /// on failure. For repeated decode sessions (song switching), prefer - /// calling `sceMp3InitResource` once at startup and using - /// [`new_reuse`](Self::new_reuse) for each song. + /// Initializes the MP3 resource subsystem, reserves a handle, and + /// feeds the initial data to the decoder. pub fn new(data: &[u8]) -> Result { let ret = unsafe { sys::sceMp3InitResource() }; if ret < 0 { @@ -76,27 +82,19 @@ impl Mp3Decoder { }) } - /// Create a decoder when the MP3 resource subsystem is already initialized. - /// - /// Use after `sceMp3InitResource` has been called once (or after a - /// previous decoder was released via [`release`](Self::release)). - /// Avoids the Init→Term→Init cycle which crashes on real PSP hardware. - pub fn new_reuse(data: &[u8]) -> Result { - Self::create(data) - } - /// Internal constructor — does not call InitResource or TermResource. fn create(data: &[u8]) -> Result { + // Strip ID3v2 tag so all offsets are relative to raw MP3 frames. let start_offset = skip_id3v2(data); + let owned_data = Vec::from(&data[start_offset..]); - let owned_data = Vec::from(data); let mut mp3_buf = alloc::vec![0u8; MP3_BUF_SIZE]; let mut pcm_buf = alloc::vec![0i16; PCM_BUF_SIZE]; let mut init_arg = sys::SceMp3InitArg { - mp3_stream_start: start_offset as u32, + mp3_stream_start: 0, unk1: 0, - mp3_stream_end: owned_data.len() as u32, + mp3_stream_end: STREAM_END_MAX, unk2: 0, mp3_buf: mp3_buf.as_mut_ptr() as *mut c_void, mp3_buf_size: MP3_BUF_SIZE as i32, @@ -121,21 +119,41 @@ impl Mp3Decoder { // Feed initial data. if let Err(e) = decoder.feed_data() { unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; - decoder.suppress_drop(); + core::mem::forget(decoder); return Err(e); } - // Initialize the decoder. + // Initialize the decoder (parses first frame header). let ret = unsafe { sys::sceMp3Init(handle) }; if ret < 0 { unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; - decoder.suppress_drop(); + core::mem::forget(decoder); return Err(Mp3Error(ret)); } Ok(decoder) } + /// Reload the decoder with new MP3 data without releasing the handle. + /// + /// Resets the play position, replaces the source data, and re-feeds + /// the decoder. This avoids the Release→Reserve cycle which can crash + /// on real PSP hardware. + /// + /// After reload, metadata accessors (`sample_rate`, `channels`, etc.) + /// may return stale values until the first `decode_frame` call. + pub fn reload(&mut self, data: &[u8]) -> Result<(), Mp3Error> { + // Reset decoder to beginning of stream. + self.reset()?; + // Replace source data (strip ID3v2 tag). + let start_offset = skip_id3v2(data); + self._data = Vec::from(&data[start_offset..]); + self.eof = false; + // Re-feed initial data from the new source. + self.feed_data()?; + Ok(()) + } + /// Decode the next frame of MP3 data. /// /// Returns a slice of interleaved stereo i16 PCM samples. @@ -191,37 +209,6 @@ impl Mp3Decoder { if ret < 0 { Err(Mp3Error(ret)) } else { Ok(()) } } - /// Release the MP3 handle without terminating the global resource. - /// - /// Use this instead of dropping when another decoder will be created - /// afterward (e.g. switching songs). The global MP3 resource subsystem - /// stays initialized so the next [`new_reuse`](Self::new_reuse) call - /// succeeds without an Init→Term→Init cycle. - /// - /// Heap buffers are freed normally — only `sceMp3TermResource` is skipped. - pub fn release(self) { - let mut this = core::mem::ManuallyDrop::new(self); - unsafe { sys::sceMp3ReleaseMp3Handle(this.handle) }; - // SAFETY: Each field is valid and only dropped once. - unsafe { - core::ptr::drop_in_place(&mut this._data); - core::ptr::drop_in_place(&mut this.mp3_buf); - core::ptr::drop_in_place(&mut this.pcm_buf); - } - } - - /// Free heap buffers without running Drop (skips both ReleaseMp3Handle - /// and TermResource). Used in error paths where the handle was already - /// released by the caller. - fn suppress_drop(self) { - let mut this = core::mem::ManuallyDrop::new(self); - unsafe { - core::ptr::drop_in_place(&mut this._data); - core::ptr::drop_in_place(&mut this.mp3_buf); - core::ptr::drop_in_place(&mut this.pcm_buf); - } - } - /// Feed data from the source buffer into the decoder's stream buffer. fn feed_data(&mut self) -> Result<(), Mp3Error> { let mut dst_ptr: *mut u8 = core::ptr::null_mut(); From 79ef6223d54b87e4e18a61651210a337525bde90 Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 11:20:00 -0600 Subject: [PATCH 6/8] feat: add Mp3Decoder::reload_owned() for zero-copy song switching reload_owned() takes ownership of the Vec and drains the ID3v2 prefix in-place, avoiding a full copy of the MP3 data. Combined with reload(), this gives callers a choice between borrowing and owning. Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index ce9cc4c..7ff13da 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -143,13 +143,25 @@ impl Mp3Decoder { /// After reload, metadata accessors (`sample_rate`, `channels`, etc.) /// may return stale values until the first `decode_frame` call. pub fn reload(&mut self, data: &[u8]) -> Result<(), Mp3Error> { - // Reset decoder to beginning of stream. self.reset()?; - // Replace source data (strip ID3v2 tag). let start_offset = skip_id3v2(data); self._data = Vec::from(&data[start_offset..]); self.eof = false; - // Re-feed initial data from the new source. + self.feed_data()?; + Ok(()) + } + + /// Like [`reload`](Self::reload) but takes ownership of the data Vec + /// to avoid an extra copy. The ID3v2 tag prefix (if any) is drained + /// in-place. + pub fn reload_owned(&mut self, mut data: Vec) -> Result<(), Mp3Error> { + self.reset()?; + let start_offset = skip_id3v2(&data); + if start_offset > 0 { + data.drain(..start_offset); + } + self._data = data; + self.eof = false; self.feed_data()?; Ok(()) } From df2d39cc5f3c61c168aa8354564d546efddba22d Mon Sep 17 00:00:00 2001 From: AI Agent Bot Date: Sun, 15 Feb 2026 11:33:59 -0600 Subject: [PATCH 7/8] docs: add sceMp3 stability warning and audiocodec recommendation The sceMp3* API is unstable for handle reuse on real PSP hardware (PSP-3000 + 6.20 PRO-C). All approaches (drop+new, reload, reload_owned) crash after ~2-3 songs. PPSSPP does not reproduce the issue. Document that sceAudiocodec (frame-by-frame) is the recommended API for MP3 playback with song switching, and cross-reference the mp3 module's find_sync/skip_id3v2 helpers from the audiocodec docs. Co-Authored-By: Claude Opus 4.6 --- psp/src/audiocodec.rs | 9 +++++++++ psp/src/mp3.rs | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/psp/src/audiocodec.rs b/psp/src/audiocodec.rs index 2e75838..fe35a85 100644 --- a/psp/src/audiocodec.rs +++ b/psp/src/audiocodec.rs @@ -4,6 +4,15 @@ //! ATRAC3plus audio frames. The codec uses the Media Engine (ME) coprocessor //! and requires EDRAM allocation. //! +//! **This is the recommended API for MP3 playback with song switching.** +//! The higher-level [`crate::mp3::Mp3Decoder`] (`sceMp3*`) is unstable on +//! real hardware when reusing handles across songs (see its module docs for +//! details). `AudiocodecDecoder` allocates EDRAM once and can decode +//! indefinitely — just pass new frame data on each call to [`AudiocodecDecoder::decode`]. +//! +//! For MP3 frame sync detection and ID3v2 tag stripping, see +//! [`crate::mp3::find_sync`] and [`crate::mp3::skip_id3v2`]. +//! //! # Codec Buffer Layout //! //! The `sceAudiocodec*` functions operate on a 65-word (`u32`) buffer with diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index 7ff13da..85681cc 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -3,6 +3,29 @@ //! Wraps the hardware-accelerated `sceMp3*` syscalls for decoding MP3 //! audio data into PCM samples suitable for playback via [`crate::audio`]. //! +//! # Stability Warning +//! +//! The `sceMp3*` API is **unstable for handle reuse** on real PSP hardware +//! (tested on PSP-3000 with 6.20 PRO-C CFW). Specifically: +//! +//! - Dropping a decoder and creating a new one (Release→Term→Init→Reserve +//! cycle) crashes after ~2 songs. +//! - Using [`Mp3Decoder::reload`] or [`Mp3Decoder::reload_owned`] to reuse +//! a handle (ResetPlayPosition + re-feed, no Release/Term) still crashes +//! after ~3 songs. +//! - All variations (with delays, staggered Init/Term, single-Init) exhibit +//! the same instability on real hardware. PPSSPP does not reproduce it. +//! +//! **For applications that switch between songs, use +//! [`crate::audiocodec::AudiocodecDecoder`] instead.** The `sceAudiocodec` +//! API provides frame-by-frame MP3 decoding with a single EDRAM allocation +//! that can be reused indefinitely. You will need to handle MP3 frame sync +//! detection yourself — see [`find_sync`] and [`skip_id3v2`] in this module +//! for helpers. +//! +//! The `sceMp3` API works fine for **single-song playback** (e.g., a menu +//! background track that plays once and never changes). +//! //! # Example //! //! ```ignore From 266356bc5fbb560cc1f2e6bd2981b5f263043bd2 Mon Sep 17 00:00:00 2001 From: AI Review Agent Date: Sun, 15 Feb 2026 11:58:13 -0600 Subject: [PATCH 8/8] fix: eliminate mem::forget leak in Mp3Decoder::create() error paths Extract feed_data into a free function (feed_data_raw) so initial data feeding can happen before the Mp3Decoder struct is constructed. On error, the Vec allocations (_data, mp3_buf, pcm_buf) now drop normally via Rust scoping instead of being leaked by core::mem::forget. Co-Authored-By: Claude Opus 4.6 --- psp/src/mp3.rs | 112 ++++++++++++++++++++++++------------------------- 1 file changed, 55 insertions(+), 57 deletions(-) diff --git a/psp/src/mp3.rs b/psp/src/mp3.rs index 85681cc..a4bc892 100644 --- a/psp/src/mp3.rs +++ b/psp/src/mp3.rs @@ -131,30 +131,30 @@ impl Mp3Decoder { } let handle = sys::Mp3Handle(handle_id); - let mut decoder = Self { - handle, - _data: owned_data, - mp3_buf, - pcm_buf, - eof: false, + // Feed initial data before constructing the struct so that on + // error the Vecs drop normally (no mem::forget needed). + let eof = match feed_data_raw(handle, &owned_data) { + Ok(eof) => eof, + Err(e) => { + unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; + return Err(e); + }, }; - // Feed initial data. - if let Err(e) = decoder.feed_data() { - unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; - core::mem::forget(decoder); - return Err(e); - } - // Initialize the decoder (parses first frame header). let ret = unsafe { sys::sceMp3Init(handle) }; if ret < 0 { unsafe { sys::sceMp3ReleaseMp3Handle(handle) }; - core::mem::forget(decoder); return Err(Mp3Error(ret)); } - Ok(decoder) + Ok(Self { + handle, + _data: owned_data, + mp3_buf, + pcm_buf, + eof, + }) } /// Reload the decoder with new MP3 data without releasing the handle. @@ -246,57 +246,55 @@ impl Mp3Decoder { /// Feed data from the source buffer into the decoder's stream buffer. fn feed_data(&mut self) -> Result<(), Mp3Error> { - let mut dst_ptr: *mut u8 = core::ptr::null_mut(); - let mut to_write: i32 = 0; - let mut src_pos: i32 = 0; - - let ret = unsafe { - sys::sceMp3GetInfoToAddStreamData( - self.handle, - &mut dst_ptr, - &mut to_write, - &mut src_pos, - ) - }; - if ret < 0 { - return Err(Mp3Error(ret)); - } - - if to_write <= 0 || dst_ptr.is_null() { + let eof = feed_data_raw(self.handle, &self._data)?; + if eof { self.eof = true; - return Ok(()); } + Ok(()) + } +} - let src_offset = src_pos as usize; - let available = self._data.len().saturating_sub(src_offset); - let copy_len = (to_write as usize).min(available); +/// Feed source data into the decoder's stream buffer. +/// +/// Returns `Ok(true)` when all data has been fed (EOF), `Ok(false)` otherwise. +/// This is a free function so it can be called before the `Mp3Decoder` struct +/// is fully constructed (avoiding `mem::forget` leaks on error paths). +fn feed_data_raw(handle: sys::Mp3Handle, data: &[u8]) -> Result { + let mut dst_ptr: *mut u8 = core::ptr::null_mut(); + let mut to_write: i32 = 0; + let mut src_pos: i32 = 0; + + let ret = unsafe { + sys::sceMp3GetInfoToAddStreamData(handle, &mut dst_ptr, &mut to_write, &mut src_pos) + }; + if ret < 0 { + return Err(Mp3Error(ret)); + } - if copy_len == 0 { - self.eof = true; - let _ = unsafe { sys::sceMp3NotifyAddStreamData(self.handle, 0) }; - return Ok(()); - } + if to_write <= 0 || dst_ptr.is_null() { + return Ok(true); + } - // SAFETY: src_offset and copy_len are bounds-checked above. - unsafe { - core::ptr::copy_nonoverlapping( - self._data.as_ptr().add(src_offset), - dst_ptr, - copy_len, - ); - } + let src_offset = src_pos as usize; + let available = data.len().saturating_sub(src_offset); + let copy_len = (to_write as usize).min(available); - let ret = unsafe { sys::sceMp3NotifyAddStreamData(self.handle, copy_len as i32) }; - if ret < 0 { - return Err(Mp3Error(ret)); - } + if copy_len == 0 { + let _ = unsafe { sys::sceMp3NotifyAddStreamData(handle, 0) }; + return Ok(true); + } - if src_offset + copy_len >= self._data.len() { - self.eof = true; - } + // SAFETY: src_offset and copy_len are bounds-checked above. + unsafe { + core::ptr::copy_nonoverlapping(data.as_ptr().add(src_offset), dst_ptr, copy_len); + } - Ok(()) + let ret = unsafe { sys::sceMp3NotifyAddStreamData(handle, copy_len as i32) }; + if ret < 0 { + return Err(Mp3Error(ret)); } + + Ok(src_offset + copy_len >= data.len()) } impl Drop for Mp3Decoder {