dyne / frei0r

A large collection of free and portable video plugins
https://frei0r.dyne.org/
GNU General Public License v2.0
419 stars 91 forks source link

thread safety #188

Open jaromil opened 2 months ago

jaromil commented 2 months ago

Some frei0r filters are not thread safe, a non-exhaustive list of them is provided by this issue https://github.com/mltframework/mlt/issues/274 and pasted below:

baltan.so
cartoon.so
cluster.so
delay0r.so
delaygrab.so
equaliz0r.so
facebl0r.so
facedetect.so
flippo.so
glow.so
hqdn3d.so
lightgraffiti.so
mask0mate.so
nervous.so
plasma.so
rgbparade.so
scale0tilt.so
sharpness.so
squareblur.so
tehroxx0r.so
vectorscope.so
vertigo.so

We need to:

  1. see if we can fix the source of these filters to ensure thread safety (reentrancy)
  2. elaborate a strategy to allow hosts to load these filters only if explicitly desired
  3. update this list and make a thread-safety test running in this CI
esmane commented 2 months ago

I think the problem with a lot of these effects is that they use a buffer of the frame that's stored in the plugin instance. So if multiple frames were to be rendered at the same time they would all be using the same buffer and it would lead to the issues. I think these functions could be made thread safe rather easily by allocating and freeing the buffer within the f0r_update function instead of f0r_construct/destruct so that each thread would have it's own buffer.

But some of the more animated like effects (for example vertigo) I think would be extremely difficult to make thread safe since the operations done to each frame depend on the operations done to the previous frame. I don't think it would be possible to render multiple frames at once with vertigo because to work correctly the frames must be rendered sequentially and in order.

jaromil commented 2 months ago

Thanks @esmane your explanation is clear and includes for anyone approaching the issue.