Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster Image Capture - Try 2 #3018

Draft
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

rajat2004
Copy link
Contributor

@rajat2004 rajat2004 commented Sep 13, 2020

Another rebased version of #2713 😅
For details and discussion, see #2713 & the original PR #2472 .

On Linux requires OpenGL (-opengl arg when starting Editor or binary)
(Outdated binaries, doesn't have depth image impleentation, does BGRA image)
Linux binary - https://drive.google.com/file/d/1sK-S0CX6cqGtuMeQLqz2OhAVVg6dZAq1/view?usp=sharing (Before merging master branch though)
Win - https://drive.google.com/file/d/1pNadRe0DuuyHOpYpzwKWHxCdZKONk6Qf/view?usp=sharing

Current major items left -

  • Get Depth images working (Thanks to @cthoma20-arc!)
  • Figure out a way for Vulkan on Linux (Maybe use the original method?)

Other changes left -

  • Get compress, pixelsasfloat options, camera pose working
  • Currently, this PR does 4 channel, i.e. BGRA image, docs, scripts need to be updated (will cause major incompatibility issues) (Changed to BGR, thanks to @cthoma20-arc)
  • Currently, disabling camera after doing capture is commented out since it was causing crashes, might need to instead add a new simDisableCamera API, this effect is seen prominently when running the multirotor/high_res_camera.py script.

Some stats on my system (using image_benchmarker.py) -
Latest Blocks release - 24 FPS
PR (in Editor) - 27 FPS
PR binary - 44 FPS

Thanks to all the testers who have tried this out, just to name a few @WouterJansen
Still needs lots and lots of testing!

Nicholas Gyde (Collabera) and others added 30 commits March 25, 2020 16:38
…more. Buffer pool might be useful for other things.
…er pool to the render thread so it can ask for the correct size of dest buffer when src buffer is known.
…e-0 buffer back into the response's image_data_uint8, which then returns that useless buffer to the buffer pool. Since RPC can't take over the unique_ptr that manages our buffer, we should send RPC a copy.
… Vulkan"

With this modification, garbage data is returned
This reverts commit e85fee6.
@rajat2004 rajat2004 mentioned this pull request Sep 13, 2020
@rajat2004 rajat2004 marked this pull request as draft September 13, 2020 08:47
@WouterJansen
Copy link
Contributor

WouterJansen commented Sep 15, 2020

Hi, I'm trying this pull request and running into an issue currently, the RPC API response for an image request has often an incorrect data size and therefore can't be correctly shaped into the resolution of the image with 4 channels. It's missing data it seems.

For example:

  • 240x512= works fine
  • 512x512= works fine
  • 480x650= returns an image_data_uint8 of 1290240 bytes. Where it should have been 1248000 bytes. So it is missing exactly 22 rows of the image ((1248000 - 1290240) / 4) / 480. Not sure if that is useful information :p
  • 480x752= returns an image_data_uint8 of 1474560 bytes. Where it should have been 1443840 bytes. So it is missing exactly 16 rows of the image ((1443840 - 1474560 ) / 4) / 480.

It is also always consistent in these incorrect data sizes. are only certain aspect ratios supported perhaps currently? or is there another underlying issue.

my settings:

{
   "SimMode":"ComputerVision",
   "Vehicles":{
      "vehicle":{
         "VehicleType":"ComputerVision",
         "AutoCreate":true,
         "Cameras":{
            "camera1":{
               "CameraName":"left",
               "CaptureSettings":[
                  {
                     "ImageType":0,
                     "X": 0,
                     "Y": 0,
                     "Z": 0,
                     "Roll": 0,
                     "Pitch": 0,
                     "Yaw": 0,
                     "Width":752,
                     "Height":480
                  }
               ]
            }
         }
      }
   }
}

My test script:

    client = airsim.CarClient()
    client.confirmConnection()

    responses = client.simGetImages([airsim.ImageRequest( "camera1", airsim.ImageType.Scene, False, False)])
    rgba_array = np.frombuffer(responses[0].image_data_uint8, dtype=np.uint8)
    rgba_array_reshaped = rgba_array .reshape((responses[0].height, responses[0].width,-1))

Biggest difference I suppose it that I am also running a slightly older version of AirSim with many other custom changes with only this pull request merged and also running on Unreal 4.22. However, I never made custom changes to the camera system.
Running on Windows from the Editor.

Finally, at least in my environments, I am reaching similar performance to you. In my environment which runs ideally at 80FPS, if I make one image request of 512x512 it takes me 0.0629s. Unfortunately, it doesn't scale well at all. Two requests of 512x512 of two different cameras takes 0.1770s. BUT, if find it to already scale much better than previously! Where previously 3 512x512 scene camera requests would result me in 2FPS, it is now doubled already at 4FPS.

Some stats on my system in my custom environment (not blocks!) for single image request then using your script:

before pull request merged:
EDITOR - idle running (no script) : 75 FPS
PACKAGED - idle running (no script) : 80 FPS
EDITOR - using image_benchmarker.py - 8 average camera FPS
PACKAGED - using image_benchmarker.py - 10 average camera FPS

after pull request merged:
EDITOR - idle running (no script) : 75 FPS
PACKAGED - idle running (no script) : 80 FPS
EDITOR - using image_benchmarker.py - 14 average camera FPS
PACKAGED - using image_benchmarker.py - 12 average camera FPS

@WouterJansen
Copy link
Contributor

WouterJansen commented Sep 15, 2020

Another issue I experience when running Airsim, on both the current released v1.3.1 Windows Blocks packaged build as on this branch is that it runs at over 300FPS when I start it. As soon as I make one request for images, the FPS drops to about 100FPS and stays there, without any additional requests for images happening. It this some issue with RPC staying open? Or the camera being activated but never deactivated again?

On the blocks environment this is not an issue of course, but on custom environments with less good performance this extreme drop in framerate after just one simple call is really impactful.

I guess because it also happens in 1.3.1 Blocks on Windows, this is something that others can test for as well.

before:
image

run this code:

    # Make connection to AirSim API
    client = airsim.CarClient()
    client.confirmConnection()

    requests = []
    requests.append(airsim.ImageRequest("camera1", airsim.ImageType.Scene, False, False))
    requests.append(airsim.ImageRequest("camera1", airsim.ImageType.Scene, False, False))
    requests.append(airsim.ImageRequest("camera2", airsim.ImageType.Scene, False, False))
    requests.append(airsim.ImageRequest("camera2", airsim.ImageType.Scene, False, False))
    requests.append(airsim.ImageRequest("camera3", airsim.ImageType.Scene, False, False))
    requests.append(airsim.ImageRequest("camera3", airsim.ImageType.Scene, False, False))

    responses = client.simGetImages(requests)

after running this code once:
image

my settings:

{
	"SimMode": "ComputerVision",
	"Vehicles": {
		"vehicle1": {
			"VehicleType": "ComputerVision",
			"AutoCreate": true,
			"Cameras": {
				"camera1": {
					"CameraName": "left",
					"X": 0,
					"Y": 0,
					"Z": 0,
					"Roll": 0,
					"Pitch": 0,
					"Yaw": 0,
					"CaptureSettings": [{
						"ImageType": 0,
						"Width": 512,
						"Height": 512
					}]
				},
				"camera2": {
					"CameraName": "left",
					"X": 0,
					"Y": 0,
					"Z": 0,
					"Roll": 0,
					"Pitch": 0,
					"Yaw": 0,
					"CaptureSettings": [{
						"ImageType": 0,
						"Width": 512,
						"Height": 512
					}]
				},
				"camera3": {
					"CameraName": "left",
					"X": 0,
					"Y": 0,
					"Z": 0,
					"Roll": 0,
					"Pitch": 0,
					"Yaw": 0,
					"CaptureSettings": [{
						"ImageType": 0,
						"Width": 512,
						"Height": 512
					}]
				}
			}
		}
	}
}

@rajat2004
Copy link
Contributor Author

Will try to have a look at the first problem later, but don't remember any limitations. Just to check, there are some debug lines in the output currently, could you confirm once that the resolution being obtained is different from the one set in the settings.

As for the second, try uncommenting the line commented out in 73274b7, that could help the issue, but crashes might occur, from others feedback and testing

@WouterJansen
Copy link
Contributor

For 240x512:

Blueprint: Warning: CaptureScene: Scene capture with bCaptureEveryFrame enabled was told to update - major inefficiency.         
LogTemp: Warning: stats: H: 240,  W: 512,  S: 2048,  px_format: 2                                                                 
LogTemp: Warning: camera1: stats: H: 240  W: 512  type: 0  px_format: 2 

For 512x512:

Blueprint: Warning: CaptureScene: Scene capture with bCaptureEveryFrame enabled was told to update - major inefficiency.  
LogTemp: Warning: stats: H: 512,  W: 512,  S: 2048,  px_format: 2                                                                
LogTemp: Warning: camera1: stats: H: 512  W: 512  type: 0  px_format: 2   

for 480x650:

Blueprint: Warning: CaptureScene: Scene capture with bCaptureEveryFrame enabled was told to update - major inefficiency.
LogTemp: Warning: stats: H: 480,  W: 650,  S: 2688,  px_format: 2
LogTemp: Warning: camera1: stats: H: 480  W: 650  type: 0  px_format: 2    

for 480x752:

Blueprint: Warning: CaptureScene: Scene capture with bCaptureEveryFrame enabled was told to update - major inefficiency.                                  
LogTemp: Warning: stats: H: 480,  W: 752,  S: 3072,  px_format: 2
LogTemp: Warning: camera1: stats: H: 480  W: 752  type: 0  px_format: 2  

for 512x752:

Blueprint: Warning: CaptureScene: Scene capture with bCaptureEveryFrame enabled was told to update - major inefficiency. 
LogTemp: Warning: stats: H: 512,  W: 752,  S: 3072,  px_format: 2                                                                
LogTemp: Warning: camera1: stats: H: 512  W: 752  type: 0  px_format: 2 

Another thing from the log that is shown always once on the first image request, not sure if this is important:

LogRenderer: Reallocating scene render targets to support 512x512 Format 10 NumSamples 1 (Frame:341).  

I also updated my first post here with some additional stats running your script. So TLDR: it helps already a lot with this PR :)

@cthoma20-arc
Copy link
Contributor

* 480x650= returns an image_data_uint8 of 1290240 bytes. Where it should have been 1248000 bytes. So it is missing exactly 22 rows of the image  ((1248000 - 1290240) / 4) / 480. Not sure if that is useful information :p

* 480x752= returns an image_data_uint8 of 1474560 bytes. Where it should have been 1443840 bytes. So it is missing exactly 16 rows of the image  ((1443840 - 1474560 ) / 4) / 480.

It is also always consistent in these incorrect data sizes. are only certain aspect ratios supported perhaps currently? or is there another underlying issue.

Hi @WouterJansen, do you mean the image data returned is more than the expected image size? I think what is happening is that you are running issues because of the "stride". Some rows might have extra bytes added to the end of them because the GPU is designed to work with images of set sizes, usually powers of 2, so the stride might be larger than the number of bytes actually in the row. In other words, stride does not equal width * 4. We can fix this by copying each individual row instead of doing BigMemcpy in Unreal/Plugins/AirSim/Source/RenderRequest.cpp. Also change the calculation of the size of the buffer to allocate to:

    size_t size = height * width * 4;

Instead of:

    size_t size = height * stride;

You still need to use the stride to calculate the position of the first byte of each row though. I hope that explanation was clear.

I would make a pull request but I am unfortunately wrapping up some stuff at work, so I've been working off of a custom AirSim, and I'm out of space on my laptop to fit another project :(.

@WouterJansen
Copy link
Contributor

WouterJansen commented Sep 16, 2020

Hi @WouterJansen, do you mean the image data returned is more than the expected image size?

Yes sorry I was mistaken. It is indeed the other way around :)

Sorry if asking an obvious question but I couldn't help but notice the warning related to bCaptureEveryFrame .
It seems it is always turned on for a manually placed camera. Due to APIPCamera::onViewModeChanged(false) being called from
APIPCamera::BeginPlay() if I am not mistaken.
So why does it still need to call fast_param_.render_component->CaptureScene()? I tried removing this line but then the image was always black. So that wasn't a solution.

So then trying the other way around, by removing APIPCamera::onViewModeChanged(false) from APIPCamera::BeginPlay() and disabling bCaptureEveryFrame for every 2DSceneCaptureComponent of the BP_PIPCamera blueprint. No more black images. And I do notice some performance improvements with it. But not had the time to confirm with some benchmarks in packaged builds.

Is there a particular reason that AirSim has everything with bCaptureEveryFrame on by default?

@cthoma20-arc
Copy link
Contributor

So why does it still need to call fast_param_.render_component->CaptureScene()? I tried removing this line but then the image was always black. So that wasn't a solution.

@WouterJansen, I am currently working on a pull request to add support for depth image data. I was able to remove CaptureScene() and everything seemed to work fine for me (without any blank images). Maybe there is a difference in the hardware we are using? The specs for my computer are:

I'm not sure about the reason bCaptureEveryFrame is enabled, but my guess is it is so that the sub windows can be shown correctly. I think it is probably better to keep bCaptureEveryFrame enabled and get rid of CaptureScene() if possible.

@rajat2004
Copy link
Contributor Author

I've merged a PR rajat2004#25 which adds depth images and goes back to BGR, thanks a lot @cthoma20-arc!
Binaries need to be updated though, would be great if it were possible to download them from Azure pipelines

@WouterJansen removing the CaptureScreen() call doesn't seem to have any effect for me as well, I'm on UE 4.24.3 on Linux with OpenGL.

@WouterJansen
Copy link
Contributor

WouterJansen commented Sep 18, 2020

I was able to remove CaptureScene() and everything seemed to work fine for me (without any blank images).

@WouterJansen removing the CaptureScreen() call doesn't seem to have any effect for me as well, I'm on UE 4.24.3 on Linux with OpenGL.

I'm on 4.22 on Windows 10 but have you both tried as well with ViewMode set to NoDisplay? It indeed works in normal ViewMode but for me at least the black result comes from using NoDisplay.

I've tried this latest rajat2004#25. I can say the issues related to the padding in the stride are solved!

However, getting results through RPC in Python just does not work for me when attempting any of the float type images. I find this really strange. With debugger I can see it all working all the way to line 75 of dispatcher.inl. Where it just returns without any errors (airlib/deps/rpclib/include/rpc/dispatcher.inl) .

But on the Python side it just waits forever for a return of RPC that never comes.
I'm using the only version of librpc that is available and airsim Python library 1.3.1.

Are you guys also having this issue or is it all working fine?

Simple test code with a seperate camera with 960x540 with 90° FOV:

    client = airsim.CarClient()
    client.confirmConnection()
    requests = []
    requests.append(airsim.ImageRequest("camera1", airsim.ImageType.DepthVis, True, False))

    responses = client.simGetImages(requests) # Here it will hang forever

EDIT: it does actually work. I changed the resolution to much smaller 256x144 and then the response comes through after a while through RPC in python. But it does take really long, and so for large images it takes forever.

EDIT 2: Also, for 256x144 it returns list of 147456 (256x144x4) floats for image_data_float. Isn't this wrong? Logically I expect it to be 147456 bytes that we can convert to 256x144 float32 depth numbers. But to compare it to the current Airsim 1.3.1 method, it was simply a list of 36864 (256x144) floats.

@rajat2004
Copy link
Contributor Author

@WouterJansen I hadn't tested NoDisplay mode till now,so missed that, thanks for such in-depth testing!
Without the CatureScreen() call, I was getting blank images in the subwindows and the API as well. But for some reason, after adding back the call, the subwindows are still blank, and after doing an API call for say the camera 1 which is in the subwindow, the window starts showing the images, quite strange!
Added back the CaptureScreen() call since it makes the API work again in NoDisplay mode. I'm not seeing the RPC hang atleast

Wonder if this part in PIPCamera.cpp is causing some issues?

@WouterJansen
Copy link
Contributor

WouterJansen commented Sep 18, 2020

Wonder if this part in PIPCamera.cpp is causing some issues?

Yes I believe it is indeed related to that part. I think it is calling a viewmode change for all cameras and turning off bCaptureEveryFrame, while it should only do it for those that are related to the viewport, and not to the other cameras such as those defined as subwindows and especially those being used by the API.

@stale
Copy link

stale bot commented Apr 16, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Apr 16, 2022
@jonyMarino jonyMarino changed the base branch from master to main July 17, 2022 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants