VapourSynth-BM3D
VapourSynth-BM3D copied to clipboard
Fix crash constructing BM3D_FilterData, data race with std::unordered_map
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
.
Thread 16 Crashed:
0 libfftw3f.3.dylib 0x000000010516b109 fftwf_mkapiplan + 338
1 libfftw3f.3.dylib 0x000000010516da63 fftwf_plan_many_r2r + 225
2 libfftw3f.3.dylib 0x000000010516db5a fftwf_plan_r2r + 39
3 libfftw3f.3.dylib 0x000000010516db2e fftwf_plan_r2r_3d + 72
4 libbm3d.dylib 0x000000010505f383 BM3D_FilterData::BM3D_FilterData(bool, double, int, int, double) + 643
5 libbm3d.dylib 0x0000000105061419 BM3D_Data_Base::init_filter_data() + 601
6 libbm3d.dylib 0x0000000105070cf8 BM3D_Basic_Data::arguments_process(VSMap const*, VSMap*) + 120
7 libbm3d.dylib 0x000000010508c12a BM3D_Basic_Create(VSMap const*, VSMap*, void*, VSCore*, VSAPI const*) + 138
8 libvapoursynth.dylib 0x00000001046bf822 VSPluginFunction::invoke(VSMap const&) + 844
...
Thread 17:
0 libfftw3f.3.dylib 0x00000001050b3260 dotile_buf + 89
...
24 libfftw3f.3.dylib 0x000000010516b0f9 fftwf_mkapiplan + 322
25 libfftw3f.3.dylib 0x000000010516da63 fftwf_plan_many_r2r + 225
26 libfftw3f.3.dylib 0x000000010516db5a fftwf_plan_r2r + 39
27 libfftw3f.3.dylib 0x000000010516db2e fftwf_plan_r2r_3d + 72
28 libbm3d.dylib 0x000000010505f383 BM3D_FilterData::BM3D_FilterData(bool, double, int, int, double) + 643
29 libbm3d.dylib 0x0000000105061419 BM3D_Data_Base::init_filter_data() + 601
30 libbm3d.dylib 0x0000000105070cf8 BM3D_Basic_Data::arguments_process(VSMap const*, VSMap*) + 120
31 libbm3d.dylib 0x000000010508c12a BM3D_Basic_Create(VSMap const*, VSMap*, void*, VSCore*, VSAPI const*) + 138
32 libvapoursynth.dylib 0x00000001046bf822 VSPluginFunction::invoke(VSMap const&) + 844
...
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 to BM3D_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 prefer std::shared_mutex
but it is only available in C++17 and I consider upgrading out of scope for this PR.