Skip to content

threading: explicitly re-assign task type when adding the next row

Ronald S. Bultje requested to merge rbultje/dav1d:users/rbultje/fix400 into master

The previous row type can be updated from deblock_rows to cdef when deblock is disabled, but the top buffers are not yet available. This can lead to race conditions, e.g. see #400. Explicitly re-assigning type fixes the issue.

[edit] @kuroso asked about whether this is a regression. The upstream bug report has the following regression range (I added the ^):

$ git log --oneline -5 a55ff11da2430ae7057862f6565ce3b5a222afdc^..f7e0d4c032dad31a46129c9eaf30ad5c2f704d2b
f7e0d4c0 Remove lpf_stride parameter from LR filters
609fbaba Allow CDEF and LR to run sbrows in parallel
8e6d5214 CI: Add tests for negative stride
8c94f95c meson: Check for the pthread_getaffinity_np function before deciding to use it
a55ff11d arm64: Add Armv8.5-A BTI support to assembly files

So presumably 609fbaba is the one that introduced this issue.

The issue isn't really a race in the strict sense, rather just stale data. It appears to me that t->type is not constant since that revision (it can switch between _DEBLOCK_ROWS and _CDEF if deblock.level_y[] = { 0, 0 }), and so the copy of sby's task into sby + 1's task (*next_t = *t;) can use the "wrong" value, since it assumes it's constant (next_t->type should always be _DEBLOCK_ROWS for the deblock.level_y[] = { 0, 0 } case).

Edited by Ronald S. Bultje

Merge request reports

Loading