After learning interesting things from David about volatile keyword use, memory barriers and so on, I want to show three solutions to my original problem, with pros and cons.
I can't be defined a guru, so my solutions could be wrong and I will be happy if someone will write if they are right or wrong, giving suggestions. I'm writing those solutions mainly because explaining something to others is the best method to check if you have understood the topic.
Here I will try to better define the original problem.
A simple audio library should be written based around the main function play(). The audio stream to play is in RAM and is simply an array of samples to generate at the output of a DAC peripheral, embedded in the CPU. If the application calls play() when an audio stream is actually playing, the library should stop the actual stream and starts playing the new stream. The samples are generated at the output at the sampling frequency, generated by a timer interrupt.
--- audio.c --- typedef struct { const uint16_t *samples; size_t size; uint16_t *s; bool init; } AudioStream;
static AudioStream curr_s; // current stream in playing static AudioStream next1_s, next2_s; // next streams to play static AudioStream *curr, *next; // used mainly as atomic flags
void Timer_ISR(void) { uint16_t sample; bool dac_refresh = false; if (next) { // a new stream is pending curr_s = *next; curr = &curr_s; curr->init = 0; next = NULL; } if (curr) { // we are playing a stream if (!curr->init) { curr->s = curr->samples; curr->init = 1; } sample = *(curr->s); dac_refresh = true; if (++curr->s - curr->samples == curr->size) { curr = NULL; // last sample processed } } if (dac_refresh) DAC_VAL = sample; }
void play(const uint16_t *samples, size_t size) { volatile AudioStream **const c = (volatile AudioStream **)&curr; volatile AudioStream **const n = (volatile AudioStream **)&next; if (*c == NULL) { volatile AudioStream *const s = (volatile AudioStream *)&curr_s; s->samples = samples; s->size = size; s->init = 0; *c = &curr_s; } else if (*n != &next1_s) { volatile AudioStream *const s = (volatile AudioStream *)&next1_s; s->samples = samples; s->size = size; s->init = 0; *n = &next1_s; } else { volatile AudioStream *const s = (volatile AudioStream *)&next2_s; s->samples = samples; s->size = size; s->init = 0; *n = &next2_s; } }
--- audio.c ---
This is my preferred solution, because it doesn't use memory barriers at all, so it is perfectly portable on different platforms and completely standard.
I use two buffers for the next audio stream to play. They are necessary in situation where three (or more) calls to play() functions happens in main application, without any timer interrupt occurred:
play(audio1, audio1_size); ... play(audio2, audio2_size); ... play(audio3, audio3_size);
With only one next buffer, there is a real possibility that troubles could happen if timer interrupt triggers exactly at a precise point (that is my original question in my original post).
The static variables that are changed in ISR aren't declared volatile. This is to avoid stopping optimizing the ISR (having a small and fast ISR is surely a benefit).
The volatile memory accesses should be done in main/background application, i.e. in play() functions. This is why I don't access directly to curr, curr_s, next, next1_s and next2_s. I declared mirror pointers to volatile objects.
As pointed by David, it's important to use volatile for *all* the variables involved in play() function. Let's take a look at one part of it:
if (*c == NULL) { volatile AudioStream *s = (volatile AudioStream *)&curr_s; s->samples = samples; s->size = size; s->init = 0; *c = &curr_s;
If *c (that is curr) is NULL, we aren't playing any stream and ISR doesn't change any variable. It could appear safe to not use volatile in this first branch in the following *wrong* way:
if (*c == NULL) { curr_s.samples = samples; curr_s.size = size; curr_s.init = 0; *c = &curr_s; ...
Here curr_s isn't volatile so compiler could reorder instructions. If the last statement "*c = &curr_s" is moved at the top (and this is allowed) we will be in trouble if interrupt triggers exactly after.
In order to avoid reordering of instructions, we *must* use *only* volatile accesses. As David explained, compiler can't change the order of volatile memory accesses.
The only drawback of this approach is the programmer must carefully check the critical code (function play), checking if an interrupt trigger *at any point* could generate problems. To avoid problems, you will discover you need two next buffers and an atomic flag that switches from one buffer to the other.
Regarding curr and next flags, they are pointers. So I'm supposing the architecture is capable to atomically read/write pointers. If this isn't the case (for example, AVR architectures), you could change pointers to unsigned char: curr would be current_stream_is_active (0 or 1) and next could be next_stream_pending (0 for no pending stream, 1 for stream pending in next1_s, 2 for stream pending in next2_s).
I hope I haven't written too wrong things.