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

mac-vth264: Add GPU name to hardware encoder names

Open gxalpha opened this issue 2 years ago • 9 comments

Description

Adds the name of the GPU to the hardware encoder if its RegistryID is exposed by VideoToolbox (which it usually is for dedicated GPUs, but not integrated ones).

This makes it possible to more easily differentiate between the different "Apple VT H264 Hardware Encoder"s, which wasn't really possible before.

With a dedicated RX580 and an Intel chip with iGPU, it now looks like this: Bildschirmfoto 2021-10-18 um 00 27 29

Motivation and Context

Right now, there really isn't a way to tell the GPUs apart: image Fixes #4611

How Has This Been Tested?

macOS 12.0 beta 10 (Intel), compiled and run. As you can see, encoders with dedicated GPUs will get shown with the name of the GPU.

Apparently, the Apple T2 chip has encoding capabilities as well. As I don't have one on my Mac, it would be good if someone who does could test this and see whether it gets shown.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Tweak (non-breaking change to improve existing functionality)

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.
  • [x] 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.

This PR is split into two commits to keep git from thinking I deleted a file and created a new one.

gxalpha avatar Oct 17 '21 23:10 gxalpha

Built on M1 Mac Mini without an external GPU, and confirmed that internal hardware is not named.

image

WizardCM avatar Oct 18 '21 00:10 WizardCM

Built this on my 2019 Mac Pro, equipped with the Radeon Pro W6800X Duo. That graphics card has, as the name implies: two discrete GPU chips on it. The Mac Pro also has the (accursed) T2. Results are ... uh ... interesting?

encoder

jasonmvp avatar Oct 18 '21 13:10 jasonmvp

So which one is the T2? And why did code yield the name of the Radeon for it? 😂 If you don't want to look at code you might be able to check the actual IDs by selecting one, saving the settings and then looking at the ini file inside Application Support to check which ID was written to file.

PatTheMav avatar Oct 18 '21 18:10 PatTheMav

saving the settings and then looking at the ini file inside Application Support to check which ID was written to file.

No clue which one is the T2. :-) That's been the challenge for the last while once the change was made (last year?) to list out the encoders based on what MacOS reports. One of the guys on the Discord channel wrote me a quick test app to list out all of the encoders, and:

Name: Apple H.264 (HW)
Display Name: Apple H.264 (HW)
Id: com.apple.videotoolbox.videoencoder.h264.gva.100000bb4
=========================
Name: Apple H.264 (HW)
Display Name: Apple H.264 (HW)
Id: com.apple.videotoolbox.videoencoder.h264.gva
=========================
Name: Apple H.264 (HW)
Display Name: Apple H.264 (HW)
Id: com.apple.videotoolbox.videoencoder.h264.gva.100000bb2

There are more, of course. Those are just the hardware H.264 ones. OBS shows the same set in the .ini file.

jasonmvp avatar Oct 18 '21 18:10 jasonmvp

Just for tracking here: Discussion in Discord showed that

  • most likely, the T2 encoder does not show up and is not in this list
  • the GPURegistryIDs for two of the three encoder is the same, meaning the encoders are from the same GPU. One of them has as encoder ID just com.apple.videotoolbox.videoencoder.h264.gva, with the other one having a suffix. The ID of the first one is the one usually assigned to the QuickSync encoder. However the machine in question doesn't have QuickSync (it's a Xeon). My guess is that VT assigns the first encoder it has to that ID (usually that's QuickSync), and then iterates through all GPU encoders giving them the IDs with suffixes. QuickSync perhaps not being seen as such an encoder (since its not from a dedicated GPU or something) doesn't appear in that list, while encoders of dedicated GPUs do, thus creating the duplicate. If my theory is true, I would argue that while being very strange, this is not on OBS to fix. It could be done (checking if an encoder with the GPURegistryID already exists before registering it in OBS - Keep in mind this could cause peoples setups to break), but I'm not sure it's worth the hassle.

gxalpha avatar Oct 18 '21 23:10 gxalpha

If my theory is true, I would argue that while being very strange, this is not on OBS to fix. It could be done (checking if an encoder with the GPURegistryID already exists before registering it in OBS - Keep in mind this could cause peoples setups to break), but I'm not sure it's worth the hassle.

I'd argue this is most appropriate. Setups with dGPUs operate differently from those without, as well as those with a dGPU but no iGPU.

It also should only cause minimal breakage; OBS forgets my GPU on the regular anyhow (iGPU + eGPU), and it would be a one-time fix to go in and reset the encoder settings for those whose are lost. OBS has done similar breaking changes as needed before (e.g. changes to the color correction filter)

Alternatively, you could, in theory, "map" one encoder to the other, so it only shows one option in the drop down, masking the fact there are 2 options internally.

MisutaaAsriel avatar Mar 04 '22 01:03 MisutaaAsriel

So what's the action here? It sounds like there is nothing further we could do to differentiate those values as reported on the Mac Pro and the overall feature is good to go?

PatTheMav avatar Mar 13 '22 15:03 PatTheMav

That is my understanding as well. It would be great if Apple were to add the ID to other encoders as well, but that shouldn't block the PR imo.

~~I didn't notice the conflicts, will fix them up.~~ Interestingly, it didn't actually conflict when rebasing in the command line while GitHub showed confilcts - weird.

gxalpha avatar Mar 13 '22 16:03 gxalpha

Is there anything this is waiting on? I've been using a custom build with this for some time, and it's actually highly preferable because of #6077 / #4942, as otherwise I literally have to change my encoder selection, set all my custom settings up, do a test recording, then watch OBS in Activity Monitor just to see which GPU VTEncoderServiceXPC spawns on.

With this pull I know for a fact I'm at least selecting the right GPU.

Practically makes "stock" OBS impossible to use correctly without this as an eGPU user.

MisutaaAsriel avatar Aug 06 '22 00:08 MisutaaAsriel

@gxalpha let's review this after HEVC and ProRes are merged because the encoder source will have changed a lot by then.

PatTheMav avatar Oct 04 '22 20:10 PatTheMav

I no longer think that this is a good PR.

Given that kVTVideoEncoderList_GPURegistryID appears to only be added to eGPU's (and in rare cases dedicated GPU's, but crucially never on Apple Silicon), this PR is only relevant for eGPU machines. It's very clear Apple believes that eGPU's are not the way forward (and thinks the same about Intel), so this PR is effectively already legacy code. I do not wish to add maintenance cost (and waste reviewer's time) by adding legacy code with my PR's.

As such, I'm closing this PR. If someone else wants to pick it up, fix the conflicts, drive the review process and maintain the code into the future that'd be fine by me, but personally I don't think it's a good idea.

gxalpha avatar Oct 11 '22 15:10 gxalpha