letmaik / pyvirtualcam

🎥 Send frames to a virtual camera from Python
GNU General Public License v2.0
452 stars 49 forks source link

Unity Video Capture backend #56

Closed graphemecluster closed 3 years ago

graphemecluster commented 3 years ago

(Discussion: https://github.com/letmaik/pyvirtualcam/discussions/51)

Many of the files are rewrote for this pull request. Just as you said, I'm not including the install scripts though the shared.inl is necessary. And I'm finally not going to touch the UnityCaptureFilter.cpp file.

Looking at the source code of UnityCaptureFilter.cpp:

With the above information:

A few more things to notice:

Please let me know if there's anything unclear or forgotten.

letmaik commented 3 years ago

Looks great! Do you mind if I commit to your branch and fix up the remaining issues?

graphemecluster commented 3 years ago

By all means! I'm trying to fix it too. Edit: Although the errors are fixed, be sure to check if it is completed.

graphemecluster commented 3 years ago

If the RegGetValueW method does not work at all, try replacing

if (RegGetValueW(HKEY_CLASSES_ROOT, guid, L"", RRF_RT_REG_SZ, NULL, &str, &size) != ERROR_SUCCESS) return false;

with the following:

HKEY key;
if (RegOpenKeyExW(HKEY_CLASSES_ROOT, guid, 0, KEY_READ, &key) != ERROR_SUCCESS) return false;
if (RegQueryValueExW(key, L"", NULL, NULL, (LPBYTE)str, &size) != ERROR_SUCCESS) return false;

I don’t know which one is better if both work though.

letmaik commented 3 years ago

I did the following changes:

I'll do some more testing etc. later today, also it has to be added to CI. The color is still wrong for me. Can you try in multiple apps? The latency.py sample gives you a good idea of the expected color (see terminal output).

graphemecluster commented 3 years ago

Note that std::stoi() return 3 for strings like 3DCamera, which is not expected.

letmaik commented 3 years ago

Note that std::stoi() return 3 for strings like 3DCamera, which is not expected.

You're right, didn't think of that. It may make sense to remove the numeric case altogether for now since it becomes ambiguous if multiple backends do the same. It probably makes more sense for each backend to expose all available devices and then let the user choose, based on some criteria, but that's out of scope in this PR.

graphemecluster commented 3 years ago

OK, let’s remove it then. Oh, I didn’t edit the README either.

letmaik commented 3 years ago

I've added the proper pixel format conversions now and it looks fine for me. Could you give it a try? If it doesn't look right for you, then your frames are probably not in RGBA channel order. This would be fully opaque red: [255, 0, 0, 255]

letmaik commented 3 years ago

I just stumbled upon these: https://github.com/obsproject/obs-studio/discussions/4476 https://stackoverflow.com/q/66854423 Once we're done here those two should get a follow-up response.

letmaik commented 3 years ago

I updated the README and added a new sample which sends transparent gif frames to the cam. I included some instructions on how to configure OBS to capture in RGBA. From my side I'm done now. Would be good if you could give it a final test before I merge it. Note that you can also download the wheels from the artifacts here: https://github.com/letmaik/pyvirtualcam/suites/2528611452/artifacts/54812494

graphemecluster commented 3 years ago

Sorry for letting you wait. I am in a different time zone from you. Currently there are some problems to my computer so I am not able to test it. It should work on most computers without my test. Thank you for your patience.

letmaik commented 3 years ago

I found an issue when using multiple cameras which requires a fix upstream https://github.com/schellingb/UnityCapture/issues/17.

graphemecluster commented 3 years ago

For now, let's not use unicode. This has to be done across all backends, and in theory it makes sense, but should be a separate PR.

So should all of them be changed to wide-characters now? This is a minor problem though.

letmaik commented 3 years ago

For now, let's not use unicode. This has to be done across all backends, and in theory it makes sense, but should be a separate PR.

So should all of them be changed to wide-characters now? This is a minor problem though.

Sure, I won't have time for it though. So far, all the virtual cameras didn't need it as the names are all ascii. And I have a feeling that probably no one is using custom names for unity capture. Do you have a use case?

graphemecluster commented 3 years ago

For now, let's not use unicode. This has to be done across all backends, and in theory it makes sense, but should be a separate PR.

So should all of them be changed to wide-characters now? This is a minor problem though.

Sure, I won't have time for it though. So far, all the virtual cameras didn't need it as the names are all ascii. And I have a feeling that probably no one is using custom names for unity capture. Do you have a use case?

I think so too. Actually I don't need it either, so let's just put it aside.