Skip to content

Fix A/V sync bugs and add sync test infrastructure #1977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
richiemcilroy merged 14 commits into from
Jul 4, 2026
+5,724 −309
Merged

Fix A/V sync bugs and add sync test infrastructure #1977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
10f86db
Fix A/V sync timestamp bugs across recording, encoding and playback
richiemcilroy Jul 3, 2026
9e93a90
Add av-sync selftest, synthetic device sync matrix and sync CI workflow
richiemcilroy Jul 3, 2026
4fb9561
Harden sync CI for shared runners and Linux ffmpeg setup
richiemcilroy Jul 3, 2026
a2e0a52
Use ubuntu-24.04 and per-OS target triples in sync CI
richiemcilroy Jul 3, 2026
af34238
Add randomized combined A/V cases and encoder-load content to sync ma…
richiemcilroy Jul 3, 2026
2fad8bb
Track pts holes explicitly and serve true VFR holds in editor decoders
richiemcilroy Jul 3, 2026
48236f2
Excise pauses from the video timeline and harden sync checks and tests
richiemcilroy Jul 3, 2026
f8b8fe8
Add editor playback sync selftest with headless audio sink and CI wiring
richiemcilroy Jul 3, 2026
15d59df
Fix clippy needless borrow in desktop logging
richiemcilroy Jul 3, 2026
aee8804
Adapt playback selftest for shared CI runners and add decoder override
richiemcilroy Jul 3, 2026
e01758e
Run CI playback harness at 30fps with the ffmpeg decoder on macOS run…
richiemcilroy Jul 3, 2026
336f309
Scope CI playback harness to Linux where runners can present in real …
richiemcilroy Jul 3, 2026
9ecef54
Scale sync matrix relative tolerance with frame period for low-fps cases
richiemcilroy Jul 4, 2026
571fd50
Guard wall-clock-sensitive tests against saturated CI runners
richiemcilroy Jul 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 161 additions & 0 deletions .github/workflows/sync-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
name: A/V Sync Tests

# Verifies audio/video sync correctness on every platform:
# 1. Unit + property tests for the timestamp pipeline (encoders, drift
# trackers, muxers).
# 2. The synthetic device matrix: fake cameras/screens/microphones across
# frame rates, sample rates, channel counts and delivery pathologies
# (jitter, drops, static-screen gaps), driven through the real recording
# pipeline and verified at the container level. No capture hardware or
# GPU required, so results are deterministic on hosted runners.
#
# Findings are published as a job-summary table and a JSON artifact per OS.

on:
workflow_dispatch:
schedule:
- cron: "0 5 * * *"
pull_request:
paths:
- "crates/recording/**"
- "crates/enc-ffmpeg/**"
- "crates/enc-avfoundation/**"
- "crates/enc-mediafoundation/**"
- "crates/timestamp/**"
- "crates/rendering/**"
- "crates/media-info/**"
- ".github/workflows/sync-tests.yml"

concurrency:

@tembo tembo Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding an explicit permissions: block here. This workflow checks out code and uploads artifacts, so something like this keeps the token scoped tightly.

Suggested change
concurrency:
permissions:
contents: read
actions: write
concurrency:

group: sync-tests-${{ github.head_ref || github.ref_name }}
cancel-in-progress: true

permissions:
contents: read

jobs:
sync-tests:
strategy:
fail-fast: false
matrix:
runner:
- macos-latest
- windows-2022
- ubuntu-24.04
runs-on: ${{ matrix.runner }}

@superagent-security superagent-security Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Workflow does not declare permissions, leaving GITHUB_TOKEN scope to repo defaults

Workflow has no permissions: block at top or job level.

Add permissions: {} at the workflow top and grant least-privilege per job.

AI prompt 
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name=".github/workflows/sync-tests.yml">
<violation number="1" location=".github/workflows/sync-tests.yml:42">
<priority>P1</priority>
<title>Workflow does not declare `permissions`, leaving `GITHUB_TOKEN` scope to repo defaults</title>
<evidence>The workflow has no `permissions:` block at the top level or job level. The default GITHUB_TOKEN permissions depend on repo and org settings; older repos may default to write-all, giving this test workflow broad access it does not need.</evidence>
<recommendation>Add `permissions: {}` at the workflow top level, then grant each job only the permissions it needs. For example, the `sync-tests` job needs `contents: read` for checkout and `actions: write` for artifact upload.</recommendation>
</violation>
</file>

permissions:
contents: read
steps:
- name: Checkout
uses: actions/checkout@v4

@superagent-security superagent-security Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Actions are pinned to mutable tags instead of commit SHAs

uses: lines reference mutable tags instead of immutable commit SHAs.

Pin all actions to full 40-character SHAs with version comments.

AI prompt 
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name=".github/workflows/sync-tests.yml">
<violation number="1" location=".github/workflows/sync-tests.yml:45">
<priority>P1</priority>
<title>Actions are pinned to mutable tags instead of commit SHAs</title>
<evidence>The workflow references several third-party and first-party actions by mutable tags: `uses: actions/checkout@v4`, `uses: dtolnay/rust-toolchain@1.88.0`, `uses: actions/setup-node@v4`, and `uses: actions/upload-artifact@v4`. Tags can be force-pushed by an attacker who compromises the action repository, silently injecting malicious code into this workflow.</evidence>
<recommendation>Pin every action to the full 40-character commit SHA of the intended version and append a version comment for readability. For example: `uses: actions/checkout@<sha> # v4`.</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 actions/checkout leaves credentials persisted in the workspace

Checkout step persists GITHUB_TOKEN in git config by default.

Add persist-credentials: false to the checkout step.

AI prompt 
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name=".github/workflows/sync-tests.yml">
<violation number="1" location=".github/workflows/sync-tests.yml:50">
<priority>P2</priority>
<title>actions/checkout leaves credentials persisted in the workspace</title>
<evidence>The workflow uses `actions/checkout@v4` without setting `persist-credentials: false`. The default behavior keeps the GITHUB_TOKEN in the local git config, where it can be read by later steps or untrusted build scripts.</evidence>
<recommendation>Add `persist-credentials: false` to the checkout step, or explicitly set it to `true` only if the workflow needs to push back to the repository.</recommendation>
</violation>
</file>


- name: Rust setup
uses: dtolnay/rust-toolchain@1.88.0

- name: Rust cache
uses: ./.github/actions/setup-rust-cache
with:
target: ${{ runner.os == 'Windows' && 'x86_64-pc-windows-msvc' || runner.os == 'macOS' && 'aarch64-apple-darwin' || 'x86_64-unknown-linux-gnu' }}

- name: Install desktop dependencies
uses: ./.github/actions/install-desktop-deps

- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: 24

- name: Native dependencies
env:
RUST_TARGET_TRIPLE: ${{ runner.os == 'Linux' && 'x86_64-unknown-linux-gnu' || runner.os == 'Windows' && 'x86_64-pc-windows-msvc' || 'aarch64-apple-darwin' }}
run: node scripts/setup.js

- name: Add FFmpeg DLLs to PATH
if: runner.os == 'Windows'
shell: pwsh
run: Add-Content -Path $env:GITHUB_PATH -Value "${{ github.workspace }}\\target\\ffmpeg\\bin"

- name: Install software Vulkan driver (Linux)
if: runner.os == 'Linux'
run: |
sudo apt-get update
sudo apt-get install -y mesa-vulkan-drivers libvulkan1

- name: Timestamp pipeline unit + property tests
shell: bash
run: |
cargo test --locked -p cap-timestamp -p cap-enc-ffmpeg
cargo test --locked -p cap-recording --lib
cargo test --locked -p cap-rendering

- name: Synthetic device matrix
id: matrix
continue-on-error: true
shell: bash
env:
CAP_SYNC_MATRIX_REPORT: ${{ github.workspace }}/sync-matrix-${{ matrix.runner }}.json
CAP_SYNC_MATRIX_RANDOM_CASES: ${{ github.event_name == 'schedule' && '40' || '6' }}
run: |
cargo test --locked -p cap-recording --test sync_matrix -- --nocapture

# Verifies the editor's playback machinery (decoders, frame scheduling,
# audio pipeline) preserves sync. The fixture recording is generated
# through the real recording pipeline, so no capture hardware is needed;
# rendering uses the platform's software adapter where no GPU exists.
# 30s of pattern stabilizes the drift slope against frame-quantization
# noise. Playback runs at the default 30 fps: lower rates trip the audio
# sync policy's drift-correction threshold every few frames and accrue
# real (policy-induced) drift that fails the gate.
#
# Linux-only: the Windows WARP adapter composites blank frames and the
# macOS runners' paravirtualized Metal collapses to ~2 fps presentation
# regardless of decoder, so neither can sustain a wall-clock playback
# measurement. The decoder logic under test (FFmpeg gap holds) is fully
# exercised here; the macOS AVAssetReader path is covered by running
# `cap selftest playback` locally on real hardware.
- name: Editor playback sync harness
if: runner.os == 'Linux'
shell: bash
run: |
cargo run --locked -p cap -- --log-level info selftest playback --duration 30 --json

@tembo tembo Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the comment calls out “default 30 fps”, might be worth pinning it explicitly so this can’t change silently if the CLI default ever shifts.

Suggested change
cargo run --locked -p cap -- --log-level info selftest playback --duration 30 --json
cargo run --locked -p cap -- --log-level info selftest playback --duration 30 --fps 30 --json


- name: Report findings
if: always()
shell: bash
run: |
REPORT="${{ github.workspace }}/sync-matrix-${{ matrix.runner }}.json"
{
echo "## A/V sync matrix — ${{ matrix.runner }}"
echo ""
if [ -f "$REPORT" ]; then
PYTHONIOENCODING=utf-8 python3 - "$REPORT" << 'PYEOF'

@tembo tembo Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On windows-2022 the Bash environment usually has python but not always python3. Might be worth making this pick the right binary per runner so the summary generation can’t flake.

Suggested change
PYTHONIOENCODING=utf-8 python3 - "$REPORT" << 'PYEOF'
PYTHON_BIN=python3
if [ "$RUNNER_OS" = "Windows" ]; then PYTHON_BIN=python; fi
PYTHONIOENCODING=utf-8 "$PYTHON_BIN" - "$REPORT" << 'PYEOF'

import json, sys
report = json.load(open(sys.argv[1]))
print(f"Randomized seed: `{report.get('seed')}` (rerun with CAP_SYNC_MATRIX_SEED)")
print()
print("| Case | Result | Detail |")
print("| --- | --- | --- |")
for case in report.get("cases", []):
verdict = "PASS" if case["pass"] else "FAIL"
detail = case["detail"].replace("|", "\\|")
print(f"| {case['name']} | {verdict} | {detail} |")
PYEOF
else
echo "No report produced — the matrix crashed before writing results."
fi
} >> "$GITHUB_STEP_SUMMARY"

- name: Upload findings
if: always()
uses: actions/upload-artifact@v4
with:
name: sync-matrix-${{ matrix.runner }}
path: ${{ github.workspace }}/sync-matrix-${{ matrix.runner }}.json
if-no-files-found: ignore

- name: Fail on matrix failures
if: steps.matrix.outcome == 'failure'
shell: bash
run: |
echo "Synthetic sync matrix reported failures; see the job summary." >&2
exit 1
Loading
Loading