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

Integrate AV-Metrics #2184

Merged
merged 1 commit into from
Mar 29, 2020
Merged

Integrate AV-Metrics #2184

merged 1 commit into from
Mar 29, 2020

Conversation

vibhoothi
Copy link
Collaborator

@vibhoothi vibhoothi commented Feb 25, 2020

This patchset introduces av-metrics to rav1e, so after encoding we can get PNSR, PSNR-HVS, CIDE2000, SSIM etc.

Things to do:

  • Adapt [WIP] Add metrics from AWCY into rav1e #1589
  • Build using rav1e
  • Fix panics
  • Test with different metrics
    • PSNR
    • PSNR-HVS
    • APSNR
    • CIEDE2000
    • SSIM
    • MS-SSIM
  • Cleanup
  • Use upsteam for
    • av-metrics
    • v-frame
  • Squash changes
  • Implement CLI alone without touching API changes

This PR Closes #1762

@rzumer rzumer added the WorkInProgress Incomplete patchset label Feb 25, 2020
Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

Might be a good idea to remove from the encoder the psnr feature and keep it only in the command line.

We should discuss it today.

src/api/util.rs Outdated Show resolved Hide resolved
src/bin/common.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

This is an API breaking change, albeit small.

Cargo.toml Outdated Show resolved Hide resolved
src/api/config.rs Outdated Show resolved Hide resolved
src/api/internal.rs Outdated Show resolved Hide resolved
src/bin/common.rs Outdated Show resolved Hide resolved
src/bin/stats.rs Outdated Show resolved Hide resolved
@vibhoothi
Copy link
Collaborator Author

vibhoothi commented Mar 6, 2020

Currently, it works without touching/calculating inside API. It is done in rav1e binary's encode_loop.

Mering depends on a few factors and PRs

Needs a clean up after the above things to be merged

@vibhoothi vibhoothi changed the title WIP: Integrate AV-Metrics Integrate AV-Metrics Mar 6, 2020
@vibhoothi vibhoothi requested a review from lu-zero March 6, 2020 15:36
src/api/internal.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

It requires some changes and some discussion.

Cargo.toml Outdated Show resolved Hide resolved
src/api/config.rs Show resolved Hide resolved
src/api/internal.rs Outdated Show resolved Hide resolved
src/api/util.rs Show resolved Hide resolved
@vibhoothi vibhoothi force-pushed the av-metrics-2 branch 4 times, most recently from c4c4df2 to a621683 Compare March 17, 2020 18:32
@vibhoothi vibhoothi requested a review from lu-zero March 18, 2020 07:28
Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

There are some issue to address. Beside that I'm fine with it.

src/bin/rav1e.rs Outdated Show resolved Hide resolved
src/bin/rav1e.rs Outdated Show resolved Hide resolved
src/bin/stats.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/bin/rav1e.rs Outdated Show resolved Hide resolved
src/bin/stats.rs Outdated Show resolved Hide resolved
src/bin/stats.rs Outdated Show resolved Hide resolved
@vibhoothi vibhoothi requested a review from lu-zero March 18, 2020 15:06
@vibhoothi vibhoothi force-pushed the av-metrics-2 branch 2 times, most recently from a23b6e0 to fa100d1 Compare March 18, 2020 16:32
@vibhoothi vibhoothi requested a review from lu-zero March 19, 2020 07:54
src/bin/stats.rs Outdated Show resolved Hide resolved
@vibhoothi vibhoothi force-pushed the av-metrics-2 branch 2 times, most recently from 8c33be2 to f449299 Compare March 19, 2020 13:50
@lu-zero
Copy link
Collaborator

lu-zero commented Mar 19, 2020

@sdroege can you confirm this is not going to break the gst plugin?

src/bin/rav1e.rs Outdated Show resolved Hide resolved
src/bin/stats.rs Outdated Show resolved Hide resolved
@shssoichiro shssoichiro dismissed their stale review March 19, 2020 15:14

Changes addressed, waiting for gst review before approving

@vibhoothi vibhoothi force-pushed the av-metrics-2 branch 3 times, most recently from 0e935d1 to 3232c16 Compare March 24, 2020 21:32
shssoichiro
shssoichiro previously approved these changes Mar 25, 2020
This commit introduces:
- Frame Metrics function to calculate
PSNR, PSNR-HVS, SSIM, MS-SSIM, CIEDE2000.
- Quality Metrics Structure to store all the calculated values
- Adding METRICS as an argument
- Updates --psnr calculation based on av-metrics
- Show calculated metrics in a neat way
- Introduce metrics_cli as an additional parameter for
process_frame as parse_cli is an expensive function to be made in
encode_loop, to make it more efficient we have moved metrics_cli
enum as an argument to process_frame and making the calculation
in do_encode function.
@sdroege
Copy link
Contributor

sdroege commented Mar 25, 2020

@sdroege can you confirm this is not going to break the gst plugin?

Compiles and tests are still passing too. Is there anything specific you want me to test otherwise?

Also don't worry about breaking API for the GStreamer plugin at least, I'm fine with updating regularly as long as you follow semver :)

Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

It seems good to me as well.

@vibhoothi vibhoothi merged commit 81b25d8 into xiph:master Mar 29, 2020
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.

Integrate av-metrics to rav1e CLI
6 participants