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

Implement pipeless scene detection for Vapoursynth inputs #844

Merged
merged 1 commit into from
May 22, 2024

Conversation

shssoichiro
Copy link
Collaborator

See rust-av/av-scenechange#168

By avoiding piping, we gain significant performance improvements. The amount of improvement varies depending on the source video format, as this moves the bottleneck to be on the decoder, but tests showed anywhere from 20-75% improvement on 8-bit for the fast scenechange method, with smaller but still respectable improvements of up to 20% for 10-bit and for the standard scenechange method, compared to piping from vspipe.

Currently, this implementation is limited to Vapoursynth inputs, and does not work when using ffmpeg filters through av1an (this will fall back to using pipes).

@shssoichiro shssoichiro changed the title Implement zerocopy scene detection for Vapoursynth inputs Implement pipeless scene detection for Vapoursynth inputs May 22, 2024
@redzic
Copy link
Collaborator

redzic commented May 22, 2024

Looks good although I believe this isn't fully zero copy. My old branch was using some unsafe trickery to bypass calling copy_from_raw_u8 in av-scenechange when constructing Frame. However this is still a big improvement over the current code. Perhaps an ideal solution would be to have a separate crate outside of rav1e (so it doesn't have frame padding requirements) for scene detection which is able to take references to frame data directly, instead of either owned frames or unsafe trickery.

IIRC the luma planes were never read for scene detection, so another easy improvement would perhaps be to avoid copying the chroma planes in av-scenechange for the purposes of scene detection and just fill those values with empty planes instead.

@shssoichiro
Copy link
Collaborator Author

shssoichiro commented May 22, 2024

I'll have to check on that last part, that's a smart idea if true... I'm sure it is for the fast method, I'll have to check if chroma is ignored for the standard method as well.

And yeah, this isn't truly zero copy, but avoiding the pipe is a large improvement. Unfortunately there's not currently a way to do the standard scene change method without padding, because rav1e requires padding to do its motion estimation (part of the inter cost estimation process). We would need to rework rav1e's motion estimation to not require padding, which is something I've looked into before but it's more complicated than it sounds because there's a lot of different pieces to it.

@shssoichiro
Copy link
Collaborator Author

Merging this for now, I'll make a second PR for the chroma planes change if it looks like it'll work.

@shssoichiro shssoichiro merged commit f259c44 into master-of-zen:master May 22, 2024
5 checks passed
@shssoichiro shssoichiro deleted the faster-scenechange branch May 22, 2024 12:41
@redzic
Copy link
Collaborator

redzic commented May 22, 2024

We would need to rework rav1e's motion estimation to not require padding, which is something I've looked into before but it's more complicated than it sounds because there's a lot of different pieces to it.

The solution I had to this on my old PR with non-VS inputs (ie regular videos) was to configure the padding on the FFmpeg-decoded frames so rav1e has no issue with them. It was actually quite simple to do with FFmpeg, but I never found any documentation to do that with Vapoursynth.

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.

3 participants