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

Support AV1 encoding using AMF on Windows #1967

Merged
merged 9 commits into from
Feb 12, 2024
Merged

Conversation

barnabwhy
Copy link
Contributor

It seems to be working nicely from my testing with a Quest 3 and RX 7700XT.

There's a couple of changes to settings, namely:

  • Removed "(VAAPI only)" from AV1 codec name.
  • Renamed "Enable VBAQ" to "Enable VBAQ/AQ" and added help text explaining that it's AQ on AV1 and VBAQ on the other two.

With this I also updated AMF to v1.4.33, which may bring some improvements elsewhere. (It may be worth looking into the "New AVC / HEVC encoder rate control methods" added in v1.4.28)

@barnabwhy barnabwhy requested a review from Vixea January 26, 2024 23:04
@zmerp
Copy link
Member

zmerp commented Jan 30, 2024

I think this PR should be split between AMF headers upgrade and the rest, because the changes are hard to navigate.

@phobos2077
Copy link

I think this PR should be split between AMF headers upgrade and the rest, because the changes are hard to navigate.

IMO it's not hard to navigate. Why would you split PR if it all related? Unless the headers upgrade is not required for the rest.

@zmerp
Copy link
Member

zmerp commented Jan 30, 2024

If anything else, i'd like to split the PR because of the "single concern" principle.

@Vixea
Copy link
Collaborator

Vixea commented Feb 2, 2024

^ it would be much easier to figure out what the issue is if an upgrade of the headers happens to cause issues however unlikely

Copy link
Collaborator

@Vixea Vixea left a comment

Choose a reason for hiding this comment

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

I changed my mind though I would like a warning when a user enables pre-analysis because it incurs a latency penalty... something you generally don't want in a realtime vr driver.

@barnabwhy barnabwhy mentioned this pull request Feb 5, 2024
@zmerp
Copy link
Member

zmerp commented Feb 10, 2024

I think the PR can be rebased @barnabwhy

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

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

LGTM

@zmerp zmerp merged commit 0ab3cb6 into alvr-org:master Feb 12, 2024
6 checks passed
@MommasLittleHelper
Copy link

Does AV1 also work on Windows with Nvidia cards? 🤔

@zmerp
Copy link
Member

zmerp commented Apr 19, 2024

@MommasLittleHelper Noone contributed that yet

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.

5 participants