Skip to content

fix: PSP boot crash, audio stability, and streaming MP3 player#19

Merged
AndrewAltimit merged 9 commits intomainfrom
fix/psp-boot-debug
Feb 15, 2026
Merged

fix: PSP boot crash, audio stability, and streaming MP3 player#19
AndrewAltimit merged 9 commits intomainfrom
fix/psp-boot-debug

Conversation

@AndrewAltimit
Copy link
Owner

@AndrewAltimit AndrewAltimit commented Feb 15, 2026

Summary

  • Fix EBOOT boot crash from kernel-mode partition allocation on real PSP hardware
  • Fix priority inversion deadlock by replacing SpinMutex with bare atomics
  • Fix PRX codec scan stutter by reducing scan frequency
  • Fix JoinHandle::drop() killing background threads
  • Replace unstable sceMp3 API with streaming sceAudiocodec MP3 player
  • Add OASIS OS detection to PRX plugin for ME coprocessor EDRAM sharing
  • Update rust-psp dependency to main (ID3v2 skip + reload APIs merged upstream)

Audio Architecture

The sceMp3* high-level API is unstable on real PSP hardware (PSP-3000 + 6.20 PRO-C) -- crashes after ~2-3 song switches regardless of handle reuse strategy (drop+new, reload, reload_owned, timing delays). PPSSPP does not reproduce the issue.

New approach: stream MP3 from file using a fixed 32KB read buffer with frame-by-frame sceAudiocodec decoding. Zero large heap allocations, matching the PRX plugin's proven pattern. The AudiocodecDecoder EDRAM and AudioChannel are allocated once and reused indefinitely across songs.

Changes

Boot fixes:

  • Use SceKernelPrimaryUserPartition instead of kernel partition for volatile memory
  • Lazy AV module loading to avoid conflicts with PRX at boot
  • Bare atomics instead of SpinMutex for lock-free audio state

Audio player rewrite (audio.rs):

  • sceAudiocodec frame-by-frame decoder replaces sceMp3 streaming API
  • File streaming with 32KB read buffer (compact + refill pattern from PRX)
  • MP3 frame sync detection via psp::mp3::find_sync
  • ID3v2 tag skip via psp::mp3::skip_id3v2
  • Single EDRAM allocation reused across all songs
  • Error resilience: skip corrupt frames, bail after 100 consecutive errors

PRX plugin:

  • OASIS OS detection via user memory scan for "OASIS_OS" module string
  • Skips audio codec when EBOOT is running (shared ME EDRAM)

Dependency:

  • psp crate updated from fix/mp3-id3v2-skip branch to main

Test plan

  • EBOOT boots on real PSP-3000 + 6.20 PRO-C
  • First song plays correctly
  • Song switching works reliably (5+ songs tested)
  • Pause/resume works across song switches
  • Volume control works
  • No crashes after extended use

Generated with Claude Code

The default feature `kernel-mode` enabled `psp/kernel`, causing the
rust-psp global allocator to use partition 1 (kernel memory) for all
heap allocations. Since the EBOOT runs in user mode, the kernel
partition is inaccessible — every allocation returned ILLEGAL_PARTITION,
triggering an OOM panic on the first heap allocation (font atlas).

Change default features to empty so the allocator uses partition 2
(user memory). Kernel features remain available via `--features
kernel-mode` for real PSP hardware with CFW kernel access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Code Review

Issues (if any)

(none)

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

  • crates/oasis-backend-psp/Cargo.toml:17 - Consider if any logic depends on kernel-mode being a superset of all kernel features. Currently, kernel-exception is excluded from the kernel-mode feature bundle (likely intentional given the comment about PSP-3000 bugs).

Notes

  • 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

  • The fix correctly addresses the OOM panic on boot by moving the default build to user-mode memory partition.

  • Verified that #[cfg] gates for kernel-specific features (kernel-volatile, kernel-me, etc.) are present in the codebase and will correctly deactivate in the new default user-mode build.

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

The codec NID scan + stub extraction was running every 2s for up to 60s
(30 attempts), causing periodic stutter in games. Reduce to 3 attempts
at 15s intervals (45s total) so only a few hiccups occur before the
fallback to loading AV modules directly.

Also clarify kernel-mode feature comments per review: kernel-exception
is intentionally excluded from the kernel-mode bundle (broken on
PSP-3000 + 6.20 PRO-C).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

  • crates/oasis-plugin-psp/src/audio.rs:1285 - Minor comment inconsistency. The comment states "up to 45s (3 attempts)", but the loop logic results in 2 delays of 15s (30s total sleep) before falling back.

Notes

  • 3 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

  • The fix correctly addresses the ILLEGAL_PARTITION panic by defaulting the oasis-backend-psp EBOOT to user-mode memory (partition 2).

  • Kernel features (kernel-me, kernel-me-clock, kernel-volatile) are appropriately #[cfg]-gated, ensuring graceful degradation when kernel-mode is disabled.

  • The reduction in polling frequency in oasis-plugin-psp/src/audio.rs is a sensible optimization to prevent CPU-intensive NID scanning from causing stutter during game initialization.

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

AI Agent Bot and others added 2 commits February 15, 2026 07:00
…equency

The EBOOT never loaded AVCODEC/MPEGBASE/MP3 modules, so the PRX
overlay's codec NID scan always failed repeatedly (causing periodic
stutter) before falling back to loading modules itself.

Now the EBOOT calls sceUtilityLoadModule for all three AV modules
during init, so the PRX finds them on its first scan attempt.

Also reduce the PRX scan from 30 attempts x 2s to 3 attempts x 15s
as a safety net for games that load codecs late.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Loading AvCodec/AvMpegBase/AvMp3 via sceUtilityLoadModule conflicts
with the EBOOT's statically-linked sceMp3 imports -- both resolve to
the same kernel codec but through different paths, causing
sceMp3Decode to hang when the user plays audio.

The PRX overlay handles its own module loading via its fallback path
(load_av_modules after 3 scan attempts).  With the reduced scan
frequency (3x15s) the stutter is minimal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

  • [RESOLVED] crates/oasis-backend-psp/Cargo.toml:17 - Default build now correctly targets user-mode partition by disabling kernel-mode by default.
  • [RESOLVED] crates/oasis-plugin-psp/src/audio.rs:1285 - Polling frequency reduced to minimize stutter, though timing comment remains slightly imprecise.

Suggestions (if any)

  • crates/oasis-plugin-psp/src/audio.rs:1285 - Minor comment correction.
    • The comment states "up to 45s (3 attempts)", but the loop logic (3 attempts with 15s intervals) completes the final check at T=30s. Consider updating to "up to 30s" or adding a 4th attempt if 45s was the intended timeout.

Notes

  • The fix correctly resolves the ILLEGAL_PARTITION panic on boot for user-mode EBOOTs by moving kernel-specific features to an optional bundle.
  • Verified that granular features like kernel-volatile and kernel-me correctly enable psp/kernel, which is appropriate as these APIs typically require kernel privileges or specific memory partition access.
  • The optimization in audio.rs to use a 15s interval for NID scanning is a prudent change to avoid performance degradation during game startup on real hardware.

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

@AndrewAltimit AndrewAltimit changed the title fix: PSP EBOOT boot crash from kernel-mode partition allocation fix: PSP EBOOT boot crash and PRX overlay audio stutter Feb 15, 2026
@AndrewAltimit
Copy link
Owner Author

Note on the sceUtilityLoadModule add/revert (commits 5e90111 -> 9f566f3):

We tried pre-loading AvCodec/AvMpegBase/AvMp3 via sceUtilityLoadModule in the EBOOT so the PRX overlay could piggyback on them immediately (zero-stutter codec resolution). This worked in PPSSPP -- all three modules loaded successfully (return 0) and the EBOOT ran fine.

However, on real hardware this caused a deadlock when the EBOOT's music player tried to play audio. The root cause: the EBOOT statically imports sceMp3 (stubs resolved by the kernel at module load), and sceMp3 internally depends on sceAudiocodec (AvCodec). When we also dynamically loaded the same modules via sceUtilityLoadModule, both the static and dynamic paths resolved to the same kernel codec mutex but through different registration entries. sceMp3Decode() would hang waiting for the codec, freezing the app (though the kernel remained responsive -- HOME button still worked).

The PRX's own fallback mechanism (load_av_modules() after 3 scan timeouts) doesn't have this problem because it resolves NIDs through sctrlHENFindFunction in kernel space, which operates on the kernel module table rather than user-space utility module loading.

Takeaway: Don't mix sceUtilityLoadModule with statically-imported PSP modules that share internal codec resources. The PRX should be the only one loading AV modules dynamically.

…n deadlock

On single-core PSP with preemptive scheduling, the audio thread
(priority 16/high) preempting the main thread (priority 32/low) while
it holds the SpinMutex causes deadlock -- the audio thread spins
forever waiting for a lock that the main thread can never release.

Replace SharedAudioState/SpinMutex with individual AtomicBool/AtomicU32
fields for lock-free reads and writes with zero contention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@AndrewAltimit AndrewAltimit changed the title fix: PSP EBOOT boot crash and PRX overlay audio stutter fix: PSP EBOOT boot crash, audio deadlock, and PRX codec scan stutter Feb 15, 2026
@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues

  • [BUG] crates/oasis-backend-psp/src/threading.rs:68 - Potential data race/inconsistency in audio metadata reading
    • While AtomicU32 prevents memory corruption, reading sample_rate, bitrate, and channels via individual load calls in PspAudioBackend is not atomic as a group. A reader might see metadata from a new track while still seeing the "playing" state of an old track if is_playing() is checked at the wrong moment.
    • Since this is a simple UI display on a single-core system, the impact is low (flicker), but using a version counter or a single atomic 64-bit word for small metadata sets would be more robust.

Previous Issues

  • [RESOLVED] crates/oasis-backend-psp/Cargo.toml:17 - default feature is now empty, correctly avoiding kernel-mode OOM on boot.
  • [RESOLVED] crates/oasis-plugin-psp/src/audio.rs:1285 - NID scan frequency reduced to 15s intervals to prevent game stutter.

Suggestions

  • crates/oasis-plugin-psp/src/audio.rs:1285 - The comment "up to 45s (3 attempts)" is mathematically slightly off (3 attempts with 15s delays in between means the last check happens at T=30s). Suggest updating to "up to 30s" or adding a 4th attempt to match the 45s claim.
  • crates/oasis-backend-psp/src/threading.rs:280 - Consider using Ordering::Release for metadata stores and Ordering::Acquire for is_playing() loads to ensure proper memory visibility of the metadata when the "playing" flag is set.

Notes

  • The replacement of SpinMutex with atomics in threading.rs is a critical and correct fix for priority inversion deadlocks on the PSP's single-core MIPS processor.
  • The revert of sceUtilityLoadModule in the EBOOT is necessary due to the documented conflict with static sceMp3 imports and kernel-level codec mutexes.
  • The logic in publish_audio_state correctly stores metadata before setting the AUDIO_PLAYING flag, which minimizes the window for inconsistent state reads.

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

AI Agent Bot and others added 2 commits February 15, 2026 09:28
- Leak JoinHandles with core::mem::forget() to prevent
  sceKernelTerminateDeleteThread from killing worker threads on drop
- Defer sceUtilityLoadModule (AvCodec/AvMpegBase/AvMp3) to first play
  to avoid ME EDRAM conflict with PRX overlay at boot time
- Remove sceMp3InitResource probe from audio.rs (SDK now skips ID3v2)
- Remove all file-based debug logging and atomic debug counters
- Use sceKernelDelayThread directly for idle sleep (avoids TLS)
- Switch psp dependency to local path for ID3v2 skip fix

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace sceMp3 with sceAudiocodec for EBOOT MP3 playback. The sceMp3
API is unstable on real PSP hardware (PSP-3000 + 6.20 PRO-C), crashing
after ~2-3 song switches regardless of handle reuse strategy.

New approach streams MP3 from file using a fixed 32KB read buffer with
frame-by-frame sceAudiocodec decoding -- zero large heap allocations,
matching the PRX plugin's proven pattern. Decoder EDRAM and audio
channel are allocated once and reused across all songs.

Also adds OASIS OS detection to the PRX plugin so it skips audio codec
usage when the EBOOT is running (both share the ME coprocessor's EDRAM).

Updates rust-psp dependency from fix/mp3-id3v2-skip branch to main
(ID3v2 skip fix, reload APIs, and stability docs now merged upstream).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@AndrewAltimit AndrewAltimit changed the title fix: PSP EBOOT boot crash, audio deadlock, and PRX codec scan stutter fix: PSP boot crash, audio stability, and streaming MP3 player Feb 15, 2026
@AndrewAltimit
Copy link
Owner Author

Audio player rewrite: sceMp3 -> sceAudiocodec + file streaming

The root cause of the 3rd-song crash was identified through extensive testing on real PSP-3000 hardware:

sceMp3 API instability: The sceMp3* high-level API crashes after ~2-3 song switches on real hardware, regardless of handle reuse strategy. All approaches were tested and documented:

  1. Drop + new (Release->Term->Init->Reserve cycle) -- crash on 2nd song
  2. release() + new_reuse() (skip Term/Init) -- crash on 3rd song
  3. reload() / reload_owned() (ResetPlayPosition, no Release) -- crash on 3rd song
  4. Memory optimization + timing delays -- crash on 3rd song

PPSSPP does not reproduce any of these issues.

Working solution: Replaced sceMp3 with sceAudiocodec frame-by-frame decoding + file streaming:

  • Stream MP3 from file using a fixed 32KB read buffer (no large heap allocations)
  • AudiocodecDecoder EDRAM allocated once, reused across all songs
  • MP3 frame sync + header parsing using psp::mp3::find_sync / skip_id3v2
  • Compact + stream-refill pattern (4KB chunks) to avoid audio stalls

The sceMp3 findings and audiocodec recommendation have been documented upstream in rust-psp (AndrewAltimit/rust-psp#9, now merged to main). The psp dependency has been updated accordingly.

Tested: 5+ consecutive song switches with no crashes, pause/resume works, volume control works.

@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues

  • [BUG] crates/oasis-backend-psp/src/audio.rs:328 - Race condition between file write and read

    • After writing MP3 data to temp file, reopening for reading may occur before kernel flushes write buffer
    • Call sceIoSync or ensure write descriptor is fully closed before reopening for reading
  • [BUG] crates/oasis-backend-psp/src/audio.rs:189 - Incomplete ID3v2 tag skipping

    • skip_id3v2 only checks initial read_buf. If ID3v2 tag exceeds READ_BUF_SIZE (32KB), common for MP3s with embedded album art, buf_pos will point within the tag, causing decoder to parse ID3 data as MP3 frames
    • Implement loop to seek past ID3v2 tag based on size header when it exceeds current buffer
  • [BUG] crates/oasis-backend-psp/src/threading.rs:271 - Non-atomic multi-word metadata update

    • Reader in main thread could observe inconsistent metadata (e.g., new sample rate but old duration) between individual store calls in publish_audio_state
    • Use sequence counter (atomic increment before and after update) to allow readers to detect and retry during concurrent update

Previous Issues

  • [STILL UNRESOLVED] crates/oasis-backend-psp/src/threading.rs:68 - Logical inconsistency in audio metadata reading
    • While memory safety guaranteed by atomics, individual load calls for sample_rate, bitrate, etc., do not guarantee consistent snapshot across fields

Suggestions

  • crates/oasis-plugin-psp/src/audio.rs:1285 - Update comment "up to 45s (3 attempts)" to reflect actual logic (3 attempts × 15s = 30s total sleep before fallback)
  • crates/oasis-backend-psp/src/audio.rs:356 - Use Ordering::Acquire when loading AUDIO_PLAYING and Ordering::Release when storing to ensure proper memory visibility of metadata fields

Notes

  • SpinMutex replacement with atomics is critical fix for priority inversion deadlock
  • is_oasis_running memory scan in PRX robustly avoids ME/codec conflicts without complex IPC
  • Transition to sceAudiocodec for frame-by-frame decoding significantly improves stability over high-level sceMp3 API on real hardware

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

Automated fix by Claude in response to Gemini/Codex review.

Iteration: 1/5

Co-Authored-By: AI Review Agent <noreply@anthropic.com>
@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 1)

Status: Changes committed and pushed

Commit: 99c38e6

Fixed Issues

  • [BUG] ID3v2 tag skip for large tags (crates/oasis-backend-psp/src/audio.rs:245-279): When an MP3 file has an ID3v2 tag larger than the 32KB read buffer (common with embedded album art), skip_id3v2() returns a value larger than buf_valid, the guard condition id3_skip < self.buf_valid fails, and buf_pos stays at 0 — causing the decoder to parse ID3 tag bytes as MP3 frames (noise, errors, potential crash). Fixed by seeking past the tag in the file and re-reading the buffer from the first audio frame position. Also adjusted data_size to exclude the tag for more accurate duration estimation.
  • [SUGGESTION] Comment math error (crates/oasis-plugin-psp/src/audio.rs:1285): Comment said "up to 45s (3 attempts)" but 3 attempts with 2 delays of 15s = 30s total sleep. Fixed to "up to 30s (3 attempts)".

Ignored Issues

  • [BUG] Race condition in load_and_play_owned file write/read (audio.rs:328): Not a real bug. sceIoClose() is synchronous on PSP and flushes the write. The new fd is opened after close completes.
  • [BUG] Non-atomic multi-word metadata update (threading.rs:271): Speculative. The code already stores metadata before setting AUDIO_PLAYING last (line 281-282). Worst case on single-core PSP is a one-frame UI flicker showing stale metadata. Adding a sequence counter adds complexity for no user-visible benefit.
  • [SUGGESTION] Use Release/Acquire ordering (threading.rs:280): PSP is single-core MIPS where all memory operations are naturally ordered. Relaxed ordering is sufficient and correct. Release/Acquire would add no value on this hardware.

Deferred to Human

  • None

Notes

  • Codex review failed due to OpenAI rate limits, so only Gemini feedback was available.
  • The formatting changes in crates/oasis-backend-psp/src/audio.rs are from cargo fmt normalizing the file I modified (PSP crate is excluded from workspace cargo fmt --all).
  • Workspace clippy and tests pass cleanly after the changes.

Automated summary of agent fixes.

@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues

  • [BUG] crates/oasis-backend-psp/src/audio.rs:252 - Potential crash on large ID3 tags

    • When id3_skip >= self.buf_valid, the code re-reads and seeks to id3_skip. If the ID3 tag is exceptionally large (e.g., several megabytes of album art), self.data_size.saturating_sub(id3_skip as u32) could overflow u32, causing incorrect duration or position calculations.
    • Check if id3_skip exceeds u32::MAX or cap the skip value for extremely large tags.
  • [BUG] crates/oasis-backend-psp/src/audio.rs:324 - Thread-safety of pcm_buf and read_buf

    • AudioPlayer is used within a loop in audio_thread_fn. If any AudioBackend trait implementation were to allow access to these buffers from the main thread while the audio thread is inside update(), it would cause a data race.
    • Ensure AudioPlayer remains strictly owned by the audio thread and is never shared via Arc or & with the main thread.

Previous Issues

  • [RESOLVED] crates/oasis-backend-psp/src/threading.rs:68 - Replaced SpinMutex with atomics to avoid deadlocks.
  • [RESOLVED] crates/oasis-backend-psp/src/audio.rs:189 - Large ID3 tags now handled by seeking past them in the file.
  • [RESOLVED] crates/oasis-backend-psp/src/threading.rs:271 - Metadata stored before AUDIO_PLAYING is set to ensure visibility.

Suggestions

  • crates/oasis-backend-psp/src/audio.rs:200 - Consider using std::ffi::CString instead of manual null-termination for safer path handling.

  • crates/oasis-backend-psp/src/audio.rs:434 - Clearing the entire 4KB pcm_buf every frame is inefficient on MIPS. Since the decoder returns bytes written, only clear the unwritten portion or trust the decoder to overwrite the buffer.

  • crates/oasis-plugin-psp/src/audio.rs:1506 - Scanning 1.5MB of memory word-by-word at boot (~393k iterations) is slow. Since SceModuleInfo is usually near the start of the text segment, reducing the scan range or checking only known module header offsets could speed up detection.

Notes

  • The transition to sceAudiocodec is a necessary architectural shift for real-hardware stability.
  • Using atomics instead of SpinMutex in threading.rs correctly addresses priority inversion risk on PSP's single-core CPU.
  • The "OASIS_OS" memory scan in the PRX is a clever, robust way to handle EDRAM resource contention without requiring kernel-level IPC.

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 2)

Status: No changes needed

Fixed Issues

  • [DOC] Stale API reference in is_oasis_running doc comment (crates/oasis-plugin-psp/src/audio.rs:1516): Updated "sceMp3" to "sceAudiocodec" to reflect the EBOOT's new audio architecture from this PR. The EBOOT no longer uses sceMp3 — it was replaced with sceAudiocodec frame-by-frame decoding.
  • [DOC] Incorrect scan range in doc comment (crates/oasis-plugin-psp/src/audio.rs:1520): Updated "first 1MB" to "first 1.5MB" to match the actual scan range (0x0880_0000 to 0x0898_0000 = 1.5MB). The code on line 1527 already correctly said "1.5MB" but the doc summary was inconsistent.

Ignored Issues

  • [BUG] audio.rs:252 - u32 overflow on large ID3 tags (Gemini): Not a real bug. PSP is 32-bit MIPS so usize is u32, making id3_skip as u32 a no-op. saturating_sub correctly handles the case where id3_skip > data_size by returning 0.
  • [BUG] audio.rs:324 - Thread-safety of pcm_buf and read_buf (Gemini): Speculative. AudioPlayer is created locally in audio_thread_fn() and is never shared. It's strictly owned by the audio thread.
  • [SUGGESTION] Use CString for path handling (Gemini): PSP no_std environment — manual null-termination is the standard pattern and avoids heap overhead from CString.
  • [SUGGESTION] Don't clear pcm_buf every frame (Gemini): Zeroing ensures no stale audio data leaks on partial decodes. The cost (~4.5KB zero-writes) is trivial compared to the blocking audio output call.
  • [SUGGESTION] Reduce memory scan range for OASIS detection (Gemini): The scan runs only 4 times during boot with multi-second delays. Not a hot path. Reducing range risks missing OASIS if loaded at higher addresses.

Deferred to Human

  • None

Notes

  • Codex review failed again due to OpenAI rate limits (gpt-5.2-codex), so only Gemini feedback was available.
  • The Gemini review for this iteration was mostly suggestions and speculative issues — no real bugs found. The one actionable item was the stale doc comment referencing sceMp3 when the EBOOT now uses sceAudiocodec.
  • Workspace clippy and format checks pass cleanly.

The agent reviewed feedback but determined no code changes were required.

@github-actions
Copy link

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues

  • [BUG] crates/oasis-backend-psp/src/audio.rs:160 - Potential double-subtraction of ID3 tag size

    • data_size is decremented by id3_skip, but the large tag handling (lines 245-279) may also perform this subtraction
    • Ensure the subtraction only happens once to avoid under-counting actual audio data size
  • [BUG] crates/oasis-backend-psp/src/audio.rs:431 - Incorrect buffer zeroing causes audio artifacts

    • Buffer is cleared before every decode, but decoder may only fill partially or return 0 when waiting for data
    • AudioChannel::output_blocking (line 462) outputs 4608 bytes regardless, causing pops/gaps when decoder hasn't produced samples
    • Only output to audio channel if consumed > 0 and use actual sample count if API provides it

Previous Issues

  • [RESOLVED] crates/oasis-backend-psp/src/threading.rs:68 - Replaced SpinMutex with bare atomics to prevent priority inversion deadlocks
  • [RESOLVED] crates/oasis-backend-psp/src/audio.rs:189 - Large ID3 tags handled by seeking and re-filling buffer
  • [RESOLVED] crates/oasis-backend-psp/src/threading.rs:271 - Metadata stored before AUDIO_PLAYING flag set
  • [RESOLVED] crates/oasis-backend-psp/src/audio.rs:328 - No race condition; sceIoClose is synchronous
  • [RESOLVED] crates/oasis-backend-psp/src/audio.rs:252 - saturating_sub handles skip logic safely

Suggestions

  • crates/oasis-backend-psp/src/audio.rs:38 - Make BITRATES and SAMPLE_RATES tables static instead of const to avoid binary duplication
  • crates/oasis-backend-psp/src/audio.rs:437 - Consider passing slice directly to decoder method instead of manually managing buf_pos index
  • crates/oasis-plugin-psp/src/audio.rs:1527 - Memory scan could be optimized by narrowing to likely .rodata segment offsets (already checks 4-byte alignment)

Notes

  • 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

  • Migration from sceMp3 to sceAudiocodec with manual streaming is architecturally sound for PSP hardware stability

  • Correct use of core::mem::forget on JoinHandle (lines 173, 186) ensures background threads persist for application lifetime

  • "OASIS_OS" string scan in PRX (line 1520) elegantly detects EDRAM contention without kernel-to-user IPC

Reaction


Generated by Gemini AI (gemini-3-flash-preview). Supplementary to human reviews.

@AndrewAltimit
Copy link
Owner Author

Review Response Agent (Iteration 3)

Status: No changes needed

Fixed Issues

  • None required

Ignored Issues

  • [BUG] audio.rs:160 - Potential double-subtraction of ID3 tag size (Gemini): Not a bug. The data_size.saturating_sub(id3_skip) at line 251 executes exactly once, before the if/else branch that handles small vs. large tags. Neither branch subtracts data_size again. The line numbers in the review (160) don't match the actual code location (251).
  • [BUG] audio.rs:431 - Incorrect buffer zeroing causes audio artifacts (Gemini): Not a bug. The review claims output_blocking outputs silence when consumed == 0, but the code explicitly returns early on consumed == 0 (line 451) and on decode errors (line 466). output_blocking is only reached after a successful decode with consumed > 0. The pre-zeroing ensures partial decodes don't leak stale audio — this is correct defensive behavior.
  • [SUGGESTION] Make BITRATES and SAMPLE_RATES static instead of const (Gemini): Negligible binary size difference for small lookup tables (~60 bytes). The compiler will optimize const tables to a single copy at the use site.
  • [SUGGESTION] Pass slice directly to decoder method (Gemini): The code already does this at line 440: decoder.decode(&self.read_buf[buf_pos..buf_valid], &mut self.pcm_buf).
  • [SUGGESTION] Narrow memory scan range for OASIS detection (Gemini): Already dismissed in iteration 2 — scan runs only 4 times at boot with multi-second delays. Not a hot path. Narrowing risks missing OASIS if loaded at higher addresses.

Deferred to Human

  • None

Notes

  • Codex review failed again due to OpenAI rate limits (gpt-5.2-codex), so only Gemini feedback was available.
  • The Gemini review for this iteration contained two BUG claims that both misread the control flow — one claiming double subtraction that doesn't exist, and one claiming output_blocking is called unconditionally when the code has explicit early returns. Both were hallucinated issues.
  • Workspace clippy and format checks pass cleanly.

The agent reviewed feedback but determined no code changes were required.

@AndrewAltimit AndrewAltimit merged commit a56e573 into main Feb 15, 2026
7 checks passed
@AndrewAltimit AndrewAltimit deleted the fix/psp-boot-debug branch February 15, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant