FS#8918 - Voice multiple thumbnails and talk race fixes
Opened by Stephane Doyon (sdoyon) - Sunday, 20 April 2008, 20:42 GMT
Last edited by Stephane Doyon (sdoyon) - Tuesday, 15 July 2008, 14:24 GMT
I've been stuck on this stuff for a while and I'd like to move
forward. This patch reverts a previous commit so a review is called for.
Starting a fresh task to reduce confusion. This supersedes
I need the ability to enqueue more than one .talk clip, aka thumbnail, at
a time. Needed for proper voicing of the bookmark selection menu, and
eventually for the playlist viewer and to speak full paths in the id3
Also I see some juggling of talk queue indicies in talk.c that don't look
thread safe and that are begging for a mutex.
When speaking a thumbnail, the current implementation just grabs the
thumbnail buffer and uses it, not caring whether it might have been in
use already by a previously queued thumbnail in the process of being
played out. It mostly works simply because we always shutup before
enqueuing a thumbnail.
Fixing this and allowing to enqueue multiple thumbnails requires to
properly track when the thunbnail buffer is used, and when we are done
with it, in particular when shutting up.
The first thing this patch does is to make the shutup operation
synchronous again. This means reversing r15872 by preglow: "Attempt at
fixing the statusbar showing up late in some screens and circumstances."
I do two things to try to mitigate the original problem (what I've been
able to find out about it anyway...):
-mp3_play_stop() does nothing until the audio thread is ready. This
should eliminate initial delays when initializing audio.
-the shutup operation is made a noop when no voice has been queued,
avoiding having to wait on the VOICE_STOP message being processed. In
particular it does nothing if voice is not being used.
Hopefully this solves the visual UI lag issues, but as a blind user I
can't say I've really confirmed this.
Having shutup synchronous simplifies things. At least in the case of
thumbnails, I need to be able to wait for shutup to have completed and
for the buffer to be freed. If shutup is not synchronous, I'll need to
setup event notification in talk.c to synchronize the threads. That can
be done, I had some code for it in
Given that there are still some stability and buffering issues with voice
and with pcm, such as B#8066 and the inconsistent pcm beep issue among
others, I would much prefer the shutup function to be deterministic and
simpler: it helps debugging in general to know that it has really stopped
when talk_shutup() returns.
This patch allows loading multiple thumbnails back-to-back in the one
thumbnail buffer. That buffer is currently 64KB, and that is usually
sufficient to accomodate two .talk clips. I keep track of when the
buffer is used and how much data is in it.
Finally, i put in a mutex to prevent race conditions in handling the talk
queue indices and the thumbnail buffer state.
Tuesday, 15 July 2008, 14:24 GMT
Reason for closing: Accepted