Thread2

[Lucarda]

Improvements to the threading part

[Spacechild1]

There's still a problem with flite_do_textfile because it calls flite_synth, even if called from the worker thread.

I don't think it's necessary to call flite_synth in flite_do_textfile at all. Instead, the function should only read the text file. (Maybe you can find a better name, e.g. flite_read_textfile. After this function you can call either flite_synth or flite_thread_synth.

[Spacechild1]

Finally, please check all places where you set x_inprogress. You should only touch this variable on the main thread: set it to 1 in the Pd method and set it back to 0 in the clock tick function.

In fact, you only have to set x_inprogress=1 in asynchronous methods. E.g. there is no point in doing x_inprogress = 1 and x_inprogress = 0 in flite_synth() because it will execute synchronously on Pd's main thread.

[Spacechild1]

Finally, I would recommend to rearrange the code a bit: flite_synth and flite_thr_synth really belong together, while flite_thread_synth belongs to flite_thread. Some thing for other methods. Also, make sure that the code comments are still valid (there's nothing more confusing than outdated comments :-) (For example, the comment above flite_thrd_synth says call the thread to do flite_synth - which is not correct)

This might sound pedantic, but you will thank me when you have to read this code again in 1 year :-)

[Lucarda]

There's still a problem with flite_do_textfile because it calls flite_synth, even if called from the worker thread.

done

Finally, please check all places where you set x_inprogress. You should only touch this variable on the main thread: ...

done

Finally, I would recommend to rearrange the code a bit: flite_synth and flite_thr_synth really belong together, while flite_thread_synth belongs to flite_thread.

I fail to understand what this is about. You mean a better ordering of the functions?

[Spacechild1]

I fail to understand what this is about. You mean a better ordering of the functions?

yes

[Lucarda]

@Spacechild1 I think this is roughly ready to merge, bump to 0.3.2 and do a Deken release.

What do you think?

[Spacechild1]

This is really nitpicky, but why do you have the assignment on a seperate line? This should be simply const char *voxstring = vox->s_name

(I guess this comes from a misunderstanding about C. With C89 you had to declare any local variables at the top of the block, which often lead to a split between declaration and assignment. With C99 local variables can be declared anywhere.)

[Spacechild1]

you don't have to change it, it's more meant as a comment

[Spacechild1]

@Spacechild1 I think this is roughly ready to merge, bump to 0.3.2 and do a Deken release.

What do you think?

Looks good to me! I haven't really tested it in practice, but I guess you did :-)

[Lucarda]

Looks good to me! I haven't really tested it in practice, but I guess you did :-)

Total thanks @Spacechild1 :)

v0.3.2 is up on Deken.