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. >
> Maybe we should just remove this feature instead?
having some variable that is not local to the current point evaluation is
needed for random() as it uses it as state
ld/st are also documented in doc/utils.texi and are the basis for multiple
features so they themselfs cant be removed either
>
> Fixes ticket #7528.
>
> Signed-off-by: Marton Balint <c...@passwd.hu>
> ---
> libavutil/eval.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/libavutil/eval.c b/libavutil/eval.c
> index 62d2ae938b..e726c94195 100644
> --- a/libavutil/eval.c
> +++ b/libavutil/eval.c
> @@ -54,7 +54,7 @@ typedef struct Parser {
> int log_offset;
> void *log_ctx;
> #define VARS 10
> - double *var;
> + double var[VARS];
> } Parser;
>
> static const AVClass eval_class = {
> @@ -754,11 +754,14 @@ int av_expr_count_vars(AVExpr *e, unsigned *counter,
> int size)
> double av_expr_eval(AVExpr *e, const double *const_values, void *opaque)
> {
> Parser p = { 0 };
> - p.var= e->var;
> + double ret;
>
> + memcpy(p.var, e->var, sizeof(p.var));
> p.const_values = const_values;
> p.opaque = opaque;
> - return eval_expr(&p, e);
> + ret = eval_expr(&p, e);
> + memcpy(e->var, p.var, sizeof(p.var));
this does not look thread safe to me
also it slows the code down by extra memcopies
I think the main question is, do we need to share a expression evaluator
between threads ?
Is that useful to any feature or usecase ?
If it has no use case then the obvious solution is not to share the expression
evaluator or at least not the things written to per evaluation
Maybe for combining statistics of pixels between threads but iam not sure
how to do that with a shared expression evaluator efficeiently
Anyone has any ideas what a shared expression evaluator could be usefull for ?
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
signature.asc
Description: PGP signature
_______________________________________________ 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".
