-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Faster image capture #2472
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 #2472
Conversation
|
Still some compilation errors on Linux, when building Unreal project Will open PR to the branch if I'm able to fix these Edit: Also, it would be great to have some measure of the improvement which can be expected from this, such as FPS of |
|
After modfiying the code a bit for compilation, more prblems occurred Compilation successful, but error while starting UE Multiple error messages with the same info, and then seg fault with the following trace |
a23db70 to
4731e5e
Compare
|
Rebased onto current master as this had some merge conflicts wrt change in the simSwapTextureApi (noticed some space<->tab inconsistencies in swaptexture PR just now). |
960c3a6 to
de9cf5b
Compare
| //msr::airlib::ClockBase* clock = msr::airlib::ClockFactory::get(); | ||
|
|
||
| ImagesResult result; | ||
| /*ImagesResult result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
| left_img[idx-counter] = result->response.at(0).image_data_uint8 [idx]; | ||
| right_img[idx-counter] = result->response.at(1).image_data_uint8 [idx]; | ||
| //left_img[idx-counter] = result->response.at(0).image_data_uint8 [idx]; | ||
| //right_img[idx-counter] = result->response.at(1).image_data_uint8 [idx]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
2e55e47 to
142f860
Compare
| def image_callback_benchmark_simGetImage(self): | ||
| self.image_benchmark_num_images += 1 | ||
| iter_start_time = time.time() | ||
| image = self.airsim_client.simGetImage("fpv_cam", airsim.ImageType.Scene) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to update simGetImage to return an ImageResponse instead of only raw data, for help in decoding
|
|
||
| virtual void initialize() = 0; | ||
|
|
||
| virtual std::vector<ImageCaptureBase::ImageResponse> getImages(const std::vector<ImageCaptureBase::ImageRequest>& request) const = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated to this PR
but are the functions just above this on L40-44 needed anymore?
virtual const ImageCaptureBase* getImageCapture() const = 0;
virtual ImageCaptureBase* getImageCapture()
{
return const_cast<ImageCaptureBase*>(static_cast<const VehicleSimApiBase*>(this)->getImageCapture());
}|
@rajat2004 agreed wrt before and after benchmark. We'll add share some benchmarks in the comments here. |
|
@madratman Great, and thanks for the script! Will test things on Linux and see if it's working |
|
I'm still getting the error from #2472 (comment), which I think is happening due to the |
|
weird. mind doing a full cleanup / asserting you're not missing any step in the following? I haven't seen that error (have compiled this PR branch on linux multiple times now). cd AirSim;
git checkout master; # update if not already: git pull upstream master --rebase
git clean -fdx;
git fetch upstream pull/2472/head:PR/2472; git checkout PR/2472;
./setup.sh && ./build.sh;
cd Unreal/Environments/Blocks;
./update_from_git.sh
PATH_TO/UnrealEngine/Engine/Binaries/Linux/UE4Editor "${PWD}/Blocks.uproject" P.S. PR is not done yet. there some issues in decoding the image |
|
@madratman I ran the |
|
I tested on 4.22, 4.24. Any reason you're on 4.18? With this PR and static mesh one, it seems that we'll have to drop support for 4.18. |
|
Just that I didn't have any specific reason to upgrade to 4.22. I'll install 4.24 and test, but since you've tested, should be good to go |
458c023 to
220ad4b
Compare
|
I updated my UE installation to 4.24(clang-8), now I'm getting compilation errors Is anyone else also getting this? I ran the |
| #include "Async/Async.h" | ||
|
|
||
| RenderRequest::RenderRequest(std::vector<uint8_t>& rgba_output) : rgba_output_(&rgba_output) | ||
| RenderRequest::RenderRequest(BufferPool *buffer_pool) : buffer_pool_(buffer_pool) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buffer_pool_ hasn't been declared as a variable in the class
|
Hey, so some good news, I was back on Windows today after 2-3 weeks, and got this PR working! FPS jumped by quite a bit, on master I was getting ~43, with the PR I'm getting ~69 on my laptop. This is with the Blocks env running in Editor. Need to test on Linux and see if it's working there, will mostly open a new PR after that Re the .uasset change, it's currently using the master branch camera asset, so no change in that one, still not sure what was modified in the asset One thing I noticed is that with the PR, we're getting 4 channels RGBA instead of 3 in master Update: On Linux, Vulkan is causing the crash, OpenGL worked, but FPS is very low, ~29, need to compare to master. Opened #2713 which replaces this one |
|
@ironclownfish @madratman Thanks a lot for this awesome PR! I've opened #2713 which is upto date with master, and should work on Linux & Win. Any testing would be great! Edit: It doesn't work properly right now, am getting random data in the image on Linux. Works with OpenGL, details in PR |
…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.
9b74b56 to
f56ace7
Compare
|
Closing for the sake of #3018 |
|
@ironclownfish @rajat2004 would this work on Unity too? |
A new PR for speeding up image capture, better than my previous PR #2453.
Includes previous PR, which:
And adds the following