On Sun, 15 Dec 2019, Michael Niedermayer wrote:
On Sun, Dec 15, 2019 at 01:52:52PM +0100, Marton Balint wrote:
On Sun, 15 Dec 2019, Michael Niedermayer wrote:
On Sun, Dec 15, 2019 at 02:16:49AM +0100, Marton Balint wrote:
Unfortunately the ld() and st() operations store the variables in the AVExpr
context and not in the parser in order to keep the stored values between
evaluations (which is a rarely(?) used undocumented(?) feature of AVExpr).
This causes data race issues when the same expression is evaluated from
multiple threads. Let's use the Parser as variable store during evaluation and
only sync the AVExpr context variables with the Parser before and after
evaluation. This keeps the feature working for single threaded cases and fixes
using ld() and st() usage for the "normal" use case when the ld()-s only
reference st()-d values from the same evaluation.
[...]
The confusion is caused by mixing the state with the parsed expression.
Maybe we should decouple them, or at least add the possibility to the user
to use a custom state.
yes decoupling them / giving the user more control over the state makes sense
Anyway, I am inclined to accept what Gyan proposed, to determine if the
expression uses lt() st() or rand() and if it does fall back to 1 thread. Or
implement something like av_expr_is_stateless() to do this in a helper
function.
This solution kind of sucks because it forces a heap of cases to run
single threaded which would work find with multiple threads
OTOH with one AVExpr or AVExprState per thread it would work multi threaded
That would resolve the data race issue in the ticket. However, if the
expression depends on previous state then the result will be
undeterministic because it is random which slices get assigned to which
threads, so the initial AVExprState they see will be random as well.
If you force single thread, the result will be deterministic.
The one case the state per thread will need to take a bit of care of is the
random seed, which we would want to be different per thread
We can just say that it is up to the user to initialize the random seed if
needed, that is the case for single thread as well.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
