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 photon noise based grain synthesis #2924

Merged
merged 1 commit into from Apr 29, 2022

Conversation

shssoichiro
Copy link
Collaborator

@shssoichiro shssoichiro commented Apr 12, 2022

This changeset adds two new command line options:

--photon-noise allows the user to specify a strength from 0-64 (0 = disabled) to enable grain synthesis in rav1e. This implementation of grain synthesis is based on the photon noise tables used by aomenc and SVT-AV1. rav1e internally generates a noise table based on the user's strength setting, the video resolution, and whether the video is HDR, and then applies it to the encode. This implementation does not do anything such as denoising, or matching grain to the amount of grain in the source. The amount of grain is based on the luma values of each block. Many users of the photon noise tables in aomenc have said that they prefer the way this looks over aomenc's traditional grain synth method.

--photon-noise-table is a more advanced version which allows the user to manually specify a path to a film grain table. These tables can be generated by aomenc's photon_noise_table tool, or by av1an. This allows the user to have more customization over the grain synthesis, enabling features such as chroma grain.

The recommended option for most users will be to use the simpler --photon-noise. It is disabled by default.

The patch has virtually no speed impact, and adds an overhead of about 40 bytes per frame with luma-only synth, or 90 bytes per frame with luma and chroma synth.

For now, this changeset will be behind the unstable feature flag, until denoising is implemented.

Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

Hi, thanks for this,
Quick comment, license headers is missing for few files^^

@BlueSwordM
Copy link
Contributor

Very nice patch overall.

Certainly a great step forward, especially since photon noise grain synth has been working quite well for me and many people.

While a complete excellent grain synth implementation would be the best, having this is great honestly.

The grain synth implementations in aomenc and SVT-AV1 are currently sub-optimal. Tuned photon noise, while a bit less adaptive, has far more consistent and better performance in general. SVT-AV1 is certainly better than aomenc, but after a lot of experimentation, not that far ahead of aomenc honestly.

There's also the advantage of having no runtime performance loss over even a very optimized implementation.

@shssoichiro shssoichiro force-pushed the grain-tables branch 2 times, most recently from fc19e6a to 53d6c08 Compare April 12, 2022 19:07
@shssoichiro
Copy link
Collaborator Author

I decided that I don't want to inflate the size of this patch, so in order to keep it relatively small, I've moved the new code behind the unstable feature flag, and will make a separate changeset to add denoising (and move photon noise to stable at that time).

@shssoichiro shssoichiro force-pushed the grain-tables branch 2 times, most recently from 6f418b2 to e34345d Compare April 14, 2022 19:22
@shssoichiro shssoichiro force-pushed the grain-tables branch 4 times, most recently from ee0509c to 66e8372 Compare April 26, 2022 01:54
Copy link
Collaborator

@tdaede tdaede left a comment

Choose a reason for hiding this comment

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

Looks fine.

FWIW I wouldn't even be opposed to exposing this more easily than unstable if that makes testing with av1an's film grain tools better, as long as we understand what those are.

@@ -52,7 +52,7 @@ impl<T: Pixel> SceneChange<T> {
let seq = Arc::new(Sequence::new(enc));

let detector = SceneChangeDetector::new(
*enc,
enc.clone(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why this switched to being a clone().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately there needed to be a field added for the film grain settings which is a Vec, which prevents it from being copy-able.

@shssoichiro
Copy link
Collaborator Author

Re. this moving out of unstable: I have a branch for denoising which is mostly complete, which will satisfy everyone's concerns and allow this to move to the stable featureset.

@tdaede
Copy link
Collaborator

tdaede commented Apr 29, 2022

OK. (Could this be an ArrayVec like is used by other parts of the code? Maybe we don't want ArrayVec in the public api?)

@shssoichiro
Copy link
Collaborator Author

A user could have a table with any number of different film grain segments, so unfortunately a Vec is needed. (Also, ArrayVec is unfortunately also !Copy)

@shssoichiro shssoichiro merged commit 202e645 into xiph:master Apr 29, 2022
@shssoichiro shssoichiro deleted the grain-tables branch April 29, 2022 19:42
@barrbrain barrbrain added this to Done in Release 0.6 Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants