- Status Closed
- Percent Complete
- Task Type Bugs
- Category Music playback
- Assigned To No-one
- Operating System SW-codec
- Severity Medium
- Priority Very Low
- Reported Version Daily build (which?)
- Due in Version Next release
-
Due Date
Undecided
- Votes
- Private
FS#9935 - audio_get_buffer() doesn't make the buffering code mark the buffer as invalid soon enough
When enabling the “software” USB stack (-DUSE_ROCKBOX_USB), the buffering code (sometimes) crashes with a data abort in update_data_counters() on USB disconnect. This happens especially if the USB connection was established while playback was active.
The software USB Storage implementation uses audio_get_buffer() to get memory to use.
audio_get_buffer() is called *after* SYS_USB_CONNECTED, so the buffering thread is blocked in usb_wait_for_disconnect() (buffering.c:1385) at that time. On disconnect, the buffering thread leaves the switch block and unconditionally calls update_data_counters(), which assumes valid data in the buffer.
References : http://www.rockbox.org/irc/log-20090219#21:57:44 and http://forums.rockbox.org/index.php?topic=20664.0
2009-02-25 22:52
Reason for closing: Accepted
Additional comments about closing:
Committed (to allow enabling USB)
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
How about:
- move audio_get_buffer() from playback to buffering (and rename it) - it can then “invalidate” all open buffering handles
- ensure that all buffering functions are safe to call with an invalidated handle (i.e they return a null-block, or error) without crashing.
- make sure that there’s a standard way for the client to detect an invalidated handle (either a new function or as part of the previous point).
- (bonus point) send a system wide event when the buffer is grabbed so that playback (and any other users) can reset their state.
Here is a patch that I believe fixes the issue.
It simply adds a call to buffering_reset() in audio_get_buffer().
The bigger issue is that I’m not sure whether the audio_stop_playback call that happens on USB connect actually manages to close all the handles: if the buffering thread has acknowledged the USB connect before the audio thread, I guess the answer is no and I would even expect a deadlock.