FS#5691 - Fix for crash if stop pressed during voice output

Attached to Project: Rockbox
Opened by Steve Bavin (pondlife) - Friday, 21 July 2006, 20:31 GMT
Last edited by Linus Nielsen Feltzing (linusnielsen) - Monday, 24 July 2006, 08:15 GMT
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity High
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


If the user stops playback while a filename is still being spelt, Rockbox locks up. This seems to be because playback.c waits for a Q_VOICE_STOP event (to clear voice_is_playing), but this never occurs. Looking at the changelog, it seems that this loop was added to fix crashes when pressing stop in the tree view. A slightly simpler loop, just waiting for the voice queue to be empty, will do the job and doesn't loop forever.

This patch also performs a talk_id(VOICE_PAUSE, false) to cause voice output to be truncated faster so that filenames are not spelt out over the first 10 seconds of a track. It would be neater to add a talk_shutup() function to talk.h/talk.c and use this, but I didn't want to put any dependency on the other talk.c patch I've submitted.
This task depends upon

Closed by  Miika Pekkarinen (miipekk)
Saturday, 05 August 2006, 07:30 GMT
Reason for closing:  Accepted
Comment by Steve Bavin (pondlife) - Friday, 21 July 2006, 20:31 GMT
And here's the patch!
Comment by Steve Bavin (pondlife) - Monday, 24 July 2006, 08:37 GMT
It would be good if anyone knows whether Q_VOICE_STOP should be occuring in this case. If so,, a better fix would be to find out why it's not being generated (or handled).
Comment by Steve Bavin (pondlife) - Monday, 31 July 2006, 17:40 GMT
I've determined that there's no Q_VOICE_STOP being produced in this situation (still not sure if this is intentional).

Looking at the logic in playback.c, the loop:
while (voice_is_playing || !queue_empty(&voice_codec_queue)) ...
is resulting in an endless loop, as the queue remains empty and nothing is going to post a Q_VOICE_STOP in there.

Maybe this should read
while (voice_is_playing && !queue_empty(&voice_codec_queue)) ...
i.e. if we know the queue is empty, don't hang around waiting for a Q_VOICE_STOP?

This one-line mod is included in the updated patch attached here.