Unity Video Capture backend #56

graphemecluster · 2021-04-18T11:55:49Z

(Discussion: #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:

Yes, the conversion of RGBA to BGRA inside the file is unavoidable without modify it.
A maximum number of 74 devices can be installed.
The devices are named name #index except for index 0, which is named name. Yes, the default value of name is Unity Video Capture, but it can be changed by the user by the install script argument UnityCaptureName. The number of devices installed is specified by the argument UnityCaptureDevices with the default value 1. All devices have different CLSID in the format 5C2CD55C-92AD-4999-8666-912BD3E700XX.

With the above information:

We parse the device parameter by the following condition:
If device is missing, we iterate through 0 to 74 to check for the first device registered;
If device is a number-like string from 0 to 74, the index is used;
Otherwise the device is assumed to be the device name; we iterate through 0 to 74 to check if device matches the name given by the register key; or an error is fired if there are no matches.
The device() function returns the device name given by the register key, not necessary Unity Video Capture.

A few more things to notice:

I think it won't works on a notable number of Windows devices due to the RegGetValueW method.
The red-blue channels are not switched at least for my device in the final display.
I am not changing argb to bgra in image_formats.h due to consistency, but of course you can if you want.
All uint32_t are changed to int32_t, though only the argb_to_argb method has actual use to negative numbers.
I used std::wstring instead of std::string.
The fps parameter is not used currently.
I don't know if we also need to include the shared.inl file in setup.py.
I think it is completed but I'm not sure.

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

letmaik · 2021-04-18T13:33:50Z

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

graphemecluster · 2021-04-18T13:38:32Z

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 · 2021-04-18T13:54:17Z

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 · 2021-04-18T14:37:20Z

I did the following changes:

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.
I renamed some bits to be more consistent. I think I'll rename the folder as well, to native_windows_unity_capture.
SendIsReady, as you had it earlier, unfortunately has to go into send() as it will always be false if no app is capturing the cam yet.

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).

Some minor fixes and cleanup.

graphemecluster · 2021-04-18T15:28:13Z

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

letmaik · 2021-04-18T15:57:03Z

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 · 2021-04-18T17:39:36Z

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

letmaik · 2021-04-18T19:59:48Z

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 · 2021-04-19T10:33:46Z

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

letmaik · 2021-04-19T11:59:38Z

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 · 2021-04-19T12:04:44Z

pyvirtualcam/native_windows_unity_capture/shared_memory/shared.inl

@@ -58,7 +58,7 @@ struct SharedImageMemory
 		callback(m_pSharedBuf->width, m_pSharedBuf->height, m_pSharedBuf->stride, (EFormat)m_pSharedBuf->format, (EResizeMode)m_pSharedBuf->resizemode, (EMirrorMode)m_pSharedBuf->mirrormode, m_pSharedBuf->timeout, m_pSharedBuf->data, callback_data);
 		ReleaseMutex(m_hMutex); //unlock mutex

-		return RECEIVERES_NEWFRAME;
+		return (IsNewFrame ? RECEIVERES_NEWFRAME : RECEIVERES_OLDFRAME);


With this line, the timeout should be set to a large enough number (previously I set it to 100000000) in milliseconds or the frame will be cleared immediately.

I just don't want the frame being cleared even after a day, but that's up to you.

This is the receiver, so there's no point changing any code in it as it won't be used by pyvirtualcam but rather by the filter DLL that you install. I'll have a look at the timeout value on the sender side, it's a fair point.

Yeah.. the timeout definitely needs to be higher, otherwise low framerates also lead to interruptions. I've changed it to 1 day.

The receive method is used here and this is the only place which prints "Unity has stopped sending image data". That's OK if you think 1 day is enough (though I want to keep the frame forever).

The receive method is only used in the filter. I can remove the receive method in the backend and everything would compile fine. Do you see? Any change in that method does not have an effect on our side. The shared.inl is a header which gets pulled in both by the receiver (filter) and sender (us). Some code is only used by one of both.

What's your use case for keeping a frame longer than a day? I'm happy to make the timeout longer but would like to understand a bit more.

The timeout is of type int (signed 32 bit) and since it's in milliseconds the maximum you can get is something like 24 days.

I changed it to the maximum now :)
d03b9a6

Honestly, no special reason – just think that I am a perfectionist.
Anyway, the point is that this is actually a feature written in README.md of UnityCapture. So I am opening an issue to due with it.
schellingb/UnityCapture#18

graphemecluster · 2021-04-19T12:05:03Z

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 · 2021-04-19T14:42:12Z

I found an issue when using multiple cameras which requires a fix upstream schellingb/UnityCapture#17.

graphemecluster · 2021-04-19T18:07:46Z

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 · 2021-04-19T18:48:03Z

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 · 2021-04-19T18:59:18Z

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.

graphemecluster added 2 commits April 18, 2021 19:05

Add New Backend for Windows

cf702d4

Fix String Issue

992f232

Fix Minor Errors

61241b1

various fixes and cleanup

20fada4

fix bad merge

1f33e72

rename folder, add license

19a7fa0

letmaik added 3 commits April 18, 2021 18:45

remove numeric device string; query reg value size

c21ec96

fix registry query

de35c51

pixel format

776b17d

letmaik added 8 commits April 18, 2021 23:25

fix i420 and nv12 conversions

32252e3

adapt tests

9ffacc5

use unchanged shared.inl

0f81869

minor cleanup

182bf37

avoid tmp buffer for I420/NV12, other cleanup

36d9632

error handling

3c20186

enable scaling to be consistent with other backends

8432575

update readme

ac002be

add rgba sample

b05c47b

graphemecluster commented Apr 19, 2021

View reviewed changes

fix timeout

f9be071

letmaik merged commit 6218473 into letmaik:master Apr 19, 2021

letmaik changed the title ~~Unity Video Capture~~ Unity Video Capture backend Apr 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unity Video Capture backend #56

Unity Video Capture backend #56

graphemecluster commented Apr 18, 2021 •

edited by letmaik

Loading

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021 •

edited

Loading

graphemecluster commented Apr 18, 2021 •

edited

Loading

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021

letmaik commented Apr 18, 2021

letmaik commented Apr 19, 2021

letmaik commented Apr 19, 2021

graphemecluster Apr 19, 2021

graphemecluster Apr 19, 2021

letmaik Apr 19, 2021

letmaik Apr 19, 2021

graphemecluster Apr 19, 2021

letmaik Apr 19, 2021

letmaik Apr 19, 2021

letmaik Apr 19, 2021

graphemecluster Apr 19, 2021 •

edited

Loading

graphemecluster commented Apr 19, 2021

letmaik commented Apr 19, 2021

graphemecluster commented Apr 19, 2021

letmaik commented Apr 19, 2021

graphemecluster commented Apr 19, 2021

Unity Video Capture backend #56

Unity Video Capture backend #56

Conversation

graphemecluster commented Apr 18, 2021 • edited by letmaik Loading

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021 • edited Loading

graphemecluster commented Apr 18, 2021 • edited Loading

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021

letmaik commented Apr 18, 2021

graphemecluster commented Apr 18, 2021

letmaik commented Apr 18, 2021

letmaik commented Apr 19, 2021

letmaik commented Apr 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graphemecluster Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

graphemecluster commented Apr 19, 2021

letmaik commented Apr 19, 2021

graphemecluster commented Apr 19, 2021

letmaik commented Apr 19, 2021

graphemecluster commented Apr 19, 2021

graphemecluster commented Apr 18, 2021 •

edited by letmaik

Loading

graphemecluster commented Apr 18, 2021 •

edited

Loading

graphemecluster commented Apr 18, 2021 •

edited

Loading

graphemecluster Apr 19, 2021 •

edited

Loading