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

Add developer CLI flags for tuning encoder internals #3047

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shssoichiro
Copy link
Collaborator

@shssoichiro shssoichiro commented Oct 25, 2022

Currently adds the following:

  • --deblock-strength
  • --deblock-sharpness
  • --ip-qidx-ratio
  • --temporal-rdo-strength

These flags will be available only if the devel feature flag is active, as they are intended to be used to ease development and not to be tweaked by end users. rav1e continues to subscribe to the philosophy that end users should not need to be cargo culting their command lines to get optimal results, and we would prefer to tune the encoder internals so that users do not have to. This change helps us do that.

@shssoichiro shssoichiro force-pushed the expose-options-2 branch 3 times, most recently from ed552c4 to 5fa188d Compare October 25, 2022 16:47
@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2022

Codecov Report

Base: 86.41% // Head: 86.36% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (f37702c) compared to base (f869e16).
Patch coverage: 92.55% of modified lines in pull request are covered.

❗ Current head f37702c differs from pull request most recent head bbd0291. Consider uploading reports for the commit bbd0291 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3047      +/-   ##
==========================================
- Coverage   86.41%   86.36%   -0.05%     
==========================================
  Files          84       83       -1     
  Lines       33158    33085      -73     
==========================================
- Hits        28652    28573      -79     
- Misses       4506     4512       +6     
Impacted Files Coverage Δ
src/bin/common.rs 54.85% <ø> (ø)
src/rate.rs 77.77% <89.83%> (+0.53%) ⬆️
src/encoder.rs 87.17% <92.85%> (+0.01%) ⬆️
src/api/config/encoder.rs 89.89% <100.00%> (+0.65%) ⬆️
src/api/test.rs 99.23% <100.00%> (+<0.01%) ⬆️
src/rdo.rs 85.79% <100.00%> (+0.55%) ⬆️
src/context/frame_header.rs 66.06% <0.00%> (-2.27%) ⬇️
src/context/mod.rs 91.57% <0.00%> (-1.06%) ⬇️
src/context/transform_unit.rs 89.78% <0.00%> (-0.72%) ⬇️
src/asm/x86/lrf.rs 93.48% <0.00%> (-0.64%) ⬇️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@BlueSwordM
Copy link
Contributor

BlueSwordM commented Oct 25, 2022

Everything seems good on my end, as all flags seem to work(except --ip-qidx-ratio as I haven't tested it yet).
I'd wait until tomorrow to merge it since there might be some more stuff that I would want to add.

@shssoichiro
Copy link
Collaborator Author

What is wrong with ip-qidx-ratio?

@BlueSwordM
Copy link
Contributor

I should have been clearer: I haven't tested it yet :P

@shssoichiro
Copy link
Collaborator Author

Okay, well I will tell you from my testing in AWCY... It doesn't seem to work. :p at least it did not result in the output changing, which means it did not do what I wanted it to do.

src/encoder.rs Outdated Show resolved Hide resolved
Currently adds the following:
- `--deblock-strength`
- `--deblock-sharpness`
- `--ip-ratio`
- `--pb-ratio`
- `--b-ratio`
- `--temporal-rdo-strength`

These flags will be available only if the `devel` feature flag
is active, as they are intended to be used to ease development
and not to be tweaked by end users. rav1e continues to subscribe
to the philosophy that end users should not need to be cargo culting
their command lines to get optimal results, and we would prefer to
tune the encoder internals so that users do not have to.
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