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.

Sign up for GitHub

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

Jump to bottom

Add ffmpeg support in nvproxy #11321

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Add ffmpeg support in nvproxy #11321

merged 1 commit into from
Jan 9, 2025

Conversation

2022tgoel
Copy link
Contributor

No description provided.

@2022tgoel 2022tgoel marked this pull request as draft December 23, 2024 23:19
@google-cla Google CLA
Copy link

google-cla bot commented Dec 23, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@EtiennePerot
Copy link
Contributor

cc @ayushr2

@2022tgoel 2022tgoel marked this pull request as ready for review January 1, 2025 17:02
@EtiennePerot
Copy link
Contributor

Please notify here when this is ready for review (and after squashing all commits to one)

@2022tgoel
Copy link
Contributor Author

Hi, the PR is ready for review. I have verified that ffmpeg_test is successful on T4, A10, and L4 gpus.

EtiennePerot
EtiennePerot requested changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@EtiennePerot EtiennePerot left a comment

Choose a reason for hiding this comment

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

Thanks for the work!
Please reword the commit message to be more descriptive, especially the first line which should explain that this commit adds GPU video encoding/decoding support to nvproxy. With the current commit message, it would show up as only saying "add cap" in the commit log.

images/benchmarks/ffmpeg/Dockerfile Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/frontend.go Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/frontend.go Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/version.go Outdated Show resolved Hide resolved
@2022tgoel 2022tgoel force-pushed the nvproxy_video_cap branch 2 times, most recently from 7ec0b19 to 117632d Compare January 3, 2025 15:52
@2022tgoel 2022tgoel requested a review from EtiennePerot January 3, 2025 15:52
@EtiennePerot
Copy link
Contributor

Looks good to me now. cc @ayushr2 want to review too?

ayushr2
ayushr2 reviewed Jan 4, 2025
View reviewed changes
pkg/sentry/devices/nvproxy/frontend.go Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/version.go Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/version.go Outdated Show resolved Hide resolved
pkg/abi/nvgpu/ctrl.go Outdated Show resolved Hide resolved
pkg/abi/nvgpu/uvm.go Show resolved Hide resolved
@2022tgoel 2022tgoel requested a review from ayushr2 January 7, 2025 15:56
ayushr2
ayushr2 approved these changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@ayushr2 ayushr2 left a comment

Choose a reason for hiding this comment

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

Just nits. LGTM.

Thanks for your contribution, this is good work. Glad to see so many contributions from Modal.

P.S. please rebase to HEAD and resolve conflicts

pkg/sentry/devices/nvproxy/version.go Outdated Show resolved Hide resolved
pkg/sentry/devices/nvproxy/version.go Outdated Show resolved Hide resolved
@2022tgoel
Copy link
Contributor Author

I've rebased and fix the last few nits. Let me know if there's anything else!

@2022tgoel 2022tgoel force-pushed the nvproxy_video_cap branch 2 times, most recently from 5f2d914 to 80395f2 Compare January 8, 2025 19:37
copybara-service bot pushed a commit that referenced this pull request Jan 8, 2025
@2022tgoel @gvisor-bot
Add ffmpeg support in nvproxy
f071e7d 
FUTURE_COPYBARA_INTEGRATE_REVIEW=#11321 from 2022tgoel:nvproxy_video_cap 80395f2
PiperOrigin-RevId: 713389751
@copybara-service copybara-service bot mentioned this pull request Jan 8, 2025
Merged
@EtiennePerot
Copy link
Contributor

EtiennePerot commented Jan 8, 2025 •
edited
Loading

The tests ran and there are two failures:

Executing tests from //pkg/sentry/devices/nvproxy:nvproxy_test
-----------------------------------------------------------------------------
--- FAIL: TestFilterCapabilities (0.00s)
    --- FAIL: TestFilterCapabilities/compute,display,graphics,ngx,utility,video,compat32 (0.00s)
        nvproxy_test.go:320: uvmIoctlFilters("compute,display,graphics,ngx,utility,video,compat32") returned 30 UVM ioctls, expected 31
    --- FAIL: TestFilterCapabilities/compute,graphics,utility,video (0.00s)
        nvproxy_test.go:320: uvmIoctlFilters("compute,graphics,utility,video") returned 30 UVM ioctls, expected 31
    --- FAIL: TestFilterCapabilities/video (0.00s)
        nvproxy_test.go:320: uvmIoctlFilters("video") returned 2 UVM ioctls, expected 3
    --- FAIL: TestFilterCapabilities/utility,video (0.00s)
        nvproxy_test.go:320: uvmIoctlFilters("utility,video") returned 30 UVM ioctls, expected 31

and

Executing tests from //pkg/sentry/devices/nvproxy:nvproxy_driver_parity_test
-----------------------------------------------------------------------------
--- FAIL: TestSupportedStructNames (0.00s)
    --- FAIL: TestSupportedStructNames/550.54.15 (28.60s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.15"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.15"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.54.15"
    --- FAIL: TestSupportedStructNames/535.216.01 (29.27s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.216.01"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.216.01"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.216.01"
    --- FAIL: TestSupportedStructNames/535.183.06 (29.67s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.183.06"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.06"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.06"
    --- FAIL: TestSupportedStructNames/550.54.14 (30.43s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.14"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.54.14"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.14"
    --- FAIL: TestSupportedStructNames/560.35.03 (30.89s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "560.35.03"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "560.35.03"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "560.35.03"
    --- FAIL: TestSupportedStructNames/550.90.12 (31.28s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.12"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.90.12"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.12"
    --- FAIL: TestSupportedStructNames/550.90.07 (31.78s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.07"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.07"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.90.07"
    --- FAIL: TestSupportedStructNames/550.127.05 (31.79s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.127.05"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.127.05"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.127.05"
    --- FAIL: TestSupportedStructNames/535.183.01 (13.75s)
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.01"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.183.01"
        nvproxy_driver_parity_test.go:100: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.01"
--- FAIL: TestStructDefinitionParity (0.00s)
    --- FAIL: TestStructDefinitionParity/535.183.06 (13.83s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.06"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.183.06"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.06"
    --- FAIL: TestStructDefinitionParity/535.216.01 (13.84s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.216.01"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.216.01"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.216.01"
    --- FAIL: TestStructDefinitionParity/550.54.15 (14.50s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.15"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.15"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.54.15"
    --- FAIL: TestStructDefinitionParity/550.54.14 (14.62s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.54.14"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.14"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.54.14"
    --- FAIL: TestStructDefinitionParity/550.90.12 (14.82s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.12"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.12"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.90.12"
    --- FAIL: TestStructDefinitionParity/550.90.07 (14.94s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.90.07"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.07"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.90.07"
    --- FAIL: TestStructDefinitionParity/550.127.05 (15.05s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "550.127.05"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "550.127.05"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "550.127.05"
    --- FAIL: TestStructDefinitionParity/560.35.03 (15.59s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "560.35.03"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "560.35.03"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "560.35.03"
    --- FAIL: TestStructDefinitionParity/535.183.01 (13.15s)
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_FIFO_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.01"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_CMD_NVJPG_GET_CAPS_V2_PARAMS" not found in parser output for version "535.183.01"
        nvproxy_driver_parity_test.go:141: struct "NV0080_CTRL_GR_GET_CAPS_PARAMS_V2" not found in parser output for version "535.183.01"

For the //pkg/sentry/devices/nvproxy:nvproxy_test test failure, please check the TestFilterCapabilities test in nvproxy_test.go. It's possible that there's some missing ioctls in the seccomp ioctl filters, or that some ioctls need to be added to one of the four maps in nvproxy_test.go ({nonForwarded,nvproxyOnly}*Ioctls).

For the //pkg/sentry/devices/nvproxy:nvproxy_driver_parity_test test failure, please ensure the data returned by getStructs functions in version.go are in sync with the rest of the ABI definitions and that the structs are annotated with the structs of the same name in the NVIDIA driver source code.

You should be able to re-run these tests on your own with make test TARGETS=//pkg/sentry/devices/....

@2022tgoel
Add GPU video codecs support to nvproxy (so that tools like ffmpeg …
7399a32 
…work)

adding cap

tests work

fix merge

unit test

small fixes

additional ioctls for L4 gpu
@2022tgoel
Copy link
Contributor Author

Tests should pass now.

copybara-service bot pushed a commit that referenced this pull request Jan 9, 2025
@2022tgoel @gvisor-bot
Add ffmpeg support in nvproxy
55377e7 
FUTURE_COPYBARA_INTEGRATE_REVIEW=#11321 from 2022tgoel:nvproxy_video_cap 7399a32
PiperOrigin-RevId: 713389751
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2025
@2022tgoel @gvisor-bot
Add ffmpeg support in nvproxy
29a2567 
FUTURE_COPYBARA_INTEGRATE_REVIEW=#11321 from 2022tgoel:nvproxy_video_cap 7399a32
PiperOrigin-RevId: 713389751
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2025
@2022tgoel @gvisor-bot
Add ffmpeg support in nvproxy
de2b98d 
FUTURE_COPYBARA_INTEGRATE_REVIEW=#11321 from 2022tgoel:nvproxy_video_cap 7399a32
PiperOrigin-RevId: 713389751
copybara-service bot pushed a commit that referenced this pull request Jan 9, 2025
@2022tgoel @gvisor-bot
Add ffmpeg support in nvproxy
8561073 
FUTURE_COPYBARA_INTEGRATE_REVIEW=#11321 from 2022tgoel:nvproxy_video_cap 7399a32
PiperOrigin-RevId: 713389751
@copybara-service copybara-service bot merged commit c5c74f4 into google:master Jan 9, 2025
5 checks passed
@EtiennePerot
Copy link
Contributor

Thanks for contributing!

@ayushr2
Copy link
Collaborator

ayushr2 commented Jan 10, 2025

@2022tgoel I think TestFffmpegDecodeGPU and TestFffmpegEncodeGPU are failing now: https://buildkite.com/gvisor/release/builds/4200#01944cd9-e539-4698-bb43-0e624e68ad40

copybara-service bot pushed a commit that referenced this pull request Jan 14, 2025
@EtiennePerot @gvisor-bot
On COS, add NVIDIA library directory to LD configuration and update c…
be8196a 
…ache.

Unlike Ubuntu VMs where we use Docker's `--gpus` flag, COS VMs do not use
this flag and instead mount the NVIDIA library directories automatically.
However, nothing guarantees that these directories are added to the LD
config. This change fixes that. It take advantage of the fact that all GPU
tests have the sniffer binary as entrypoint, which slightly overloads the
role of the sniffer within the GPU test infrastructure... but then again
the ioctl sniffer is already deeply intertwined with ld configuration
because it already overrides the `ioctl` libc function, so this doesn't
seem like too big of a stretch.

This change makes the ffmpeg test succeed with `runc` on COS, but they still
fail with gVisor (with `CUDA_ERROR_OUT_OF_MEMORY` errors). So there must be
some further gVisor-specific error.

Updates #11351
Updates #11321

PiperOrigin-RevId: 715106144
@copybara-service copybara-service bot mentioned this pull request Jan 14, 2025
Merged
copybara-service bot pushed a commit that referenced this pull request Jan 14, 2025
@EtiennePerot @gvisor-bot
On COS, add NVIDIA library directory to LD configuration and update c…
3d6b429 
…ache.

Unlike Ubuntu VMs where we use Docker's `--gpus` flag, COS VMs do not use
this flag and instead mount the NVIDIA library directories automatically.
However, nothing guarantees that these directories are added to the LD
config. This change fixes that. It take advantage of the fact that all GPU
tests have the sniffer binary as entrypoint, which slightly overloads the
role of the sniffer within the GPU test infrastructure... but then again
the ioctl sniffer is already deeply intertwined with ld configuration
because it already overrides the `ioctl` libc function, so this doesn't
seem like too big of a stretch.

This change makes the ffmpeg test succeed with `runc` on COS, but they still
fail with gVisor (with `CUDA_ERROR_OUT_OF_MEMORY` errors). So there must be
some further gVisor-specific error.

Updates #11351
Updates #11321

PiperOrigin-RevId: 715106144
copybara-service bot pushed a commit that referenced this pull request Jan 14, 2025
@EtiennePerot @gvisor-bot
On COS, add NVIDIA library directory to LD configuration and update c…
3649ca9 
…ache.

Unlike Ubuntu VMs where we use Docker's `--gpus` flag, COS VMs do not use
this flag and instead mount the NVIDIA library directories automatically.
However, nothing guarantees that these directories are added to the LD
config. This change fixes that. It take advantage of the fact that all GPU
tests have the sniffer binary as entrypoint, which slightly overloads the
role of the sniffer within the GPU test infrastructure... but then again
the ioctl sniffer is already deeply intertwined with ld configuration
because it already overrides the `ioctl` libc function, so this doesn't
seem like too big of a stretch.

This change makes the ffmpeg test succeed with `runc` on COS, but they still
fail with gVisor (with `CUDA_ERROR_OUT_OF_MEMORY` errors). So there must be
some further gVisor-specific error.

Updates #11351
Updates #11321

PiperOrigin-RevId: 715222952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayushr2 ayushr2 ayushr2 approved these changes

@EtiennePerot EtiennePerot EtiennePerot approved these changes

Assignees
No one assigned
Labels
ready to pull
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@2022tgoel @EtiennePerot @ayushr2