Hi, this project is has been very helpful personally and I notice that it has amassed a lot of stars over the years. I wanted to say thank you by submitting fixes for two common issues that I have encountered with more complex pipelines.
When more than one thread is used and filters are constructed per-frame (like with FrameEvaluator) there is a crash in the constructor for BM3D_FilterData.
Execution of plans (via fftw_execute) is guaranteed to be thread-safe but all other calls are not. Plan creation involves mutation of the planner's in-memory "wisdom" - see FFTW 3.3.10 - Section 5.4: Thread Safety for more information.
Solution
I could not figure out any effective strategy for filter instances to coordinate FFTW resource access within VapourSynth. The best solution I could come up with for now is to use a static std::mutex scoped to BM3D_FilterData. Now the crash is prevented by serializing access to the planner, while plans continue to execute in parallel as before.
As a solution I introduced one std::mutex to protect access to each map. I would prefer std::shared_mutex but it is only available in C++17 and I consider upgrading out of scope for this PR.
Hi, this project is has been very helpful personally and I notice that it has amassed a lot of stars over the years. I wanted to say thank you by submitting fixes for two common issues that I have encountered with more complex pipelines.
Setup
macOS 10.15.7 BM3D - r9 vs.core.threads = 2+
1. Crash when Constructing
BM3D_FilterData
When more than one thread is used and filters are constructed per-frame (like with FrameEvaluator) there is a crash in the constructor for
BM3D_FilterData
.Problem
Execution of plans (via
fftw_execute
) is guaranteed to be thread-safe but all other calls are not. Plan creation involves mutation of the planner's in-memory "wisdom" - see FFTW 3.3.10 - Section 5.4: Thread Safety for more information.Solution
I could not figure out any effective strategy for filter instances to coordinate FFTW resource access within VapourSynth. The best solution I could come up with for now is to use a
static std::mutex
scoped toBM3D_FilterData
. Now the crash is prevented by serializing access to the planner, while plans continue to execute in parallel as before.2. Data Race with
std::unordered_map
This is a fix regarding commit https://github.com/HomeOfVapourSynthEvolution/VapourSynth-BM3D/commit/9b4c1065e2a8783e02837f5ed2aa4efc702c7f21 which introduced per-thread storage. The problem is that
std::unordered_map
is not guaranteed to be thread safe for multiple readers and writers.As a solution I introduced one
std::mutex
to protect access to each map. I would preferstd::shared_mutex
but it is only available in C++17 and I consider upgrading out of scope for this PR.