obs-studio icon indicating copy to clipboard operation
obs-studio copied to clipboard

obs-ffmpeg: Replace external/AMF folder by obs-deps headers

Open tytan652 opened this issue 3 years ago • 5 comments

Description

Depends on:

  • https://github.com/obsproject/obs-deps/pull/152

Remove external/AMF headers folder from obs-ffmpeg and use obs-deps headers.

Difference between upstream and "external":

  • "external" have outdated code compared to 1.4.28 from upstream
  • This enum value that is not present upstream and is unused in OBS Studio whole repo (the header itself is not used), so it's removal is seamless.

How ?: I pasted 1.4.28 over our "external" version and each diff (except the enum value) happen to be new code because if I blame upstream code and check the prior code, the prior code match "external".

And AMF_HQ_SCALER_ALGORITHM_FSR1_1 looks completely unused when I search in OBS Studio repo locally.

Motivation and Context

Cleanup

How Has This Been Tested?

Build on my Windows VM and on CI.

Will may need ~~NVENC and~~ AMF encoder tests.

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

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

tytan652 avatar Aug 03 '22 12:08 tytan652

Restored Nvidia's headers to not conflict with #7430.

And until obs-deps can have up to date Nvidia headers, the external folder will be kept for Nvenc encoders.

tytan652 avatar Sep 22 '22 10:09 tytan652

Note: Put as draft until AMD release their updated headers with AV1.

tytan652 avatar Nov 22 '22 08:11 tytan652

We can use AMF 1.4.28, but kept as draft until this https://github.com/obsproject/obs-deps/pull/152 is merged and obs-deps have a new release.

tytan652 avatar Jan 17 '23 14:01 tytan652

Also I added a finder for AMF with version.

tytan652 avatar Jan 17 '23 15:01 tytan652

Fixed AMF_VERSION_ values not being set properly. (So just nitpicking)

tytan652 avatar Jan 17 '23 19:01 tytan652

Is the Finder for all platforms, or just some platforms? I was able to build just the first commit on Windows seemingly fine. Does the Finder need to be introduced first, or would that cause problems?

RytoEX avatar Mar 07 '23 19:03 RytoEX

Is the Finder for all platforms, or just some platforms? I was able to build just the first commit on Windows seemingly fine. Does the Finder need to be introduced first, or would that cause problems?

Also answered off-thread, the finder is not required because the required include directory is already added. But adding it allows to check if AMF is really present and check the version.

tytan652 avatar Mar 07 '23 19:03 tytan652