Spectator view dynamic camera config (load video frame settings at runtime) #286

Ybalrid · 2018-06-14T14:41:03Z

Hi, I've made some changes internally on the spectator view compositor.

I find it quite annoying to have to rebuild the DLLs if the frame size or framerate of the camera changes.

The changes consist on the following:

If a file called camera.cfg is found inside the User's Document folder (next to the folder where the HologramCapture would be found), the value used will be the ones in the folder for things like the frame height, width and the video FPS.
If the file is not found, it will silently fallback to use the values set to CompositorConstants.h" as it always had. This doesn't change the behavior for existing users.

The way it works is basically just a few static variables that are exported from CompositorDLL. With a header to import it on the UnityCompositorInterface and a simple config file reader that is called at plugin initialization (or from the main() function of the calibration app).

The place where theses variables (FRAME_WIDTH, etc...) are originally mentioned in the README files has been updated with a not explaining that it is possible to create this file and use it that way.

That way, a user just has to build the DLL once, with the configuration for his capture card (adding or not the necessary SDKs).

Add CameraConfiguration.h, a class that parse a key=value file and get the camera parameters. Implemented in a single header in the SharedHeaders project. Added static variables to be able to change the camera frame configuraion. (They are initialized by default to their values in CameraConstants.h) Added the loading of the camrea configuration file into the UnityCompositorInterface. If file isn't found, the old behavior of SV is not changed.

Also, update teh configuration so that we can actually access the exported static variables from CompositorDLL

Calibration is using things exported from CompositorDLL. This permit to set build dependencies between the two projects

Ybalrid · 2018-06-14T14:44:30Z

And this has been built on top of the current master, that include #271

fieldsJacksonG · 2018-06-14T22:20:51Z

Love this contribution, thanks!

Can you update this readme section to mention this config file as an option (include file name, file location, expected file contents)

fieldsJacksonG · 2018-06-14T23:08:17Z

SpectatorView/Calibration/Calibration.sln

@@ -30,6 +36,14 @@ Global
 		{70239410-43FF-4F2B-ACBB-E640437F7289}.Release|x64.ActiveCfg = Release|x64
 		{70239410-43FF-4F2B-ACBB-E640437F7289}.Release|x64.Build.0 = Release|x64
 		{70239410-43FF-4F2B-ACBB-E640437F7289}.Release|x86.ActiveCfg = Release|x64
+		{16FA1D9F-8C62-4A94-9A47-EE2E153DC625}.Debug|x64.ActiveCfg = Debug|x64
+		{16FA1D9F-8C62-4A94-9A47-EE2E153DC625}.Debug|x64.Build.0 = Debug|x64
+		{16FA1D9F-8C62-4A94-9A47-EE2E153DC625}.Debug|x86.ActiveCfg = Debug|Win32


Remove Win32 build targets. The OpenCV download is x64 only.

Of course, will do. SpectatorView is 64bit only (except your new pose provider that is obviously 32bit, since it's running on an hololens device)

fieldsJacksonG · 2018-06-14T23:11:26Z

SpectatorView/Calibration/Calibration/Calibration.vcxproj

    </Link>
    <PostBuildEvent>
      <Command>xcopy /y "$(OpenCV_vc14)\bin\*.dll" "$(OutDir)"</Command>
    </PostBuildEvent>
    <PreBuildEvent>
      <Command>if EXIST "$(decklink_inc)" xcopy /y "$(decklink_inc)"
 xcopy /y "$(OpenCV_vc14)\bin\*.dll"
+xcopy /y "$(ProjectDir)\..\..\Compositor\x64\$(Configuration)\CompositorDLL.dll"


This xcopy can fail if the calibration app is built before the compositor.

You have added the CompositorDLL project to the Calibration sln, so you should instead right click on the References under the calibration project and add CompositorDLL as a reference.

I have to go to bed now, so I'll look into your remarks tomorow (I'm in france).

Project is set to depend on CompositorDLL on that solution, so it should be built after. (maybe I forgot to commit the changes in the SLN file)

The calibration SLN contains a ref to the compositor project, and it's set as a build dependency. Visual Studio will always build them in the right order. So the DLL will always be available to copy.

fieldsJacksonG · 2018-06-14T23:48:34Z

SpectatorView/Calibration/Calibration/Main.cpp

@@ -4,6 +4,11 @@
 #include "stdafx.h"
 #include "CalibrationApp.h"


Look in CalibrationApp.cpp, there are a few cv::Mat's still using FRAME_WIDTH, FRAME_HEIGHT, and FRAME_BUFSIZE

Will fix that and ping you when fixed later friday 😉

fieldsJacksonG · 2018-06-15T00:12:39Z

SpectatorView/Calibration/Calibration/Main.cpp

+	}
+	catch(const std::exception& e)
+	{
+		OutputDebugStringA(e.what());


Is there a fallback to the previously defined constants?

The statics are initialized with the values in the header. but now that I look at it, if the file is missformed ( = has width, but not height); it may leave the thing broken.

I should reset the original values in the catch{} scope

fieldsJacksonG · 2018-06-15T00:15:13Z

SpectatorView/Calibration/Calibration/Main.cpp

@@ -16,6 +21,25 @@ LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);
 // Entry point
 int WINAPI wWinMain(_In_ HINSTANCE hInstance, _In_opt_ HINSTANCE hPrevInstance, _In_ LPWSTR lpCmdLine, _In_ int nCmdShow)
 {
+
+	auto const filePath = CameraConfigurationFile::getMyDocumentPath() + "\\camera.cfg"s;


Where is the config file read for the UnityCompositorDLL?

"current user's document folder"\camera.cfg

Right next of where the calibration files and the holograms capture folders are. Since the user will be messing around with files in "My documents", why not putting it here.

Relative path would be from the current process, here when the DLL is loaded it's the Unity editor, so using an user's folder is the best solution I think.

CameraConfigurationFile::getMyDocumentPath() return the path to the user's "document" folder.

Oh, I see what you mean, it's on the plugin load callback, before everything else is done

extern "C" void UNITY_INTERFACE_EXPORT UNITY_INTERFACE_API UnityPluginLoad(IUnityInterfaces* unityInterfaces) { try { const auto configFilePath = CameraConfigurationFile::getMyDocumentPath() + "\\camera.cfg"; CameraConfigurationFile configFile(configFilePath); configFile.readConfig(); FrameProviderStaticConfig::width = int(configFile.getWidth()); FrameProviderStaticConfig::height = int(configFile.getHeight()); FrameProviderStaticConfig::fps = float(configFile.getFrameRate()); } catch(std::exception& e) { (void)e; //Will just use the hardcoded values FrameProviderStaticConfig::width = FRAME_HEIGHT; FrameProviderStaticConfig::height = FRAME_WIDTH; FrameProviderStaticConfig::fps = VIDEO_FPS; } //[...] }

in UnityCompositorInterface.cpp, line 568

fieldsJacksonG · 2018-06-15T00:18:51Z

SpectatorView/Compositor/CompositorDLL/CompositorInterface.cpp

@@ -172,7 +173,7 @@ void CompositorInterface::TakePicture(ID3D11Texture2D* outputTexture)

    // Get bytes from texture because screengrab does not support texture format provided by Unity.
    DirectXHelper::GetBytesFromTexture(_device, outputTexture, FRAME_BPP, photoBytes);
-    ID3D11Texture2D* tex = DirectXHelper::CreateTexture(_device, photoBytes, FRAME_WIDTH, FRAME_HEIGHT, FRAME_BPP, DXGI_FORMAT_B8G8R8A8_UNORM_SRGB);
+    ID3D11Texture2D* tex = DirectXHelper::CreateTexture(_device, photoBytes, FrameProviderStaticConfig::width, FrameProviderStaticConfig::height, FRAME_BPP, DXGI_FORMAT_B8G8R8A8_UNORM_SRGB);


How do these dimensions fall back to FRAME_WIDTH/ FRAME_HEIGHT if the config file is not present?
Looks like width and height will be undefined.

Can you make width and height private, then the function get_width() and get_height() should return the cached loaded value (if file existed) or the fallback constant if not.

variables are initialized to FRAME_WIDTH/FRAME_HEIGHT inside CompositorDLL. They are __declspec(dllexport/importd)ed from that DLL. So if they aren't touched by the code that load the config file, they are set to whatever was set in the header.

Also, the configuration file loader class throws exceptions when it cannot read the file. I should explicitly reset theses values to FRAME_WIDTH/FRAME_HEIGHT/VIDEO_FPS in the catch() statements

Some of the source files have been encoded in Unicode (2byte per character on windows). Git thinks they are binary so GitHub doesn't really show diff correctly on theses files.

Compositor.DLL contains the static initialization of theses files

#include "FrameProviderStaticConfig.h" int FrameProviderStaticConfig::width = FRAME_WIDTH; int FrameProviderStaticConfig::height = FRAME_HEIGHT; float FrameProviderStaticConfig::fps = VIDEO_FPS;

In either case, now this is specifically handled when catching an exception if there's any problem with reading the config file

Ybalrid · 2018-06-15T00:32:28Z

Thanks for the feedback @fieldsJacksonG! Will push fixes on this PR during business hours in France today (Friday)

…ng camera.cfg

OpenCV is 64bit only here. The 32bit version will not link.

Ybalrid · 2018-06-15T08:11:16Z

@fieldsJacksonG I think I got around to do all your requested changes, can you review again?

Ybalrid · 2018-07-13T08:36:36Z

@fieldsJacksonG Just so you know, I'm finishing an internship in two weeks, after that I will not have access to any HoloLens hardware. It will be harder for me to contribute on this without being able to test anything 😉

Ybalrid added 4 commits June 14, 2018 15:39

Add loading of configuration variables to the calibration program

076eefb

Also, update teh configuration so that we can actually access the exported static variables from CompositorDLL

Add project build dependencies

256cb6b

Calibration is using things exported from CompositorDLL. This permit to set build dependencies between the two projects

Fix iniitaliazation variable. Fix bad file encoding

edd30d8

keveleigh requested a review from fieldsJacksonG June 14, 2018 22:00

keveleigh added the SpectatorView label Jun 14, 2018

fieldsJacksonG suggested changes Jun 15, 2018

View reviewed changes

Ybalrid and others added 4 commits June 15, 2018 09:37

Add explicit fallback to default values in case of problem with loadi…

73e5a67

…ng camera.cfg

Remove 32bit Configuration from Calibration.sln

19d194f

OpenCV is 64bit only here. The 32bit version will not link.

Replace some missing FRAME_WIDTH/HEIGHT/BUFFSIZE

0f6cd56

Update "Camera Setup" README section

397faff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spectator view dynamic camera config (load video frame settings at runtime) #286

Spectator view dynamic camera config (load video frame settings at runtime) #286

Ybalrid commented Jun 14, 2018

Ybalrid commented Jun 14, 2018

fieldsJacksonG commented Jun 14, 2018

fieldsJacksonG Jun 14, 2018

Ybalrid Jun 15, 2018

fieldsJacksonG Jun 14, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

fieldsJacksonG Jun 14, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

fieldsJacksonG Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

fieldsJacksonG Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018 •

edited

Loading

fieldsJacksonG Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid Jun 15, 2018

Ybalrid commented Jun 15, 2018 •

edited

Loading

Ybalrid commented Jun 15, 2018

Ybalrid commented Jul 13, 2018

		@@ -4,6 +4,11 @@
		#include "stdafx.h"
		#include "CalibrationApp.h"

Spectator view dynamic camera config (load video frame settings at runtime) #286

Are you sure you want to change the base?

Spectator view dynamic camera config (load video frame settings at runtime) #286

Conversation

Ybalrid commented Jun 14, 2018

Ybalrid commented Jun 14, 2018

fieldsJacksonG commented Jun 14, 2018

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

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

Ybalrid Jun 15, 2018 • edited Loading

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

Ybalrid commented Jun 15, 2018 • edited Loading

Ybalrid commented Jun 15, 2018

Ybalrid commented Jul 13, 2018

Ybalrid Jun 15, 2018 •

edited

Loading

Ybalrid commented Jun 15, 2018 •

edited

Loading