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

Create hardware device context with flags #94

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

l0rem1psum
Copy link
Contributor

Despite the official doc claiming that the flags are not used (see: hwcontext.c File Reference), it is in reality used by AVCUDADeviceContext (see: hwcontext_cuda.h).

@asticode
Copy link
Owner

asticode commented Nov 4, 2024

I've added flags this way in the lib (example is for codec context flags):

We should do the same thing for the new flags you're mentioning.

It would mean:

  • create a HardwareDeviceContextFlag type in a new hardware_device_context_flag.go file with both AV_CUDA_USE_PRIMARY_CONTEXT and AV_CUDA_USE_CURRENT_CONTEXT as possible values with go constants named HardwareDeviceContextFlagCudaUsePrimaryContext and HardwareDeviceContextFlagCudaUseCurrentContext
  • modify the generate flags command to generate a HardwareDeviceContextFlags struct and run it to actually add it to flags.go
  • use HardwareDeviceContextFlags instead of int in CreateHardwareDeviceContext

@l0rem1psum
Copy link
Contributor Author

l0rem1psum commented Nov 5, 2024

Thanks for the reply. My main concern is such a flag (AV_CUDA_USE_CURRENT_CONTEXT in this case) wouldn't be available to users who didn't build ffmpeg with CUDA, rendering this binding strongly coupled to a particular hardware device.

As such, I still strongly recommend using a int flag and avoid adding custom types not available in the C code. (And avoid adding additional logic in FFI bindings in general.)

edit: It still needs to be verified whether ffmpeg without CUDA has those #defines, but the same logic applies.

@l0rem1psum
Copy link
Contributor Author

I just added the suggested code but it wouldn't compile without linking CUDA. Hence my above argument still stands.

@asticode asticode merged commit cfdc7f1 into asticode:master Nov 5, 2024
3 checks passed
@asticode
Copy link
Owner

asticode commented Nov 5, 2024

You're right, unfortunately I don't see a workaround for this 🤔

Thanks for this PR too! ❤️

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.

2 participants