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

Nvidia linux. Add encoder nvenc #906

Merged
merged 18 commits into from Jan 15, 2022
Merged

Nvidia linux. Add encoder nvenc #906

merged 18 commits into from Jan 15, 2022

Conversation

Toxblh
Copy link
Contributor

@Toxblh Toxblh commented Jan 3, 2022

Added NVenc encoder for Linux.

The current solution is to implement a loop to get the Vulkan frame and copy that through the CPU to the GPU again.
Current latency in my case is in the range of 5-10ms for encoding. But that is already playable quality and latency.

I introduce a new flag --no-nvidia to build in dev mode for people with Intel/AMD cards, which is not required to install Nvidia libs to their machines. The flag should pass together with --bundle-ffmpeg like example cargo xtask build-server --bundle-ffmpeg --no-nvidia. But if you will link to the existing FFmpeg on your machine it's don't need to provide at all, it's only like a modification flag for --bundle-ffmpeg.

For devs with Arch need to create a symbolic link from /opt/cuda to /usr/local/cuda because current scripts are prepared to be built in CI for the final release.

CEncdoder is a change from waiting for an answer to skipping to sending the current frame because nvenc is required more than one frame to encode. And PushFrame returns try_again error till the next frames what it created infinity loop of waiting.

AppImage with nvenc included
ALVR-x86_64.zip

Screenshot from 2022-01-03 21-36-47

@Toxblh
Copy link
Contributor Author

Toxblh commented Jan 3, 2022

alvr_server_linux.zip

alvr/xtask/src/dependencies.rs Outdated Show resolved Hide resolved
packaging/alvr_build_linux.sh Outdated Show resolved Hide resolved
@Incuh
Copy link
Contributor

Incuh commented Jan 3, 2022

On Arch, all of the cuda tools are in /opt/cuda/bin & lib64 and cannot be found when bundling ffmpeg.

Can someone verify if that's also the case for other distros.

@Incuh
Copy link
Contributor

Incuh commented Jan 3, 2022

A symbolic link from /opt/cuda to /usr/local/cuda seems to work

@ckiee ckiee marked this pull request as draft January 5, 2022 04:01
@ckiee ckiee changed the title [WIP] Nvidia linux. Add encoder nvenc Nvidia linux. Add encoder nvenc Jan 5, 2022
@Toxblh Toxblh marked this pull request as ready for review January 6, 2022 01:11
@Toxblh Toxblh requested a review from xytovl January 6, 2022 04:11
Copy link
Collaborator

@xytovl xytovl left a comment

Choose a reason for hiding this comment

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

All comments are pretty minor, the PR seems good overall

alvr/server/cpp/platform/linux/EncodePipelineNvEnc.cpp Outdated Show resolved Hide resolved
alvr/xtask/src/main.rs Outdated Show resolved Hide resolved
Copy link
Member

@ckiee ckiee left a comment

Choose a reason for hiding this comment

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

Nitpicks and grammar, I'll commit some fixes then hopefully merge.

alvr/server/cpp/platform/linux/EncodePipelineNvEnc.cpp Outdated Show resolved Hide resolved
alvr/xtask/src/dependencies.rs Show resolved Hide resolved
alvr/xtask/src/dependencies.rs Outdated Show resolved Hide resolved
alvr/xtask/src/main.rs Outdated Show resolved Hide resolved
alvr/xtask/src/main.rs Outdated Show resolved Hide resolved
@ckiee
Copy link
Member

ckiee commented Jan 11, 2022

Rebased from master to fix merge conflict so CI runs.

@Toxblh Toxblh requested a review from ckiee January 12, 2022 23:36
@Toxblh Toxblh requested a review from xytovl January 12, 2022 23:52
@xytovl
Copy link
Collaborator

xytovl commented Jan 13, 2022

I see you have added packaging/rpm/cuda.pc, this should not be needed at all. It is the responsibility of the distribution to ship the file with a cuda package.
Adding it here will most probably breaks compile for those who have cuda in a different location, such as /opt.

@Toxblh
Copy link
Contributor Author

Toxblh commented Jan 13, 2022

I see you have added packaging/rpm/cuda.pc, this should not be needed at all. It is the responsibility of the distribution to ship the file with a cuda package.
Adding it here will most probably breaks compile for those who have cuda in a different location, such as /opt.

For fedora we don’t have a package, need to install from nvidia site. This don’t use right now, I put here as example the file, which will require for smooth dev process

@xytovl
Copy link
Collaborator

xytovl commented Jan 13, 2022

I see you have added packaging/rpm/cuda.pc, this should not be needed at all. It is the responsibility of the distribution to ship the file with a cuda package.
Adding it here will most probably breaks compile for those who have cuda in a different location, such as /opt.

For fedora we don’t have a package, need to install from nvidia site. This don’t use right now, I put here as example the file, which will require for smooth dev process

Fine for me then.

@Toxblh Toxblh requested a review from Trae32566 January 13, 2022 11:27
@ckiee ckiee merged commit cdc8dc8 into alvr-org:master Jan 15, 2022
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.

None yet

5 participants