Skip to content

svac3: decoding on several threads - #11

Merged
widgetii merged 2 commits into
masterfrom
svac3-threads
Oct 29, 2026
Merged

widgetii merged 2 commits into
masterfrom
svac3-threads

Conversation

@widgetii

Copy link
Copy Markdown
Member

Multi-threaded decoding: svac3d_set_threads(), svac3dec -t N, and FFmpeg's -threads. The output is bit-identical whatever the number of threads, since every stage keeps the decoding order.

Parse ∥ reconstruct

  • The entropy parse needs only syntax flags of earlier coding units, never reconstructed samples. So the parser now emits one record per coding unit:
    • mode, sub-type, reference, MVD;
    • intra mode indices, ctp, ΔQP, QP-group start, LCU QP;
    • the coefficients;
    • plus records for deferred chroma CUs and patch starts.
  • A reconstruction thread consumes the records in order.
  • Each side keeps its own parser_t and its own map of decoded units, because "available for prediction" means reconstructed. All the existing derivation, prediction and filter code runs unchanged on the reconstruction side.
  • Records and coefficients live in chunks that never move, published once per LCU.
  • With one thread, each record is reconstructed as soon as it is made. That is the previous order, on the previous thread.

Loop filters on a pool (svac3_pool.{c,h}, a small fork-join pool)

  • Deblocking runs in 64-row bands: all vertical edges of the picture, then all horizontal ones. Within one direction, edges 8 samples apart touch disjoint samples.
  • SAO runs by rows of LCUs. It reads the deblocked copy, so rows are independent.

Picture memory reuse: the decoder keeps each finished picture's memory (two unit maps, SAO parameters, record chunks) for the next picture (svac3_picture_cache_t). Freeing and allocating several MB per picture had cost about 170,000 page faults on the 90-picture stream, against 5,000 now, and that was a 25% slowdown on one thread.

FFmpeg: the wrapper passes thread_count to libsvac3d (0 means one thread per CPU) and declares AV_CODEC_CAP_OTHER_THREADS. The library version is now 0.2.0, which the patch's configure requires.

Checks

  • ctest decodes every golden stream on 4 threads too (recon_md5_t4_*). test_api checks the threaded API gives the same pictures. 34/34 pass in release, in ASan+UBSan, and on AArch64.
  • New CI job tsan: the threaded decoder on every corpus stream, plus test_api, under ThreadSanitizer. Clean locally.
  • Output is identical at 1, 2 and 4 threads on all 8 corpus streams and on the 2560x1440 stream.
  • FFmpeg framemd5 equals the golden hashes at -threads 0 and -threads 4.

Speed: idle Core Ultra 5 245K, P-cores, best of 3; master is single-threaded:

stream master -t 1 -t 2 -t 4 vendor -t 1 vendor -t 4
1080p CBR 0.44 s 0.45 0.32 0.30 0.51 0.26
1080p SmartP 0.22 s 0.22 0.16 0.15 0.24 0.13
1080p QP 22 ×20 1.56 s 1.59 1.05 1.02 1.83 1.40
2560x1440 3.21 s 3.24 2.04 1.90 3.34 1.76

On a Cortex-X925: 0.43 / 0.32 / 0.30 s on 1080p CBR, and 1.43 / 1.02 / 0.98 s on QP 22 ×20.

What limits scaling: beyond 2 threads, parse and reconstruction are each one thread, and they take about equal time. The loop filters still run after the whole picture. The next step is to filter in a wavefront, a couple of LCU rows behind reconstruction, so the filters overlap parse and reconstruction too.

svac3d_set_threads() (svac3dec -t N, FFmpeg's -threads):

- parsing and reconstruction run side by side. The parser emits a record
  per coding unit (modes, motion vector differences, QP state, the
  coefficients) and a reconstruction thread consumes them in order. The
  two keep separate maps of decoded units, since availability for
  prediction means reconstructed; with one thread the records are
  reconstructed as they are made, as before.
- the loop filters run in bands on a thread pool: deblocking in 64-row
  bands, all vertical edges and then all horizontal ones, and SAO by rows
  of LCUs, which touch disjoint samples.

The output is the same whatever the number of threads. A decoder keeps
each finished picture's memory (maps, SAO parameters, record chunks) for
the next: freeing and allocating several MB per picture cost tens of
thousands of page faults. The FFmpeg wrapper passes thread_count through
(0: one thread per CPU); libsvac3d is 0.2.0.

ctest decodes every golden stream on 4 threads too; a new CI job runs the
threaded decoder under ThreadSanitizer.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add deterministic multithreaded SVAC3 decoding

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Overlap entropy parsing with ordered reconstruction and parallelize loop filters without changing
 decoded pictures.
• Expose thread selection through libsvac3d, svac3dec, and FFmpeg; reuse picture memory between
 frames.
• Verify threaded output against golden hashes and add ThreadSanitizer CI coverage.
Diagram

graph TD
  API["Decoder API"] --> PARSE["Entropy parser"] --> REC["CU records"] --> RECON["Ordered reconstruction"] --> DBK["Deblocking bands"] --> SAO["SAO rows"] --> FRAME["Output frame"]
  POOL(["Thread pool"]) --> DBK & SAO
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Picture-level parallelism
  • ➕ Could scale across independent pictures without coding-unit records.
  • ➖ Reference dependencies and display ordering complicate scheduling.
  • ➖ Concurrent pictures require more memory.
2. LCU wavefront filtering
  • ➕ Could overlap filters with parsing and reconstruction for better scaling beyond two threads.
  • ➖ Requires more complex boundary dependencies and synchronization to preserve identical output.

Recommendation: The ordered-record pipeline is a reasonable first step: it overlaps the two serial stages while retaining their existing decoding order. Keep picture-level or wavefront scheduling as separate changes so their synchronization and output equivalence can be reviewed independently.

Files changed (17) +820 / -131

Enhancement (12) +777 / -129
0001-avcodec-add-SVAC-3.0-decoding-via-libsvac3d.patchPass FFmpeg thread settings to libsvac3d +12/-9

Pass FFmpeg thread settings to libsvac3d

• Updates the FFmpeg integration patch to pass thread_count, use CPU count for automatic selection, and declare external-thread capabilities. Requires libsvac3d 0.2.0.

ffmpeg/0001-avcodec-add-SVAC-3.0-decoding-via-libsvac3d.patch

svac3_dec.cOwn decoder thread pool and reusable picture cache +18/-4

Own decoder thread pool and reusable picture cache

• Creates and replaces the pool through the decoder thread-setting API. Reuses picture storage across frames and releases the pool and cache at destruction.

source/svac3/svac3_dec.c

svac3_dec.hDeclare internal decoder thread control +4/-0

Declare internal decoder thread control

• Declares the thread-setting function and documents serial fallback when fewer threads can be started.

source/svac3/svac3_dec.h

svac3_loopfilter.cParallelize deblocking and SAO work +113/-59

Parallelize deblocking and SAO work

• Runs deblocking in row bands, completing all vertical edges before horizontal edges. Runs SAO by LCU row against a deblocked working copy.

source/svac3/svac3_loopfilter.c

svac3_loopfilter.hAccept a pool in loop-filter interfaces +3/-2

Accept a pool in loop-filter interfaces

• Extends deblocking and SAO declarations to receive the optional thread pool.

source/svac3/svac3_loopfilter.h

svac3_patch.cSeparate ordered parsing and reconstruction +434/-51

Separate ordered parsing and reconstruction

• Emits coding-unit, chroma, and patch records for a reconstruction worker with its own decoded-unit map; single-threaded decoding reconstructs records immediately. Adds LCU publication, worker lifecycle handling, and reusable picture maps, SAO storage, and record chunks.

source/svac3/svac3_patch.c

svac3_patch.hDeclare pooled pictures and cache lifecycle +21/-0

Declare pooled pictures and cache lifecycle

• Adds interfaces for attaching a pool and beginning, ending, releasing, or freeing cached pictures.

source/svac3/svac3_patch.h

svac3_pool.cImplement a fork-join thread pool +126/-0

Implement a fork-join thread pool

• Adds persistent pthread workers that share indexed jobs with the caller and wait for all items to finish. Falls back to caller-only execution when a pool is unavailable.

source/svac3/svac3_pool.c

svac3_pool.hDefine the thread-pool interface +22/-0

Define the thread-pool interface

• Declares pool creation, destruction, thread-count reporting, and synchronous indexed-job execution.

source/svac3/svac3_pool.h

svac3d.cExpose validated thread selection +7/-0

Expose validated thread selection

• Adds svac3d_set_threads and rejects null decoders or thread counts below one.

source/svac3/svac3d.c

svac3d.hPublish threaded API in libsvac3d 0.2.0 +6/-1

Publish threaded API in libsvac3d 0.2.0

• Bumps the minor version and documents thread selection, unchanged output, and the returned active thread count.

source/svac3/svac3d.h

svac3dec.cAdd the svac3dec -t option +11/-3

Add the svac3dec -t option

• Accepts an optional positive thread count and configures the decoder before processing the stream.

tools/svac3dec.c

Documentation (1) +6 / -0
README.mdDocument threaded decoding and its API +6/-0

Document threaded decoding and its API

• Describes the parallel decoding stages, unchanged output, and optional svac3d_set_threads call.

README.md

Other (4) +37 / -2
ci.ymlAdd threaded ThreadSanitizer coverage +19/-0

Add threaded ThreadSanitizer coverage

• Adds a TSan job that decodes every corpus stream on four threads and runs the API test. Links the new pool source and pthreads into the fuzz build.

.github/workflows/ci.yml

CMakeLists.txtRegister four-thread golden-output tests +4/-0

Register four-thread golden-output tests

• Adds a four-thread framemd5 comparison for each stream with a golden reference.

CMakeLists.txt

check_md5.pyAllow thread selection in golden-hash checks +4/-2

Allow thread selection in golden-hash checks

• Accepts an optional thread count and passes it to svac3dec for picture-hash comparisons.

tests/check_md5.py

test_api.cCheck threaded API output and arguments +10/-0

Check threaded API output and arguments

• Checks invalid thread-setting arguments and compares four-thread output with the existing reference picture count and hash.

tests/test_api.c

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Rejected pictures leak cached memory ✓ Resolved
Description
svac3_picture_begin_cached() calls free(pic) when frame or reference validation fails, although
take_picture() may have transferred cached maps and spare chunks into that picture. A rejected
picture after cache reuse loses those allocations without releasing them, and repeated rejected
pictures accumulate the leak.
Code

source/svac3/svac3_patch.c[1610]

+    svac3_picture_t *pic = take_picture(cache);
Evidence
take_picture() clears cache->pic but retains its allocated SCU maps, SAO array and spare chunks.
The validation-error path frees only the containing structure, whereas svac3_picture_free()
explicitly frees those retained allocations.

source/svac3/svac3_patch.c[1515-1534]
source/svac3/svac3_patch.c[1566-1585]
source/svac3/svac3_patch.c[1636-1652]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Validation failure frees a reused picture structure but not the buffers retained inside it.
## Fix Focus Areas
- source/svac3/svac3_patch.c[1566-1585]
- source/svac3/svac3_patch.c[1645-1652]
## Recommended Fix
Use the picture's full cleanup routine on the validation-error path, or return the intact picture to the cache, so its maps and spare chunks remain owned and releasable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Thread setup can use an invalid condition ✓ Resolved
Description
svac3_picture_set_pool() ignores the result of pthread_cond_init() and proceeds to start the
reconstruction worker. If initialization fails, the worker can wait on an uninitialized condition
variable, and cleanup can attempt to destroy it.
Code

source/svac3/svac3_patch.c[R1778-1779]

+        pthread_cond_init(&pic->ready, NULL);
+        pic->threaded = 1;
Evidence
The unchecked initialization is followed by pthread_create(); the worker waits on pic->ready,
and the thread-creation failure path also destroys that condition variable.

source/svac3/svac3_patch.c[1441-1445]
source/svac3/svac3_patch.c[1775-1784]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed condition-variable initialization is not handled before worker creation.
## Fix Focus Areas
- source/svac3/svac3_patch.c[1775-1784]
## Recommended Fix
Check `pthread_cond_init()` and, on failure, destroy the successfully initialized mutex and continue serially. Only create the worker or destroy the condition after successful initialization.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Huge thread requests exhaust resources ✓ Resolved
Description
svac3_pool_create() allocates worker handles and attempts thread creation directly from the
requested count without a practical upper bound. A large value accepted by svac3dec -t or the
public API can drive allocation and thread creation until memory or process thread limits are
exhausted, even when the picture offers far less parallel work.
Code

source/svac3/svac3_pool.c[64]

+    p->thread = calloc((size_t)threads - 1, sizeof(*p->thread));
Evidence
The CLI accepts every positive atoi() result and forwards it. The pool allocates threads - 1
handles and loops up to that same count creating workers; svac3_dec_set_threads() applies no
additional limit.

tools/svac3dec.c[17-24]
tools/svac3dec.c[39-43]
source/svac3/svac3_dec.c[87-92]
source/svac3/svac3_pool.c[57-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A caller-provided thread count drives unbounded worker allocation and creation.
## Fix Focus Areas
- source/svac3/svac3_pool.c[57-76]
- tools/svac3dec.c[17-24]
## Recommended Fix
Set a documented practical maximum before allocating or starting workers, report the actual count through the existing API, and parse the CLI integer with range and conversion-error checks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (2)
4. Decoder can hang when memory runs out at picture end ✓ Resolved
Description
finish_worker handles a failed REC_END allocation by overwriting the last record without
advancing produced, even though the worker reads records without holding the lock. If the record
was already consumed, publish leaves published unchanged and the worker remains waiting while
pthread_join blocks; if the first chunk allocation failed, the same fallback dereferences a NULL
rec_tail.
Code

source/svac3/svac3_patch.c[R1471-1476]

+    if (pic->failed) {
+        /* ...no memory for a last chunk: end in the last record's place */
+        pthread_mutex_lock(&pic->lock);
+        pic->rec_tail->rec[pic->tail_n ? pic->tail_n - 1 : 0].kind = REC_END;
+        pthread_mutex_unlock(&pic->lock);
+    }
Evidence
decode_patch publishes after every LCU at lines 1751–1752, so previously produced records have
been published when finish_worker runs. recon_worker waits while published == done at lines
1443–1444 and stops only on a REC_END record at index done at line 1455; overwriting an earlier
record does not advance produced, so publish() at lines 1386–1392 cannot advance published to
wake a worker that has consumed it. The worker also reads records outside the lock, making the
overwrite race with a read. If emit() fails on its first chunk allocation at lines 1418–1420,
rec_tail remains NULL and the fallback dereferences it at line 1474.

source/svac3/svac3_patch.c[1386-1392]
source/svac3/svac3_patch.c[1414-1432]
source/svac3/svac3_patch.c[1435-1483]
source/svac3/svac3_patch.c[1748-1753]
source/svac3/svac3_patch.c[1403-1432]
source/svac3/svac3_patch.c[1441-1457]
source/svac3/svac3_patch.c[1462-1479]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When allocating the `REC_END` record fails, `finish_worker` overwrites a potentially consumed record and publishes without increasing `produced`. The worker can remain waiting while `pthread_join` blocks, and the fallback dereferences `rec_tail` if no chunk was allocated.
## Fix Focus Areas
- source/svac3/svac3_patch.c[1348-1356]
- source/svac3/svac3_patch.c[1435-1483]
## Recommended Fix
Add an `int stop` field to `svac3_picture` to represent producer completion separately from allocated records. In `finish_worker`, take `lock`, set `stop = 1` and `published = produced`, signal `ready`, unlock, then join; do not emit `REC_END` or overwrite a record. In `recon_worker`, wait while `published == done && !stop`, read `stop` under the lock, consume all published records, and return when `stop` is set and `done == published`, including when there are zero records. Remove the NULL `rec_tail` access.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Library builds fail to link on systems without pthreads in libc ✓ Resolved
Description
svac3_pool.c and svac3_patch.c call pthread functions, but the svac3d CMake target has no
threads link dependency and its generated libsvac3d.pc lists only -lsvac3d. When pthread symbols
require a separate library, especially with the default static build on glibc older than 2.34,
linking svac3dec, the tests, or FFmpeg’s require_pkg_config libsvac3d ... svac3d_set_threads
check fails with undefined pthread symbols; FFmpeg reports this as “libsvac3d not found”.
Code

source/svac3/svac3_pool.c[R6-7]

+#include <pthread.h>
+#include <stdlib.h>
Evidence
source/CMakeLists.txt creates svac3d with add_library but has no `target_link_libraries(svac3d
...), and it generates a pkg-config Libs: entry containing only -lsvac3d`. The pool uses pthread
mutexes, condition variables, and thread creation, while the neighbouring uavs3d target links
pthread and advertises -lpthread in its pkg-config file. The fuzz CI job adds -lpthread
manually; the ubuntu-24.04 CI environment uses glibc 2.39, where pthread symbols are provided by
libc.

source/CMakeLists.txt[145-168]
source/CMakeLists.txt[189-204]
source/svac3/svac3_pool.c[6-11]
source/svac3/svac3_pool.c[73-75]
source/CMakeLists.txt[149-153]
source/CMakeLists.txt[179-195]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
libsvac3d uses pthreads without declaring the dependency on its CMake target or in its pkg-config metadata. Static consumers on platforms requiring a separate pthread library can fail to link, including FFmpeg’s configure check.
## Fix Focus Areas
- source/svac3/svac3_pool.c[6-11]
- source/CMakeLists.txt[179-204]
## Recommended Fix
Set `THREADS_PREFER_PTHREAD_FLAG` to `ON`, call `find_package(Threads REQUIRED)`, and link `svac3d` with `target_link_libraries(svac3d PUBLIC Threads::Threads)`. Add the static-link dependency to the generated `libsvac3d.pc`, using `Libs.private: -lpthread` or adding `-lpthread` to `Libs:` as the uavs3d pc does. Verify that a static consumer links using the generated pkg-config metadata.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. Large motion vector differences decode differently than before ✓ Resolved
Description
coding_unit() narrows parsed motion-vector differences into cu_rec_t’s int16_t mvd[2] before
derive_mvp() can clip the predictor-plus-difference sum. When parse_mvd_abs() returns a value
above 32767, the stored value can wrap—for example, 32768 becomes negative—so reconstruction
produces a different motion vector than the previous code, regardless of thread count.
Code

source/svac3/svac3_patch.c[983]

+        .dqp = (int16_t)dqp, .mvd = { (int16_t)mvd[0], (int16_t)mvd[1] },
Evidence
parse_mvd_abs() computes 3 + 2*byp_eg0 + even without an upper bound (lines 471–482), while
derive_mvp() clips pred + mvd[c] to the signed 16-bit motion-vector range (line 646). The record
declares int16_t mvd[2] (line 748), coding_unit() casts the parsed difference to that type (line
983), and reconstruction reads the narrowed value (line 818), so narrowing occurs before the final
clipping step.

source/svac3/svac3_patch.c[471-482]
source/svac3/svac3_patch.c[616-646]
source/svac3/svac3_patch.c[742-751]
source/svac3/svac3_patch.c[141-153]
source/svac3/svac3_patch.c[472-482]
source/svac3/svac3_patch.c[616-647]
source/svac3/svac3_patch.c[978-985]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The coding-unit record narrows parsed motion-vector differences outside the signed 16-bit range before `derive_mvp()` clips the resulting vector, changing reconstruction from the previous behavior.
## Fix Focus Areas
- source/svac3/svac3_patch.c[742-751]
- source/svac3/svac3_patch.c[978-986]
## Recommended Fix
Store both MVD components in `cu_rec_t` as `int32_t mvd[2]` and remove the narrowing casts so reconstruction passes the parsed differences to `derive_mvp()` unchanged. Test a parsed difference outside the signed 16-bit range against the previous clipping behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread source/svac3/svac3_patch.c Resolved
Comment thread source/svac3/svac3_patch.c Resolved
Comment thread source/svac3/svac3_pool.c Resolved
Comment thread source/svac3/svac3_patch.c Resolved
Comment thread source/svac3/svac3_pool.c Resolved
Comment thread source/svac3/svac3_patch.c Resolved
- a picture rejected at begin frees what it took from the cache
- pthread_cond_init() is checked; on failure the picture runs unthreaded
- at most 64 threads (SVAC3_MAX_THREADS)
- the reconstruction thread stops on a flag set once every record is
  published, not on an end record: no allocation, nothing overwritten
- svac3d links Threads::Threads, and libsvac3d.pc lists -lpthread
- records keep the motion vector differences in 32 bits, as parsed, so
  derive_mvp() clips the same sum as before
@widgetii

Copy link
Copy Markdown
Member Author

Thanks. All six valid; all fixed in the latest commit.

  1. Rejected pictures leak cached memory: fixed. The validation-error path in svac3_picture_begin_cached() now calls svac3_picture_free(), which releases the maps, the SAO array and the spare chunks taken from the cache.
  2. Thread setup can use an invalid condition: fixed. pthread_cond_init() is checked. On failure the mutex is destroyed and the picture is decoded unthreaded.
  3. Huge thread requests exhaust resources: fixed. The pool takes at most SVAC3_MAX_THREADS (64), and svac3d_set_threads() documents the cap. svac3dec -t 100000 decodes normally on 64.
  4. Decoder can hang when memory runs out at picture end: fixed by removing the end record altogether. finish_worker() sets an ending flag and publishes under the lock. The worker exits once it has done every published record and sees the flag. Nothing is allocated or overwritten at the end, so neither the hang nor the NULL dereference can happen.
  5. Link failures without pthreads in libc: fixed. target_link_libraries(svac3d Threads::Threads), and libsvac3d.pc lists -lpthread, as uavs3d.pc does.
  6. Large MVDs narrowed: fixed. Records keep the MVDs as int32_t, exactly as parsed, so derive_mvp() clips the same predictor-plus-difference sum as before.

Rechecked: 34/34 in release, output identical at -t 4 on all 9 streams, ThreadSanitizer clean on all 8 corpus streams at -t 4, ASan clean on the threaded tests.

@widgetii
widgetii merged commit 3f5a8d4 into master Oct 29, 2026
6 checks passed
@widgetii
widgetii deleted the svac3-threads branch October 29, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant