Skip to content
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.

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

Native video support for AV1 #7557

Merged
merged 65 commits into from
Oct 7, 2024
Merged

Native video support for AV1 #7557

merged 65 commits into from
Oct 7, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Oct 1, 2024

What

What

Supports native decoding of AV1 videos.

Downsides: it is extremely slow in debug builds

In release builds it is ok, but there is still A LOT of performance on the table.

TODO before merging

  • Update MSRV to Rust 1.79 #7563
  • Re-add nasm to pixi.toml, because it is needed to compile rav1d
  • Make the asm-features of rav1d opt-in so users don't need nasm to compile rerun
  • Put it behind a feature flag, in case compiling rav1d is difficult on some platforms
  • Fix error handling so we don't crash on bad videos
  • Fix performance of debug builds, if possible
    • dav1d is always fast, but rav1d is super-slow in debug builds
  • Or show error message in debug builds
  • Review moved mp4 demux code to see nothing was lost

Proof

native-video.mp4

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

Copy link

github-actions bot commented Oct 1, 2024

Deployed docs

Commit Link
4ec11fa https://landing-9oxg78mpl-rerun.vercel.app/docs

@emilk emilk added 📺 re_viewer affects re_viewer itself 🎞️ video labels Oct 2, 2024
@emilk emilk changed the title Native AV1 decoding Native video support for AV1 Oct 4, 2024
@Wumpf Wumpf self-requested a review October 4, 2024 08:51
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

preliminary round or reviewing. excited about this!

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
crates/store/re_video/examples/frames.rs Show resolved Hide resolved
crates/store/re_video/examples/frames.rs Outdated Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
@@ -0,0 +1,268 @@
//! Video demultiplexing.
Copy link
Member

Choose a reason for hiding this comment

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

todo note to self: review this later again, checking what has moved / is new etc.. This stuff changed quite a bit so there's a risk of regressing progress, should keep an eye on this

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call!

Copy link
Member Author

Choose a reason for hiding this comment

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

I can make a separate PR where I just move the code on main, and then we can rebase and compare

crates/viewer/re_renderer/Cargo.toml Show resolved Hide resolved
crates/viewer/re_renderer/src/video/decoder/native_av1.rs Outdated Show resolved Hide resolved
crates/viewer/re_renderer/src/video/decoder/native_av1.rs Outdated Show resolved Hide resolved
@emilk emilk mentioned this pull request Oct 7, 2024
7 tasks
@emilk emilk marked this pull request as ready for review October 7, 2024 06:58
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

much simpler decoder now, nice!

Looking good, but I stick with my usual line of complaining about seqcst: Seqcst is a bad default and hard to deal with imho (Mara has a really nice section about that in her book if you want to learn more check https://marabos.nl/atomics/memory-ordering.html#common-misconceptions -> Myth: Sequentially consistent memory ordering is a great default and is always correct.)

crates/store/re_video/src/decode/av1.rs Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
crates/store/re_video/src/decode/av1.rs Outdated Show resolved Hide resolved
@emilk emilk merged commit 2ce1c82 into main Oct 7, 2024
39 checks passed
@emilk emilk deleted the emilk/native-av1-decode branch October 7, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants