+15 -1

apps/pcmbuf.c

reviewed

··· 595 595 { 596 596 void *bufstart; 597 597 598 598 + /* Hold the PCM lock across the descriptor-ring rebuild. On native 599 599 + * targets PCM playback is interrupt-driven and the codec thread 600 600 + * can disable_interrupts() to keep the rebuild atomic. On hosted 601 601 + * pthread targets (the Android cdylib's pcm-aaudio.c) the writer 602 602 + * is a free-running pthread that calls pcm_play_dma_complete_callback 603 603 + * → pcmbuf_pcm_callback whenever AAudio drains; without this lock 604 604 + * it can read half-zeroed chunk descriptors / NULL'd callback 605 605 + * pointers mid-rebuild and crash with PC=0 (call through NULL). 606 606 + * The lock translates to the sink's sink_lock op (e.g. aa_mtx for 607 607 + * AAudio); it's a no-op on bare-metal targets. */ 608 608 + pcm_play_lock(); 609 609 + 598 610 /* Set up the buffers */ 599 611 pcmbuf_desc_count = get_next_required_pcmbuf_chunks(); 600 612 pcmbuf_size = pcmbuf_desc_count * PCMBUF_CHUNK_SIZE; ··· 611 623 612 624 #ifdef HAVE_CROSSFADE 613 625 pcmbuf_finish_crossfade_enable(); 614 614 - #else 626 626 + #else 615 627 pcmbuf_watermark = PCMBUF_WATERMARK; 616 628 #endif /* HAVE_CROSSFADE */ 617 629 618 630 init_buffer_state(); 619 631 620 632 pcmbuf_soft_mode(false); 633 633 + 634 634 + pcm_play_unlock(); 621 635 622 636 return bufend - bufstart; 623 637 }

+8

apps/playback.c

reviewed

··· 1500 1500 1501 1501 buf_signal_handle(ci.audio_hid, true); 1502 1502 1503 1503 + #if defined(CODECS_STATIC) 1504 1504 + /* Hosted/Android cdylib: codec_stop() crashes the kernel scheduler 1505 1505 + * (PC=0 inside wakeup_thread_) when the codec thread or aa_thread 1506 1506 + * is mid-callback. Yield for a tick first to let the runqueue 1507 1507 + * drain any pending wake/release before we hammer codec_queue. */ 1508 1508 + sleep(HZ / 10); 1509 1509 + #endif 1510 1510 + 1503 1511 if (stop) 1504 1512 codec_stop(); 1505 1513 else

+79 -1

crates/expo/README.md

reviewed

··· 164 164 3. `daemon.rs::rb_daemon_start`: 165 165 - Installs `tracing-android` subscriber → tag `rockbox` 166 166 - Sets env vars: `HOME`, `ROCKBOX_LIBRARY` (canonical, NOT `ROCKBOX_MUSIC_DIR`), 167 167 + `TMPDIR` (= `$HOME/tmp`, created on demand, so `std::env::temp_dir()` 168 168 + resolves into the app sandbox instead of `/tmp` which doesn't exist), 167 169 `ROCKBOX_DEVICE_NAME`, `ROCKBOX_PORT/GRAPHQL_PORT/TCP_PORT/MPD_PORT` 168 170 - Spawns `rockbox-engine` pthread (2 MB stack) which calls `main_c()` 169 171 4. `main_c()` — the firmware boot in `apps/main.c`. Initializes kernel, ··· 198 200 `start_server` / `start_servers` / `start_broker`. 199 201 200 202 If a sink stops working after a refactor, check for missing keepalives. 203 203 + 204 204 + ### Firmware-command bus (`crates/server/src/fw_bus.rs`) 205 205 + 206 206 + The Rockbox kernel scheduler identifies the "current thread" via a 207 207 + single global slot — `__cores[0].running` (see 208 208 + `firmware/kernel/thread-internal.h::__running_self_entry`). There is no 209 209 + thread-local storage. The whole machinery assumes only one OS thread 210 210 + (the rockbox-engine pthread) ever touches kernel-thread state, and 211 211 + that thread updates the slot on every coroutine context switch. 212 212 + 213 213 + This breaks immediately when our HTTP/gRPC handlers run on actix 214 214 + worker pthreads and call firmware FFI: `audio_play()`, `audio_pause()`, 215 215 + `playlist_start()`, `audio_set_crossfade()`, etc. all eventually reach 216 216 + `queue_send` → `wakeup_thread`, which read/write `__cores[0].running` 217 217 + treating themselves as that thread (whichever Rockbox kernel thread 218 218 + was last switched in). Result: kernel-thread struct corruption → 219 219 + SIGSEGV at `PC=0` in `wakeup_thread_` on track switches, settings 220 220 + updates, anything that crosses `audio_thread` ↔ `codec_thread`. 221 221 + 222 222 + The bus serialises every kernel-mutating call through an `mpsc` 223 223 + channel that the **broker thread** drains. Broker is created by 224 224 + `apps/broker_thread.c::broker_init` via `create_thread`, so it IS a 225 225 + real Rockbox kernel thread — its FFI calls resolve `__running_self_entry()` 226 226 + to its own `thread_entry` and the scheduler stays consistent. 227 227 + 228 228 + ```text 229 229 + actix worker (gRPC/HTTP handler) broker (Rockbox kernel thread) 230 230 + │ │ 231 231 + ├── fw_bus::run_on_broker(|| rb::*) ─┐ │ 232 232 + │ │ │ 233 233 + │ └── mpsc ──▶│ 234 234 + │ ↑ │ drain() loop: 235 235 + │ └── reply oneshot (5s timeout) ◀──────────────┤ - try_recv() 236 236 + │ │ - execute_on_broker() 237 237 + │ (handler returns) │ - sleep(0) yield 238 238 + │ - repeat 239 239 + ``` 240 240 + 241 241 + **API:** 242 242 + - `fw_bus::init()` — call once from `start_servers()` before any handler 243 243 + can run. Idempotent. 244 244 + - `fw_bus::send(FwCmd::…)` — fire-and-forget enqueue. 245 245 + - `fw_bus::send_and_wait(|reply| FwCmd::…)` — enqueue + block on the 246 246 + reply oneshot. 5-second cap before bailing. 247 247 + - `fw_bus::run_on_broker(|| -> T { rb::* })` — generic helper that 248 248 + wraps any closure in `FwCmd::Custom` and returns the closure's 249 249 + value. Use this for ad-hoc cases instead of adding a new `FwCmd` 250 250 + variant. 251 251 + - `fw_bus::drain(&rx)` — called once per broker iteration. Yields 252 252 + (`sleep(0)`) between commands so the recipient kernel thread (e.g. 253 253 + `audio_thread`) gets a chance to dequeue our message before the next 254 254 + one is sent. 255 255 + 256 256 + **What goes through the bus** (everything in 257 257 + `crates/server/src/handlers/`): 258 258 + 259 259 + - `player.rs` — `play`, `pause`, `resume`, `next`, `previous`, `stop`, 260 260 + `ff_rewind`, `flush_and_reload_tracks` 261 261 + - `playlists.rs` — `create_playlist`, `start_playlist`, 262 262 + `shuffle_playlist`, `resume_playlist`, `resume_track`, 263 263 + `insert_tracks`, `remove_tracks` 264 264 + - `saved_playlists.rs` / `smart_playlists.rs` — `play_*` (build + start) 265 265 + - `settings.rs` — `update_global_settings` (whole `load_settings` body) 266 266 + 267 267 + **What stays direct FFI** — read-only calls that don't touch the 268 268 + scheduler: `current_track`, `status`, `next_track`, `get_track_info`, 269 269 + `amount`, `index`, `sound::current`, etc. Performance matters for 270 270 + these (60+ Hz polling) and the race doesn't apply. 271 271 + 272 272 + When you add a new handler that mutates audio engine state, wrap the 273 273 + firmware-touching block in `crate::fw_bus::run_on_broker(move || …)`. 274 274 + Read-only handlers can stay direct. 201 275 202 276 ### C firmware artefacts 203 277 ··· 484 558 485 559 --- 486 560 487 487 - ## Known pitfalls (also in `MEMORY.md`) 561 561 + ## Known pitfalls 488 562 489 563 | Symptom | Cause | Fix | 490 564 |---|---|---| ··· 498 572 | Library DB stays empty even after browsing works | Embedded daemon doesn't run the desktop CLI's startup scan | `daemon.rs::spawn_library_scan` runs after gRPC binds; force re-scan via `RockboxClient.rescanLibrary()` | 499 573 | `Permission denied` reading `/storage/emulated/0/Music` on API 33+ | `READ_EXTERNAL_STORAGE` is ignored on `targetSdk=33+`; `READ_MEDIA_AUDIO` only grants MediaStore queries | `MANAGE_EXTERNAL_STORAGE` in manifest + `useAllFilesAccessPrompt()` opens system Settings | 500 574 | Daemon dies after the app backgrounds for a few minutes | App process killed for memory; daemon dies with it | NowPlayingService is a foreground service — keep it running via `RockboxNowPlaying.start()` at app launch (called from `_layout.tsx`) | 575 575 + | SIGSEGV at PC=0 in `wakeup_thread_` / `queue_send` on track switches, settings updates, anything that crosses `audio_thread` ↔ `codec_thread` | Rockbox kernel uses `__cores[0].running` as global "current thread" — no TLS. Calling firmware FFI from a non-Rockbox pthread (actix worker handling a gRPC request) corrupts kernel-thread state. Same root cause as the older "stale blocker" / "pcmbuf race" symptoms — they were all surfaces of this | **Firmware-command bus** in `crates/server/src/fw_bus.rs`. Every kernel-mutating handler in `crates/server/src/handlers/{player,playlists,saved_playlists,smart_playlists,settings}.rs` wraps its FFI block in `crate::fw_bus::run_on_broker(move \|\| …)` so the calls run on the broker (a real Rockbox kernel thread) and `__running_self_entry()` resolves correctly. Read-only handlers stay direct | 576 576 + | pcmbuf rebuild race (separate, narrower window) | `pcmbuf_init` rewrites the chunk descriptor ring while the AAudio writer pthread may be mid-`pcmbuf_pcm_callback` | `apps/pcmbuf.c::pcmbuf_init` wrapped in `pcm_play_lock()` / `pcm_play_unlock()` — routes to our `aa_mtx` and blocks `aa_thread` for the few ms of rebuild. Kept as defence-in-depth on top of the bus fix | 577 577 + | Track-switch hiccup race (separate) | `codec_stop` from `halt_decoding_track` could race the codec_thread before it parked | `apps/playback.c::halt_decoding_track` does `sleep(HZ/10)` (CODECS_STATIC only) before `codec_stop()` to let the runqueue drain. Kept as defence-in-depth | 578 578 + | Probe / cache writes fail with `ENOENT /tmp/...` on Android | App sandbox has no writable `/tmp` | `daemon.rs::configure_environment` sets `TMPDIR=$HOME/tmp` (and `mkdir -p`s it) so `std::env::temp_dir()` resolves into the sandbox | 501 579 502 580 --- 503 581

+12

crates/expo/src/daemon.rs

reviewed

··· 161 161 // resolves to /data/.../files/Music (doesn't exist) → ENOENT. 162 162 std::env::set_var("ROCKBOX_LIBRARY", music_dir); 163 163 164 164 + // Redirect anyone calling `std::env::temp_dir()` (e.g. the HTTP-stream 165 165 + // metadata probe in crates/library that writes 166 166 + // `rockbox-remote-probe-<md5>.<ext>` files) into the app sandbox. 167 167 + // Stdlib's temp_dir() honours $TMPDIR before falling back to /tmp, 168 168 + // and /tmp doesn't exist (or isn't writable) for non-root Android 169 169 + // app processes. The dir lives under HOME so it's persistent app 170 170 + // storage — fine for our short-lived probes that are removed via 171 171 + // RemoteProbeFile's Drop impl. 172 172 + let tmp = format!("{}/tmp", config_dir); 173 173 + let _ = std::fs::create_dir_all(&tmp); 174 174 + std::env::set_var("TMPDIR", &tmp); 175 175 + 164 176 // mDNS-advertised LAN ports (match crates/discovery defaults). 165 177 if std::env::var_os("ROCKBOX_PORT").is_none() { 166 178 std::env::set_var("ROCKBOX_PORT", "6061");

+209

crates/server/src/fw_bus.rs

reviewed

··· 1 1 + //! Firmware-command bus. 2 2 + //! 3 3 + //! On the Android cdylib (and any hosted-pthread build), Rockbox kernel 4 4 + //! state is identified via the global `__cores[0].running` slot — there is 5 5 + //! no thread-local-storage involved. Calling firmware kernel functions 6 6 + //! (`audio_play`, `audio_set_crossfade`, `pcmbuf_*`, anything that 7 7 + //! eventually reaches `queue_send` / `wakeup_thread`) from a non-Rockbox 8 8 + //! pthread (e.g. an actix worker handling a gRPC request) reads/writes 9 9 + //! the wrong thread_entry and corrupts the kernel scheduler. Symptoms: 10 10 + //! SIGSEGV at PC=0 in `wakeup_thread_` on track switches or settings 11 11 + //! changes, intermittent kernel coroutine corruption. 12 12 + //! 13 13 + //! Solution: serialise every kernel-affecting call through a single 14 14 + //! mpsc channel that the **broker** thread drains. The broker is a real 15 15 + //! Rockbox kernel thread (created via `create_thread` from 16 16 + //! `apps/broker_thread.c`), so its calls into firmware run with a sane 17 17 + //! `__running_self_entry()`. 18 18 + //! 19 19 + //! Synchronous handlers wait on a oneshot reply so they keep their 20 20 + //! original signatures from the caller's POV. 21 21 + 22 22 + use std::sync::mpsc::{self, Receiver, Sender, TryRecvError}; 23 23 + use std::sync::Mutex; 24 24 + use std::sync::OnceLock; 25 25 + 26 26 + /// Discriminated union of every firmware-mutating call we need to route 27 27 + /// through the broker. Add a variant when a new handler proves to need it. 28 28 + pub enum FwCmd { 29 29 + Play { 30 30 + elapsed: i64, 31 31 + offset: i64, 32 32 + reply: Option<mpsc::SyncSender<()>>, 33 33 + }, 34 34 + Pause { 35 35 + reply: Option<mpsc::SyncSender<()>>, 36 36 + }, 37 37 + Resume { 38 38 + reply: Option<mpsc::SyncSender<()>>, 39 39 + }, 40 40 + Next { 41 41 + reply: Option<mpsc::SyncSender<()>>, 42 42 + }, 43 43 + Prev { 44 44 + reply: Option<mpsc::SyncSender<()>>, 45 45 + }, 46 46 + Stop { 47 47 + reply: Option<mpsc::SyncSender<()>>, 48 48 + }, 49 49 + FfRewind { 50 50 + newtime: i32, 51 51 + reply: Option<mpsc::SyncSender<()>>, 52 52 + }, 53 53 + FlushAndReloadTracks { 54 54 + reply: Option<mpsc::SyncSender<()>>, 55 55 + }, 56 56 + SetCrossfade { 57 57 + value: i32, 58 58 + reply: Option<mpsc::SyncSender<()>>, 59 59 + }, 60 60 + /// Escape hatch for anything not covered yet. Closure runs on the 61 61 + /// broker thread; sender is responsible for not panicking inside. 62 62 + Custom(Box<dyn FnOnce() + Send + 'static>), 63 63 + } 64 64 + 65 65 + static SENDER: OnceLock<Sender<FwCmd>> = OnceLock::new(); 66 66 + // Receiver is wrapped in a Mutex<Option> so the broker can `take()` it on 67 67 + // startup; only one broker should ever exist. 68 68 + static RECEIVER: OnceLock<Mutex<Option<Receiver<FwCmd>>>> = OnceLock::new(); 69 69 + 70 70 + /// Initialise the channel. Call once at startup, BEFORE the broker thread 71 71 + /// is spawned and BEFORE any handler tries to `send()`. Idempotent. 72 72 + pub fn init() { 73 73 + SENDER.get_or_init(|| { 74 74 + let (tx, rx) = mpsc::channel(); 75 75 + RECEIVER 76 76 + .set(Mutex::new(Some(rx))) 77 77 + .unwrap_or_else(|_| panic!("fw_bus::init called twice")); 78 78 + tx 79 79 + }); 80 80 + } 81 81 + 82 82 + /// The broker thread takes ownership of the receiver on its first iteration. 83 83 + /// Returns None if init() wasn't called or the receiver was already taken. 84 84 + pub fn take_receiver() -> Option<Receiver<FwCmd>> { 85 85 + RECEIVER.get()?.lock().ok()?.take() 86 86 + } 87 87 + 88 88 + /// Enqueue a command for the broker to execute on its next tick. Drops 89 89 + /// silently if the bus isn't initialised (e.g. desktop / non-cdylib build); 90 90 + /// callers should treat it as a fire-and-forget side effect. 91 91 + pub fn send(cmd: FwCmd) { 92 92 + if let Some(tx) = SENDER.get() { 93 93 + let _ = tx.send(cmd); 94 94 + } 95 95 + } 96 96 + 97 97 + /// Send a command and block until the broker confirms execution. Use 98 98 + /// from actix `web::block(...)` callbacks — the wait is bounded by the 99 99 + /// broker tick (~10 ms idle, immediate when busy). 100 100 + pub fn send_and_wait(make: impl FnOnce(mpsc::SyncSender<()>) -> FwCmd) { 101 101 + let (reply_tx, reply_rx) = mpsc::sync_channel::<()>(1); 102 102 + send(make(reply_tx)); 103 103 + // 5-second cap — if the broker is wedged we'd rather return than block 104 104 + // a request thread forever. 105 105 + let _ = reply_rx.recv_timeout(std::time::Duration::from_secs(5)); 106 106 + } 107 107 + 108 108 + /// Run an arbitrary closure on the broker thread and block until it 109 109 + /// finishes. Returns the closure's value. Typical use from a handler: 110 110 + /// 111 111 + /// let ret: i32 = fw_bus::run_on_broker(|| rb::playlist::shuffle(seed, idx)); 112 112 + /// 113 113 + /// The closure runs on a real Rockbox kernel thread, so any firmware 114 114 + /// FFI it makes resolves `__cores[0].running` correctly. Blocks the 115 115 + /// caller for up to 5 s; intended to be called from `web::block(...)`. 116 116 + pub fn run_on_broker<T, F>(f: F) -> T 117 117 + where 118 118 + T: Send + 'static, 119 119 + F: FnOnce() -> T + Send + 'static, 120 120 + { 121 121 + let (tx, rx) = mpsc::sync_channel::<T>(1); 122 122 + send(FwCmd::Custom(Box::new(move || { 123 123 + let _ = tx.send(f()); 124 124 + }))); 125 125 + rx.recv_timeout(std::time::Duration::from_secs(5)) 126 126 + .unwrap_or_else(|_| { 127 127 + tracing::error!("fw_bus::run_on_broker: timed out waiting for broker"); 128 128 + // Last resort — return a default. The caller's signature 129 129 + // requires us to produce a T, but T might not be Default. 130 130 + // Panic instead so the actix worker bubbles 500 to the client. 131 131 + panic!("fw_bus::run_on_broker: broker tick timed out (5 s)"); 132 132 + }) 133 133 + } 134 134 + 135 135 + /// Drain the channel and execute everything pending. Called once per 136 136 + /// broker iteration. Non-blocking — returns when the queue is empty. 137 137 + /// Must be called from the broker thread (a real Rockbox kernel thread). 138 138 + pub fn drain(rx: &Receiver<FwCmd>) { 139 139 + use rockbox_sys as rb; 140 140 + loop { 141 141 + match rx.try_recv() { 142 142 + Ok(cmd) => { 143 143 + execute_on_broker(cmd, &|| true).unwrap_or_else(|e| { 144 144 + tracing::warn!("fw_bus: broker exec failed: {e}"); 145 145 + }); 146 146 + } 147 147 + Err(TryRecvError::Empty) => return, 148 148 + Err(TryRecvError::Disconnected) => { 149 149 + tracing::error!("fw_bus: sender disconnected — bus shutdown"); 150 150 + return; 151 151 + } 152 152 + } 153 153 + } 154 154 + } 155 155 + 156 156 + fn execute_on_broker(cmd: FwCmd, _alive: &dyn Fn() -> bool) -> Result<(), &'static str> { 157 157 + use rockbox_sys as rb; 158 158 + macro_rules! reply { 159 159 + ($r:expr) => { 160 160 + if let Some(r) = $r { 161 161 + let _ = r.send(()); 162 162 + } 163 163 + }; 164 164 + } 165 165 + match cmd { 166 166 + FwCmd::Play { 167 167 + elapsed, 168 168 + offset, 169 169 + reply, 170 170 + } => { 171 171 + rb::playback::play(elapsed, offset); 172 172 + reply!(reply); 173 173 + } 174 174 + FwCmd::Pause { reply } => { 175 175 + rb::playback::pause(); 176 176 + reply!(reply); 177 177 + } 178 178 + FwCmd::Resume { reply } => { 179 179 + rb::playback::resume(); 180 180 + reply!(reply); 181 181 + } 182 182 + FwCmd::Next { reply } => { 183 183 + rb::playback::next(); 184 184 + reply!(reply); 185 185 + } 186 186 + FwCmd::Prev { reply } => { 187 187 + rb::playback::prev(); 188 188 + reply!(reply); 189 189 + } 190 190 + FwCmd::Stop { reply } => { 191 191 + rb::playback::hard_stop(); 192 192 + reply!(reply); 193 193 + } 194 194 + FwCmd::FfRewind { newtime, reply } => { 195 195 + rb::playback::ff_rewind(newtime); 196 196 + reply!(reply); 197 197 + } 198 198 + FwCmd::FlushAndReloadTracks { reply } => { 199 199 + rb::playback::flush_and_reload_tracks(); 200 200 + reply!(reply); 201 201 + } 202 202 + FwCmd::SetCrossfade { value, reply } => { 203 203 + rb::sound::audio_set_crossfade_safe(value); 204 204 + reply!(reply); 205 205 + } 206 206 + FwCmd::Custom(f) => f(), 207 207 + } 208 208 + Ok(()) 209 209 + }

+26 -8

crates/server/src/handlers/player.rs

reviewed

··· 131 131 let elapsed = query.elapsed.unwrap_or(0); 132 132 let offset = query.offset.unwrap_or(0); 133 133 134 134 + // Route through fw_bus so audio_play() runs on the broker (a real 135 135 + // Rockbox kernel thread), not on the actix worker. Calling firmware 136 136 + // kernel functions from a non-Rockbox pthread corrupts the global 137 137 + // `__cores[0].running` slot and crashes the scheduler — see 138 138 + // crates/server/src/fw_bus.rs. 134 139 web::block(move || { 135 140 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 136 141 if state.player.lock().unwrap().is_none() { 137 137 - rb::playback::play(elapsed, offset); 142 142 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Play { 143 143 + elapsed, 144 144 + offset, 145 145 + reply: Some(reply), 146 146 + }); 138 147 } 139 148 }) 140 149 .await ··· 154 163 } else { 155 164 web::block(move || { 156 165 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 157 157 - rb::playback::pause(); 166 166 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Pause { 167 167 + reply: Some(reply), 168 168 + }); 158 169 }) 159 170 .await 160 171 .map_err(ErrorInternalServerError)?; ··· 172 183 let newtime = query.newtime.unwrap_or(0); 173 184 web::block(move || { 174 185 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 175 175 - rb::playback::ff_rewind(newtime); 186 186 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::FfRewind { 187 187 + newtime, 188 188 + reply: Some(reply), 189 189 + }); 176 190 }) 177 191 .await 178 192 .map_err(ErrorInternalServerError)?; ··· 325 339 pub async fn flush_and_reload_tracks() -> HandlerResult { 326 340 web::block(|| { 327 341 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 328 328 - rb::playback::flush_and_reload_tracks(); 342 342 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::FlushAndReloadTracks { 343 343 + reply: Some(reply), 344 344 + }); 329 345 }) 330 346 .await 331 347 .map_err(ErrorInternalServerError)?; ··· 343 359 } else { 344 360 web::block(|| { 345 361 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 346 346 - rb::playback::resume(); 362 362 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Resume { 363 363 + reply: Some(reply), 364 364 + }); 347 365 }) 348 366 .await 349 367 .map_err(ErrorInternalServerError)?; ··· 355 373 pub async fn next() -> HandlerResult { 356 374 web::block(|| { 357 375 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 358 358 - rb::playback::next(); 376 376 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Next { reply: Some(reply) }); 359 377 }) 360 378 .await 361 379 .map_err(ErrorInternalServerError)?; ··· 365 383 pub async fn previous() -> HandlerResult { 366 384 web::block(|| { 367 385 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 368 368 - rb::playback::prev(); 386 386 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Prev { reply: Some(reply) }); 369 387 }) 370 388 .await 371 389 .map_err(ErrorInternalServerError)?; ··· 375 393 pub async fn stop() -> HandlerResult { 376 394 web::block(|| { 377 395 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 378 378 - rb::playback::hard_stop(); 396 396 + crate::fw_bus::send_and_wait(|reply| crate::fw_bus::FwCmd::Stop { reply: Some(reply) }); 379 397 }) 380 398 .await 381 399 .map_err(ErrorInternalServerError)?;

+113 -95

crates/server/src/handlers/playlists.rs

reviewed

··· 104 104 105 105 let start_index = web::block(move || { 106 106 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 107 107 - 108 108 - let current_is_http = rb::playback::current_track() 109 109 - .map(|t| t.path.starts_with("http://") || t.path.starts_with("https://")) 110 110 - .unwrap_or(false); 111 111 - let new_is_http = new_playlist.tracks[0].starts_with("http://") 112 112 - || new_playlist.tracks[0].starts_with("https://"); 113 113 - if current_is_http || new_is_http { 114 114 - rb::playback::hard_stop(); 115 115 - } 107 107 + // hard_stop / playlist_create / build_playlist all touch 108 108 + // firmware kernel state — route through the broker so they run 109 109 + // with the right __cores[0].running entry. 110 110 + crate::fw_bus::run_on_broker(move || { 111 111 + let current_is_http = rb::playback::current_track() 112 112 + .map(|t| t.path.starts_with("http://") || t.path.starts_with("https://")) 113 113 + .unwrap_or(false); 114 114 + let new_is_http = new_playlist.tracks[0].starts_with("http://") 115 115 + || new_playlist.tracks[0].starts_with("https://"); 116 116 + if current_is_http || new_is_http { 117 117 + rb::playback::hard_stop(); 118 118 + } 116 119 117 117 - let first = &new_playlist.tracks[0]; 118 118 - let dir = if first.starts_with("http://") || first.starts_with("https://") { 119 119 - std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()) 120 120 - } else { 121 121 - let parts: Vec<_> = first.split('/').collect(); 122 122 - parts[..parts.len().saturating_sub(1)].join("/") 123 123 - }; 124 124 - rb::playlist::create(&dir, None); 120 120 + let first = &new_playlist.tracks[0]; 121 121 + let dir = if first.starts_with("http://") || first.starts_with("https://") { 122 122 + std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()) 123 123 + } else { 124 124 + let parts: Vec<_> = first.split('/').collect(); 125 125 + parts[..parts.len().saturating_sub(1)].join("/") 126 126 + }; 127 127 + rb::playlist::create(&dir, None); 125 128 126 126 - let start_index = rb::playlist::build_playlist( 127 127 - new_playlist.tracks.iter().map(|t| t.as_str()).collect(), 128 128 - 0, 129 129 - new_playlist.tracks.len() as i32, 130 130 - ); 131 131 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 132 132 - start_index 129 129 + let start_index = rb::playlist::build_playlist( 130 130 + new_playlist.tracks.iter().map(|t| t.as_str()).collect(), 131 131 + 0, 132 132 + new_playlist.tracks.len() as i32, 133 133 + ); 134 134 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 135 135 + start_index 136 136 + }) 133 137 }) 134 138 .await 135 139 .map_err(ErrorInternalServerError)?; ··· 149 153 let offset = query.offset.unwrap_or(0); 150 154 web::block(move || { 151 155 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 152 152 - rb::playlist::start(start_index, elapsed, offset); 156 156 + // playlist_start() inside firmware sends Q_AUDIO_PLAY to audio_thread 157 157 + // → halt_decoding_track → codec_stop → kernel scheduler. Must run on 158 158 + // the broker (real kernel thread) — see crates/server/src/fw_bus.rs. 159 159 + crate::fw_bus::run_on_broker(move || { 160 160 + rb::playlist::start(start_index, elapsed, offset); 161 161 + }); 153 162 PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 154 163 }) 155 164 .await ··· 166 175 let start_index = query.start_index.unwrap_or(0); 167 176 let ret = web::block(move || { 168 177 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 169 169 - let seed = rb::system::current_tick(); 170 170 - let ret = rb::playlist::shuffle(seed as i32, start_index); 171 171 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 172 172 - ret 178 178 + crate::fw_bus::run_on_broker(move || { 179 179 + let seed = rb::system::current_tick(); 180 180 + let ret = rb::playlist::shuffle(seed as i32, start_index); 181 181 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 182 182 + ret 183 183 + }) 173 184 }) 174 185 .await 175 186 .map_err(ErrorInternalServerError)?; ··· 189 200 pub async fn resume_playlist() -> HandlerResult { 190 201 let code = web::block(|| { 191 202 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 192 192 - let status = rb::system::get_global_status(); 193 193 - let playback_status = rb::playback::status(); 194 194 - if status.resume_index == -1 || playback_status.status == 1 { 195 195 - return -1; 196 196 - } 197 197 - let code = rb::playlist::resume(); 198 198 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 199 199 - code 203 203 + crate::fw_bus::run_on_broker(|| { 204 204 + let status = rb::system::get_global_status(); 205 205 + let playback_status = rb::playback::status(); 206 206 + if status.resume_index == -1 || playback_status.status == 1 { 207 207 + return -1; 208 208 + } 209 209 + let code = rb::playlist::resume(); 210 210 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 211 211 + code 212 212 + }) 200 213 }) 201 214 .await 202 215 .map_err(ErrorInternalServerError)?; ··· 206 219 pub async fn resume_track() -> HandlerResult { 207 220 web::block(|| { 208 221 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 209 209 - let status = rb::system::get_global_status(); 210 210 - if status.resume_index == -1 { 211 211 - return; 212 212 - } 213 213 - if rb::playlist::amount() == 0 { 214 214 - let ret = rb::playlist::resume(); 215 215 - if ret == -1 { 222 222 + crate::fw_bus::run_on_broker(|| { 223 223 + let status = rb::system::get_global_status(); 224 224 + if status.resume_index == -1 { 216 225 return; 217 226 } 218 218 - } 219 219 - rb::playlist::resume_track( 220 220 - status.resume_index, 221 221 - status.resume_crc32, 222 222 - status.resume_elapsed.into(), 223 223 - status.resume_offset.into(), 224 224 - ); 225 225 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 227 227 + if rb::playlist::amount() == 0 { 228 228 + let ret = rb::playlist::resume(); 229 229 + if ret == -1 { 230 230 + return; 231 231 + } 232 232 + } 233 233 + rb::playlist::resume_track( 234 234 + status.resume_index, 235 235 + status.resume_crc32, 236 236 + status.resume_elapsed.into(), 237 237 + status.resume_offset.into(), 238 238 + ); 239 239 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 240 240 + }); 226 241 }) 227 242 .await 228 243 .map_err(ErrorInternalServerError)?; ··· 348 363 } 349 364 } 350 365 351 351 - // Built-in player: all C FFI calls must run on a blocking thread so that 352 352 - // reqwest::blocking (used by rb_net_open for HTTP tracks) does not create a 353 353 - // nested tokio context on the actix worker thread. 366 366 + // Built-in player: all firmware kernel calls must run on the broker 367 367 + // thread (real Rockbox kernel thread) — see crates/server/src/fw_bus.rs. 354 368 let response_body = web::block(move || -> Result<String, String> { 355 369 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 356 356 - let amount = rb::playlist::amount(); 370 370 + crate::fw_bus::run_on_broker(move || { 371 371 + let amount = rb::playlist::amount(); 372 372 + 373 373 + if amount == 0 { 374 374 + let first = &tracklist.tracks[0]; 375 375 + let dir = if first.starts_with("http://") || first.starts_with("https://") { 376 376 + "/".to_string() 377 377 + } else { 378 378 + let dir_parts: Vec<_> = first.split('/').collect(); 379 379 + dir_parts[0..dir_parts.len() - 1].join("/") 380 380 + }; 381 381 + let status = rb::playlist::create(&dir, None); 382 382 + if status == -1 { 383 383 + return Err("Failed to create playlist".to_string()); 384 384 + } 385 385 + let start_index = rb::playlist::build_playlist( 386 386 + tracklist.tracks.iter().map(|t| t.as_str()).collect(), 387 387 + 0, 388 388 + tracklist.tracks.len() as i32, 389 389 + ); 390 390 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 391 391 + return Ok(start_index.to_string()); 392 392 + } 357 393 358 358 - if amount == 0 { 359 359 - let first = &tracklist.tracks[0]; 360 360 - let dir = if first.starts_with("http://") || first.starts_with("https://") { 361 361 - "/".to_string() 362 362 - } else { 363 363 - let dir_parts: Vec<_> = first.split('/').collect(); 364 364 - dir_parts[0..dir_parts.len() - 1].join("/") 394 394 + let mut tracks: Vec<&str> = tracklist.tracks.iter().map(|t| t.as_str()).collect(); 395 395 + let position = match tracklist.position { 396 396 + PLAYLIST_INSERT_LAST_SHUFFLED => { 397 397 + tracks.shuffle(&mut rand::thread_rng()); 398 398 + PLAYLIST_INSERT_LAST 399 399 + } 400 400 + _ => tracklist.position, 365 401 }; 366 366 - let status = rb::playlist::create(&dir, None); 367 367 - if status == -1 { 368 368 - return Err("Failed to create playlist".to_string()); 369 369 - } 370 370 - let start_index = rb::playlist::build_playlist( 371 371 - tracklist.tracks.iter().map(|t| t.as_str()).collect(), 372 372 - 0, 373 373 - tracklist.tracks.len() as i32, 374 374 - ); 402 402 + rb::playlist::insert_tracks(tracks, position, tracklist.tracks.len() as i32); 375 403 PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 376 376 - return Ok(start_index.to_string()); 377 377 - } 378 378 - 379 379 - let mut tracks: Vec<&str> = tracklist.tracks.iter().map(|t| t.as_str()).collect(); 380 380 - let position = match tracklist.position { 381 381 - PLAYLIST_INSERT_LAST_SHUFFLED => { 382 382 - tracks.shuffle(&mut rand::thread_rng()); 383 383 - PLAYLIST_INSERT_LAST 384 384 - } 385 385 - _ => tracklist.position, 386 386 - }; 387 387 - rb::playlist::insert_tracks(tracks, position, tracklist.tracks.len() as i32); 388 388 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 389 389 - Ok(tracklist.position.to_string()) 404 404 + Ok(tracklist.position.to_string()) 405 405 + }) 390 406 }) 391 407 .await 392 408 .map_err(ErrorInternalServerError)? ··· 410 426 } 411 427 drop(player); 412 428 413 413 - let mut ret = 0; 429 429 + crate::fw_bus::run_on_broker(move || { 430 430 + let mut ret = 0; 414 431 415 415 - for position in &params.positions { 416 416 - ret = rb::playlist::delete_track(*position); 417 417 - } 432 432 + for position in &params.positions { 433 433 + ret = rb::playlist::delete_track(*position); 434 434 + } 418 435 419 419 - if params.positions.is_empty() { 420 420 - ret = rb::playlist::remove_all_tracks(); 421 421 - } 436 436 + if params.positions.is_empty() { 437 437 + ret = rb::playlist::remove_all_tracks(); 438 438 + } 422 439 423 423 - PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 424 424 - ret 440 440 + PLAYLIST_DIRTY.store(true, Ordering::Relaxed); 441 441 + ret 442 442 + }) 425 443 }) 426 444 .await 427 445 .map_err(ErrorInternalServerError)?;

+22 -13

crates/server/src/handlers/saved_playlists.rs

reviewed

··· 237 237 return Ok(HttpResponse::UnprocessableEntity().finish()); 238 238 } 239 239 240 240 - let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 241 241 - let first = &paths[0]; 242 242 - let dir = { 243 243 - let parts: Vec<_> = first.split('/').collect(); 244 244 - parts[..parts.len().saturating_sub(1)].join("/") 245 245 - }; 246 246 - rb::playlist::create(&dir, None); 247 247 - rb::playlist::build_playlist( 248 248 - paths.iter().map(|p| p.as_str()).collect(), 249 249 - 0, 250 250 - paths.len() as i32, 251 251 - ); 252 252 - rb::playlist::start(0, 0, 0); 240 240 + web::block(move || { 241 241 + let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 242 242 + // Build + start a saved playlist — `playlist_start` triggers 243 243 + // halt_decoding_track / codec_stop in the kernel scheduler. 244 244 + // Must run on the broker (real Rockbox kernel thread). 245 245 + crate::fw_bus::run_on_broker(move || { 246 246 + let first = &paths[0]; 247 247 + let dir = { 248 248 + let parts: Vec<_> = first.split('/').collect(); 249 249 + parts[..parts.len().saturating_sub(1)].join("/") 250 250 + }; 251 251 + rb::playlist::create(&dir, None); 252 252 + rb::playlist::build_playlist( 253 253 + paths.iter().map(|p| p.as_str()).collect(), 254 254 + 0, 255 255 + paths.len() as i32, 256 256 + ); 257 257 + rb::playlist::start(0, 0, 0); 258 258 + }); 259 259 + }) 260 260 + .await 261 261 + .map_err(ErrorInternalServerError)?; 253 262 254 263 Ok(HttpResponse::NoContent().finish()) 255 264 }

+18 -3

crates/server/src/handlers/settings.rs

reviewed

··· 18 18 19 19 pub async fn update_global_settings(body: web::Json<NewGlobalSettings>) -> HandlerResult { 20 20 let settings = body.into_inner(); 21 21 + // Route the settings apply through the firmware-command bus — most 22 22 + // settings just write a value, but `crossfade` calls 23 23 + // `audio_set_crossfade()` which queues `Q_AUDIO_REMAKE_AUDIO_BUFFER` 24 24 + // and ends up in the kernel scheduler. From an actix worker the 25 25 + // scheduler reads the wrong `__cores[0].running` slot and corrupts 26 26 + // kernel-thread state — see crates/server/src/fw_bus.rs. Run the 27 27 + // whole load_settings on the broker (a real Rockbox kernel thread) 28 28 + // so the FFI calls resolve to a sane current-thread. 21 29 web::block(move || { 22 30 let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 23 23 - rockbox_settings::load_settings(Some(settings))?; 24 24 - rockbox_settings::write_settings() 31 31 + crate::fw_bus::send_and_wait(|reply| { 32 32 + crate::fw_bus::FwCmd::Custom(Box::new(move || { 33 33 + if let Err(e) = rockbox_settings::load_settings(Some(settings)) { 34 34 + tracing::error!("update_global_settings: load_settings failed: {e}"); 35 35 + } else if let Err(e) = rockbox_settings::write_settings() { 36 36 + tracing::error!("update_global_settings: write_settings failed: {e}"); 37 37 + } 38 38 + let _ = reply.send(()); 39 39 + })) 40 40 + }); 25 41 }) 26 42 .await 27 27 - .map_err(ErrorInternalServerError)? 28 43 .map_err(ErrorInternalServerError)?; 29 44 Ok(HttpResponse::NoContent().finish()) 30 45 }

+21 -13

crates/server/src/handlers/smart_playlists.rs

reviewed

··· 209 209 } 210 210 211 211 let paths: Vec<String> = tracks.iter().map(|t| t.path.clone()).collect(); 212 212 - let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 213 213 - let first = &paths[0]; 214 214 - let dir = { 215 215 - let parts: Vec<_> = first.split('/').collect(); 216 216 - parts[..parts.len().saturating_sub(1)].join("/") 217 217 - }; 218 218 - rb::playlist::create(&dir, None); 219 219 - rb::playlist::build_playlist( 220 220 - paths.iter().map(|p| p.as_str()).collect(), 221 221 - 0, 222 222 - paths.len() as i32, 223 223 - ); 224 224 - rb::playlist::start(0, 0, 0); 212 212 + web::block(move || { 213 213 + let _player_mutex = PLAYER_MUTEX.lock().unwrap(); 214 214 + // Same broker routing as saved_playlists::play_smart_playlist — 215 215 + // playlist_start hits the kernel scheduler. 216 216 + crate::fw_bus::run_on_broker(move || { 217 217 + let first = &paths[0]; 218 218 + let dir = { 219 219 + let parts: Vec<_> = first.split('/').collect(); 220 220 + parts[..parts.len().saturating_sub(1)].join("/") 221 221 + }; 222 222 + rb::playlist::create(&dir, None); 223 223 + rb::playlist::build_playlist( 224 224 + paths.iter().map(|p| p.as_str()).collect(), 225 225 + 0, 226 226 + paths.len() as i32, 227 227 + ); 228 228 + rb::playlist::start(0, 0, 0); 229 229 + }); 230 230 + }) 231 231 + .await 232 232 + .map_err(ErrorInternalServerError)?; 225 233 226 234 Ok(HttpResponse::NoContent().finish()) 227 235 }

+19

crates/server/src/lib.rs

reviewed

··· 26 26 pub(crate) static PLAYLIST_DIRTY: AtomicBool = AtomicBool::new(false); 27 27 28 28 pub mod cache; 29 29 + pub mod fw_bus; 29 30 pub mod handlers; 30 31 pub mod http; 31 32 pub mod kv; ··· 526 527 527 528 #[no_mangle] 528 529 pub extern "C" fn start_servers() { 530 530 + // Set up the firmware-command bus before any HTTP/gRPC handler can 531 531 + // accept a request — handlers will enqueue here and the broker thread 532 532 + // (a real Rockbox kernel thread) will execute synchronously on its 533 533 + // own pthread context. See crates/server/src/fw_bus.rs. 534 534 + fw_bus::init(); 535 535 + 529 536 let (cmd_tx, cmd_rx) = std::sync::mpsc::channel::<RockboxCommand>(); 530 537 let cmd_tx = Arc::new(Mutex::new(cmd_tx)); 531 538 ··· 679 686 let mut last_stats_elapsed: u64 = 0; 680 687 let mut last_stats_length: u64 = 0; 681 688 689 689 + // Take ownership of the firmware-command bus receiver. We're a real 690 690 + // Rockbox kernel thread (created via apps/broker_thread.c::create_thread), 691 691 + // so any FFI we run from drain() resolves __running_self_entry() to 692 692 + // OUR thread_entry — safe to mutate kernel state. 693 693 + let fw_rx = fw_bus::take_receiver(); 694 694 + 682 695 loop { 696 696 + // Drain the bus first so handler-issued commands run with minimal 697 697 + // latency (~ broker tick period, ~10 ms idle). 698 698 + if let Some(rx) = fw_rx.as_ref() { 699 699 + fw_bus::drain(rx); 700 700 + } 701 701 + 683 702 let mutex = GLOBAL_MUTEX.lock().unwrap(); 684 703 if *mutex == 1 { 685 704 drop(mutex);

+40

crates/sys/src/sound/mod.rs

reviewed

··· 76 76 unsafe { crate::keyclick_click(rawbutton, action) } 77 77 } 78 78 79 79 + /// Stock crossfade setter — drives the firmware directly. **Don't call 80 80 + /// while audio is actively playing**: the firmware reconfigures the PCM 81 81 + /// buffer ring (pcmbuf_init / audio_remake_audio_buffers), and on hosted 82 82 + /// pthread targets (Android cdylib's pcm-aaudio.c) the writer pthread 83 83 + /// runs asynchronously and races into half-rebuilt state → SIGSEGV at 84 84 + /// PC=0 inside pcmbuf_pcm_callback. Use [`audio_set_crossfade_safe`] for 85 85 + /// any caller that might fire mid-playback. 79 86 pub fn audio_set_crossfade(crossfade: i32) { 80 87 unsafe { crate::audio_set_crossfade(crossfade) } 81 88 } 89 89 + 90 90 + /// Crossfade setter that is safe to call any time, including 91 91 + /// mid-playback. If the engine has a track loaded (`AUDIO_STATUS_PLAY`), 92 92 + /// the change is bracketed by `audio_pause()` / `audio_resume()` so the 93 93 + /// codec/audio threads quiesce before pcmbuf_init runs. Adds ~80 ms of 94 94 + /// audible interruption when toggled mid-track; no overhead when stopped. 95 95 + /// 96 96 + /// Same wrapper applies to any other setting that triggers an 97 97 + /// `audio_remake_audio_buffers` (replaygain mode, output sample rate, 98 98 + /// sound enable). Add a call-site wrapper for those too if needed. 99 99 + pub fn audio_set_crossfade_safe(crossfade: i32) { 100 100 + // AUDIO_STATUS_PLAY = 0x0001 (firmware/export/audio.h). Set whenever 101 101 + // a track is loaded — true for both actively-playing and paused 102 102 + // states. Both need the bracketing because aa_thread is still the 103 103 + // PCM consumer in both cases. 104 104 + const AUDIO_STATUS_PLAY: i32 = 0x0001; 105 105 + let was_loaded = (unsafe { crate::audio_status() } & AUDIO_STATUS_PLAY) != 0; 106 106 + 107 107 + if was_loaded { 108 108 + // Pause the engine — codec_thread parks, no new PCM is produced. 109 109 + unsafe { crate::audio_pause() }; 110 110 + // Give aa_thread a beat to drain any in-flight pcmbuf callback 111 111 + // and park on its pcm_data wait. 50 ms is comfortable; AAudio's 112 112 + // burst is typically a few ms. 113 113 + std::thread::sleep(std::time::Duration::from_millis(50)); 114 114 + } 115 115 + 116 116 + unsafe { crate::audio_set_crossfade(crossfade) }; 117 117 + 118 118 + if was_loaded { 119 119 + unsafe { crate::audio_resume() }; 120 120 + } 121 121 + }