Repository navigation
feat(build): decode AV1 with libdav1d in the web and decode presets - #84
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (11)
🧰 Additional context used📚 Code guidelines (1)📓 Path-based instructions (2)Source excerpt: CI builds `web` and `decode` for both cores, checks that each core has the codecs its preset lists and none that it leaves out, and runs a short decode and encode (`scripts/preset-check.mjs`).📄 CodeRabbit inference engine (apps/website/docs/presets.md) Files:
Source excerpt: The preset files are `build/presets/.env`.📄 CodeRabbit inference engine (apps/website/docs/presets.md) Files:
🪛 Checkov (3.3.17)Dockerfile[low] 200-200: Ensure that COPY is used instead of ADD in Dockerfiles (CKV_DOCKER_4) [low] 1-267: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [low] 1-267: Ensure that a user for the container has been created (CKV_DOCKER_3) 🪛 Shellcheck (0.11.0)build/dav1d.sh[info] 6-6: Double quote to prevent globbing and word splitting. (SC2086) 🔇 Additional comments (2)
📝 SummarySummary by CodeRabbit
WalkthroughThe change adds an Emscripten build of dav1d 1.5.4 and enables AV1 decoding through libdav1d in the web and decode presets. It also adds an AV1 fixture check and updates documentation. ChangesAV1 decoding with dav1d
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant DockerBuild
participant Dav1dBuilder
participant FFmpegBuild
participant PresetChecker
DockerBuild->>Dav1dBuilder: build and install dav1d for Emscripten
DockerBuild->>FFmpegBuild: copy dav1d installation
FFmpegBuild->>FFmpegBuild: link dav1d in web and decode presets
PresetChecker->>FFmpegBuild: decode AV1 fixture with applicable presets
Merge Risk: ⚪ Minimal · up to No concrete issue is identified that blocks merging. The configured AV1 preset checks should run as part of normal CI validation. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new decoder expands media-processing exposure, but its source is commit-pinned, existing network restrictions remain, and the default published preset does not enable it. No introduced vulnerability was established. Decoder cleanup and recovery after malformed input or interruption remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 5 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit hears the AV1 stream, Comment |
![project516-review-bot[bot]](https://avatars.githubusercontent.com/in/4903871?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request adds AV1 decoding via libdav1d to the web and decode presets. It updates the Dockerfile to build libdav1d, modifies the preset environment files to enable the libdav1d decoder, and updates documentation and tests to reflect the change.
Not approved: the check on these comments could not run, so they are unverified.
Other notes
Dockerfile: The ADD command with a fragment (#$DAV1D_COMMIT) is not standard for Git over HTTP and may not fetch the desired commit. Instead, use the same pattern as other libraries in this Dockerfile: clone with the branch and then verify the commit. Replace this line and the subsequent RUN line (line 203) with:
RUN git clone --recursive -b $DAV1D_BRANCH https://code.videolan.org/videolan/dav1d.git /src &&
test "$(git -C /src rev-parse HEAD)" = "$DAV1D_COMMIT"
COPY build/meson-cross.ini /meson-cross.ini
COPY build/dav1d.sh /src/build.sh
RUN bash -x /src/build.sh
review-bot, model nvidia/nemotron-3-super-120b-a12b:free, verdict comment
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
c0f1055 to
4ccb6fa
Compare
There was a problem hiding this comment.
The PR integrates libdav1d 1.5.4 as an AV1 decoder for the web and decode presets via dedicated build stages and environment flags. Preset documentation is updated to reflect the new AV1 decoding capability, and the preset-check script gains an AV1 decode verification step using a generated test fixture.
review-bot, model nvidia/nemotron-3.5-lightning:free, verdict approve
What and why
Adds AV1 decoding through libdav1d to the
webanddecodepresets. FFmpeg's own AV1 decoder needs hardware acceleration, so without libdav1d no core can decode AV1 in software. Backported from upstream ffmpegwasm#982.build/dav1d.shandbuild/meson-cross.ini: dav1d 1.5.4 (commit 54706fc6bc0cdecab7e9593974a4039cc038fca7) built static with meson, asm and tools off, default bitdepths (8 and 16). Upstream's/meson-cross.iniis not in our tree, so the cross file is included here.dav1d-builderstage pinned withDAV1D_BRANCHandDAV1D_COMMIT(tag dereferenced viagit ls-remote), copied intoffmpeg-baselike every other library.web.envanddecode.env:--enable-libdav1d,-ldav1d, the libdav1d decoder (and the av1 parser inweb).scripts/check-doc-versions.mjsrow, CHANGELOG.scripts/preset-check.mjs(run by thebuild-presetjobs): both presets must list libdav1d and decodetests/fixtures/av1.webm. The fixture is the 1.5 KB, 4 frame, 64x64testsrcclip from upstream Decode AV1 with libdav1d; parity with native FFmpeg 9 on FATE ffmpegwasm/ffmpeg.wasm#982, encoded with SVT-AV1 (generated video, no third-party content).Threading
dav1d is built with its default pthread support for both cores. The st core forces
av_cpu_count()to 1, so libdav1d runs with one thread and spawns no workers; the mutex and condvar calls it still makes go through the existing fiber shim. The mt core gets real threads. This is untested here, so the st AV1 decode inpreset-checkis the check to watch.fullis unchanged on purposeThe published packages use
full, which neither enables nor links libdav1d. It follows after the size is measured. Library stages build for every preset in this repo, solibdav1d.alands in the shared install dir, but--disable-autodetectkeepsfullfrom using it.Size
Read the wasm delta for
webanddecodefrom thebuild-presetjob summaries (each prints a size row) and compare with the table inapps/website/docs/presets.md. I did not update that table.Not run locally
Nothing was built or run; this is a Raspberry Pi and core builds are CI-only. Only
node scripts/check-doc-versions.mjsran, and it passes.Noticed, not fixed
Review
review-bot's verification step failed on this head, and its Dockerfile note is answered in a comment. A Sonnet reviewer found nothing blocking. It checked the meson cross file, the 1.5.4 commit pin, and the st link (with av_cpu_count() at 1, libdav1d runs without threads). It also ran the AV1 fixture through a local ffmpeg: 4 frames decode. At its suggestion, the full preset check now fails if libdav1d is linked. Size cost from the first CI run, against the presets.md table: web st +0.74 MB wasm (+0.27 MB gzip), web mt +0.68 MB (+0.26 MB gzip), decode st +0.10 MB (+0.07 MB gzip).