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

nv-filters: Update for new SDK #11814

Merged
merged 2 commits into from
Feb 5, 2025
Merged

nv-filters: Update for new SDK #11814

merged 2 commits into from
Feb 5, 2025
+3 −21

Conversation

pkviet
Copy link
Member

@pkviet pkviet commented Feb 1, 2025 •
edited by RytoEX
Loading

Description

This PR does two things:

  • updates the checks against the new redistributables of NVIDIA AUDIO & VIDEO effects to:

    • 0.7.6. for Video Effects;
    • 1.6.1.2 for Audio Effects.

  • cleans up the code related with cuda RT functions, which is not used anymore.

Motivation and Context

Allow nv-filters to work with newer SDK.

Fixes #11813

How Has This Been Tested?

The video & audio effects load fine on windows 11 PRO 24H2.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@pkviet pkviet added Seeking Testers Build artifacts on CI Bug Fix Non-breaking change which fixes an issue labels Feb 1, 2025
@pkviet pkviet added this to the OBS Studio 31.1 milestone Feb 1, 2025
@pkviet pkviet mentioned this pull request Feb 1, 2025
Closed
@pkviet pkviet added the Code Cleanup Non-breaking change which makes code smaller or more readable label Feb 1, 2025
@Lain-B
Copy link
Collaborator

Lain-B commented Feb 2, 2025

@derrod Seeing as you were the one who implemented cuda, this look okay to you? Seems fine to me but I never touhed the cuda stuff when I originally implemented it.

@Lain-B Lain-B requested a review from derrod February 2, 2025 21:05
@pkviet
Copy link
Member Author

pkviet commented Feb 2, 2025 •
edited
Loading

@derrod Seeing as you were the one who implemented cuda, this look okay to you? Seems fine to me but I never touhed the cuda stuff when I originally implemented it.

Hi Lain, for clarification, the CudaRT functions in nv-filters were not introduced by @derrod .
They're a leftover from earlier code I wrote , that I later removed. But I forgot to remove the cudaRT symbol loading.

RytoEX
RytoEX requested changes Feb 4, 2025
View reviewed changes
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

On the commit messages:

  1. A bulleted list containing sentence fragments does not need punctuation (;, .).
  2. Add a blank line after a list.
  3. Use versions are.
  4. smaller -> lower
  5. any more -> any or we don't use CUDA RT functions anymore
  6. , so this or This
  7. cuda -> CUDA
  8. cudaRT -> CUDART or CUDA RT
  9. dll -> DLL
  10. fixes also -> also fixes

Seems fine otherwise, as long as we aren't using CUDA RT and have no plans to do so.

@pkviet
nv-filters: Update SDK version targeted
e515d29 
This updates checks against current SDK redist versions:
- 0.7.6 (Video Effects)
- 1.6.1.2 (Audio Effects)

Users will receive a warning if their redist versions are lower than
these.

Signed-off-by: pkv <[email protected]>
@pkviet
Copy link
Member Author

pkviet commented Feb 4, 2025

Fixes for the wording of commits done.
@RytoEX

@RytoEX
Copy link
Member

RytoEX commented Feb 4, 2025

Fixes for the wording of commits done.

We don't use any more CUDA RT functions.

This was left, but it's not blocking.

@pkviet
nv-filters: Remove CUDA RT functions
d91e417 
We don't use CUDA RT functions anymore.
So this commit removes their loading from the CUDA RT DLL.
This also fixes a crash when loading different versions of CUDA RT when
the SDK is updated [1].

[1] obsproject#11813

Signed-off-by: pkv <[email protected]>
@pkviet
Copy link
Member Author

pkviet commented Feb 4, 2025

Fixes for the wording of commits done.

We don't use any more CUDA RT functions.

This was left, but it's not blocking.

ah sorry; pushed and fixed.

RytoEX
RytoEX approved these changes Feb 4, 2025
View reviewed changes
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Seems fine.

@RytoEX RytoEX changed the title nv-filters: Updates for new SDK nv-filters: Update for new SDK Feb 4, 2025
@RytoEX RytoEX merged commit d1249cd into obsproject:master Feb 5, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RytoEX RytoEX RytoEX approved these changes

@derrod derrod Awaiting requested review from derrod

Assignees
No one assigned
Labels
Bug Fix Non-breaking change which fixes an issue Code Cleanup Non-breaking change which makes code smaller or more readable Seeking Testers Build artifacts on CI
Projects
None yet
Milestone
OBS Studio 31.0
Development

Successfully merging this pull request may close these issues.

Nvidia VFX Filters cannot be loaded
3 participants
@pkviet @Lain-B @RytoEX