PR #20638 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20638 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20638.patch
Avoids the following Coverity warning: > CID 1666425: Concurrent data access violations (MISSING_LOCK) > Accessing "fg->best_input" without holding lock "Scheduler.schedule_lock". > Elsewhere, "SchFilterGraph.best_input" is written to with > "Scheduler.schedule_lock" held 2 out of 2 times (2 of these accesses > strongly imply that it is necessary). I'm pretty sure this is a false positive, as `fg->best_input` is only ever written by the filter thread - so there's zero risk of concurrent write with this read. However, coverity seems to not be aware of this fact. To be honest, I'm not sure why it didn't trigger before, but possibly because the only other writing location before was in the same function, so maybe it was smart enough to understand that it can't race with itself. A mutex is nowehere near expensive enough that this once-per-frame extra lock/unlock is going to be in any way measurable. >From 8248d22720e36edf1fc04269e047c5218526f14d Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 1 Oct 2025 18:15:48 +0200 Subject: [PATCH] fftools: silence coverity race condition warning Avoids the following Coverity warning: > CID 1666425: Concurrent data access violations (MISSING_LOCK) > Accessing "fg->best_input" without holding lock "Scheduler.schedule_lock". > Elsewhere, "SchFilterGraph.best_input" is written to with > "Scheduler.schedule_lock" held 2 out of 2 times (2 of these accesses > strongly imply that it is necessary). I'm pretty sure this is a false positive, as `fg->best_input` is only ever written by the filter thread - so there's zero risk of concurrent write with this read. However, coverity seems to not be aware of this fact. To be honest, I'm not sure why it didn't trigger before, but possibly because the only other writing location before was in the same function, so maybe it was smart enough to understand that it can't race with itself. A mutex is nowehere near expensive enough that this once-per-frame extra lock/unlock is going to be in any way measurable. --- fftools/ffmpeg_sched.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/fftools/ffmpeg_sched.c b/fftools/ffmpeg_sched.c index d08f4a061d..5c52779588 100644 --- a/fftools/ffmpeg_sched.c +++ b/fftools/ffmpeg_sched.c @@ -2445,19 +2445,16 @@ int sch_filter_receive(Scheduler *sch, unsigned fg_idx, av_assert0(*in_idx <= fg->nb_inputs); - // update scheduling to account for desired input stream, if it changed - // - // this check needs no locking because only the filtering thread - // updates this value - if (*in_idx != fg->best_input) { - pthread_mutex_lock(&sch->schedule_lock); + pthread_mutex_lock(&sch->schedule_lock); + // update scheduling to account for desired input stream, if it changed + if (*in_idx != fg->best_input) { fg->best_input = *in_idx; schedule_update_locked(sch); - - pthread_mutex_unlock(&sch->schedule_lock); } + pthread_mutex_unlock(&sch->schedule_lock); + if (*in_idx == fg->nb_inputs) { int terminate = waiter_wait(sch, &fg->waiter); return terminate ? AVERROR_EOF : AVERROR(EAGAIN); -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
