Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 104 Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /sites/rockbox.org/flyspray/adodb/adodb.inc.php on line 845 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.user.php on line 111 FS#9935 : audio_get_buffer() doesn't make the buffering code mark the buffer as invalid soon enough

Rockbox

Tasklist

FS#9935 - audio_get_buffer() doesn't make the buffering code mark the buffer as invalid soon enough

Attached to Project: Rockbox
Opened by Frank Gevaerts (fg) - Saturday, 21 February 2009, 15:06 GMT
Last edited by Frank Gevaerts (fg) - Wednesday, 25 February 2009, 22:52 GMT
Task Type Bugs
Category Music playback
Status Closed
Assigned To No-one
Operating System SW-codec
Severity Medium
Priority Normal
Reported Version Daily build (which?)
Due in Version Next release
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

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
This task depends upon

Closed by  Frank Gevaerts (fg)
Wednesday, 25 February 2009, 22:52 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed (to allow enabling USB)
Comment by Steve Bavin (pondlife) - Saturday, 21 February 2009, 16:18 GMT
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.
Comment by Nicolas Pennequin (nicolas_p) - Saturday, 21 February 2009, 17:37 GMT
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.
   audio_get_buffer.patch (0.5 KiB)

Loading...