Rockbox

  • Status Closed
  • Percent Complete
    100%
  • 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
Attached to Project: Rockbox
Opened by fg - 2009-02-21
Last edited by fg - 2009-02-25

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

Closed by  fg
2009-02-25 22:52
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Committed (to allow enabling USB)

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.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing