[FFmpeg-devel] [PATCH 3/3] pthread : Protect progress & progress_used access with progress_mutex.

[Reimar Döffinger]

On Thu, Mar 22, 2012 at 09:58:17AM -0700, Aaron Colwell wrote:
> Here I am just protecting progress & progress_used with progress_mutex. I
> expect this to be one of the most controversial changes because I believe
> this may cause a noticable performance drop.
> 
> Questions:
> Why is the existing code considered safe?
> How are thread protected from accidentally observing a MAX_INT progress
> right before progress gets set to -1? It seems like this could cause
> ff_thread_await_progress() calls to return early.

Assuming the progress_used part is working correctly:
It will only be reused when progress_used is 0.
progress_used is only 0 when there is no frame using
it any more.
progress can only by accessed through a frame.
Thus it can only be used after the frame allocation is done,
i.e. ff_thread_get_buffer has returned, but by that time
it already has been reset to -1.

Now as to progress_used working correctly...
It is in a per-thread context, and most of its accesses
are from that single thread (in particular, the check
for setting progress to INT_MAX and the code one setting
progress_used to 1 are always in the same thread).
The exception is the free_progress. However it and allocate_progress
are protected with the buffer lock against each other.
So that leaves the INT_MAX related code as an issue.
However due to being in the same thread, it cannot
miss progress_used being set to 1.
The worst that can happen is that is misses progress_used
being set to 0, in which case it will set a value to INT_MAX
that we know is no longer reachable except via allocate_progress.
No, this is not obvious, possibly can be improved and at
least can be documented. I do not think it is wrong though,
not even with some of the more strict models.