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 #2713

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
68297b5
Fast capture prototype.
Feb 11, 2020
f539dbf
Image capture is functional but needs optimization and cleanup.
Feb 12, 2020
57602f6
Improvements to image capture speedup
Feb 18, 2020
8c3a757
Cleaned up fast img capture, but it's less faster for some reason.
Mar 4, 2020
e899c94
remove comments for PR
Mar 10, 2020
205ae5b
Added buffer pool object and used for image capture. Increases speed …
Mar 12, 2020
33e73f8
Closer to fixing build issues
Mar 18, 2020
82dbb3e
Reinstate commented out DepthNav code
Mar 19, 2020
83a6004
Removed remaining build errors.
Mar 19, 2020
b245d5c
Replaced non-portable expression _CRT_SIZE_MAX
Mar 24, 2020
4a502b0
Restored height, width, and time_stamp data in image responses.
Mar 24, 2020
de9cf5b
Removed extraneous std:move() and made a portable bit strober to fix …
Mar 25, 2020
142f860
add image_benchmarker script
madratman Mar 26, 2020
1f89018
[formatting] maintain whitespacing consistency with AirSim code
madratman Mar 26, 2020
220ad4b
Rather than assume width is rounded up to power of two, pass the buff…
ironclownfish Mar 27, 2020
03494a5
Get simGetImages working
May 6, 2020
9b74b56
A memcopy is necessary in RpcLibAdaptorsBase. A swap would send a siz…
May 11, 2020
0b94b8a
Compilation fixes
rajat2004 May 6, 2020
6d7f405
Fix camera name in benchmark script
rajat2004 May 16, 2020
d70f0e0
Merge branch 'master' of https://github.com/microsoft/AirSim into pr/…
rajat2004 May 23, 2020
6b16976
Fix compile error
rajat2004 May 23, 2020
4603bf3
Fix image width calculation
rajat2004 May 23, 2020
e85fee6
[Unreal] Use WriteOnly LockMode to avoid crash on Linux using Vulkan
rajat2004 May 23, 2020
b5f79e5
Make no. of channles depend on image, allows it to be used on both PR…
rajat2004 May 24, 2020
92c977c
Revert "[Unreal] Use WriteOnly LockMode to avoid crash on Linux using…
rajat2004 May 24, 2020
d572411
Updated benchmark script, add save_images option
rajat2004 May 25, 2020
3de9156
[Unreal] Disable camera after capturing image
rajat2004 May 28, 2020
41dcd84
[Pythonclient] Add high resolution camera benchmark script
rajat2004 May 28, 2020
6612ea4
restore BP_FlyingPawn.uasset, fixes to benchmarking script
madratman Jun 1, 2020
0391272
Add image_type option to benchmark script
rajat2004 Jun 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions AirLib/include/api/RpcLibAdapatorsBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ class RpcLibAdapatorsBase {
return request;
}
};

struct ImageResponse {
std::vector<uint8_t> image_data_uint8;
std::vector<float> image_data_float;
Expand All @@ -401,8 +401,8 @@ class RpcLibAdapatorsBase {
ImageResponse(const msr::airlib::ImageCaptureBase::ImageResponse& s)
{
pixels_as_float = s.pixels_as_float;
image_data_uint8 = s.image_data_uint8;

image_data_uint8 = *(s.image_data_uint8);
image_data_float = s.image_data_float;

//TODO: remove bug workaround for https://github.com/rpclib/rpclib/issues/152
Expand All @@ -428,8 +428,8 @@ class RpcLibAdapatorsBase {

d.pixels_as_float = pixels_as_float;

if (! pixels_as_float)
d.image_data_uint8 = image_data_uint8;
if (!pixels_as_float)
d.image_data_uint8->insert(d.image_data_uint8->begin(), image_data_uint8.front(), image_data_uint8.back());
else
d.image_data_float = image_data_float;

Expand Down
4 changes: 2 additions & 2 deletions AirLib/include/api/VehicleSimApiBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class VehicleSimApiBase : public msr::airlib::UpdatableObject {

virtual void initialize() = 0;

virtual std::vector<ImageCaptureBase::ImageResponse> getImages(const std::vector<ImageCaptureBase::ImageRequest>& request) const = 0;
virtual std::vector<uint8_t> getImage(const std::string& camera_name, ImageCaptureBase::ImageType image_type) const = 0;
virtual void getImages(const std::vector<ImageCaptureBase::ImageRequest>& request, std::vector<ImageCaptureBase::ImageResponse> &responses) const = 0;
virtual void getImage(const ImageCaptureBase::ImageRequest& request, ImageCaptureBase::ImageResponse &response) const = 0;

virtual Pose getPose() const = 0;
virtual void setPose(const Pose& pose, bool ignore_collision) = 0;
Expand Down
43 changes: 42 additions & 1 deletion AirLib/include/common/ImageCaptureBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
#ifndef air_ImageCaptureBase_hpp
#define air_ImageCaptureBase_hpp

#pragma once
#include "common/Common.hpp"
#include "common/common_utils/EnumFlags.hpp"
#include <functional>

namespace msr { namespace airlib {

Expand Down Expand Up @@ -44,7 +46,7 @@ class ImageCaptureBase
};

struct ImageResponse {
vector<uint8_t> image_data_uint8;
std::unique_ptr<vector<uint8_t>, std::function<void(vector<uint8_t>*)>> image_data_uint8 = nullptr;
vector<float> image_data_float;

std::string camera_name;
Expand All @@ -56,10 +58,49 @@ class ImageCaptureBase
bool compress = true;
int width = 0, height = 0;
ImageType image_type;

ImageResponse() : image_data_uint8(nullptr), camera_name(""), camera_position(Vector3r::Zero()), camera_orientation(Quaternionr::Identity()), time_stamp(0), message(""), pixels_as_float(false), compress(true), width(0), height(0), image_type(ImageType::Scene) {}

ImageResponse(const ImageResponse& other)
{
image_data_uint8 = std::unique_ptr<vector<uint8_t>, std::function<void(vector<uint8_t>*)>>(other.image_data_uint8.get(), std::bind(&ImageResponse::CopyDeleter, this, std::placeholders::_1));
camera_name = other.camera_name;
camera_position = other.camera_position;
camera_orientation = other.camera_orientation;
time_stamp = other.time_stamp;
message = other.message;
pixels_as_float = other.pixels_as_float;
compress = other.compress;
width = other.width;
height = other.height;
image_type = other.image_type;
}

ImageResponse& operator=(const ImageResponse& other)
{
image_data_uint8 = std::unique_ptr<vector<uint8_t>, std::function<void(vector<uint8_t>*)>>(other.image_data_uint8.get(), std::bind(&ImageResponse::CopyDeleter, this, std::placeholders::_1));
camera_name = other.camera_name;
camera_position = other.camera_position;
camera_orientation = other.camera_orientation;
time_stamp = other.time_stamp;
message = other.message;
pixels_as_float = other.pixels_as_float;
compress = other.compress;
width = other.width;
height = other.height;
image_type = other.image_type;
return *this;
}

private:
void CopyDeleter(vector<uint8_t>* buf) {
buf = nullptr; //Avoid error for unreferenced formal parameter.
} //Copies of an ImageResponse effectively contain weak pointers. The original response handles deleting the buffer. Ultimately, response copying should be removed from everywhere.
};

public: //methods
virtual void getImages(const std::vector<ImageRequest>& requests, std::vector<ImageResponse>& responses) const = 0;
virtual void getImage(const ImageRequest& request, ImageResponse& response) const = 0;
};


Expand Down
37 changes: 37 additions & 0 deletions AirLib/include/common/common_utils/BufferPool.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

#pragma once
#include <vector>
#include <map>
#include <memory>
#include <unordered_set>
#include <functional>
#include <limits>

typedef std::vector<uint8_t> Buffer;
typedef std::function<void(Buffer*)> Deleter;
typedef std::unique_ptr<Buffer, Deleter> BufferPtr;

class BufferPool
{
public:
BufferPtr GetBufferExactSize(size_t size);
BufferPtr GetBufferAtLeastSize(size_t size, size_t max_size = std::numeric_limits<size_t>::max());

private:
class BufferCollection
{
public:
BufferCollection(size_t size) : Size(size) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be a destructor defined for this class to avoid memory leaks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very sure about this, will need to think more and check if memory leak is happening
If destructor is defined, then copy constructor, assignment might also be required, atleast from what I understand
Any pointers, suggestions and discussions on this would be great!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct me if I'm wrong, but I believe a memory leak will occur since we are allocating Buffers using new but the buffers are never deleted. We make unique_ptrs using the allocated buffers, but we use a custom deleter so they aren't deleted. This isn't really a problem at the moment since a single static BufferPool is being used, but it could be an issue in the future if someone uses more BufferPool objects. The destructor should probably delete the pointers in AvailableBuffers so that that memory is freed. I think it would probably make sense to delete the copy constructor and assignment operator, since it doesn't really make sense to copy this kind of object.
Thanks again :)

BufferPtr DemandBuffer();
BufferPtr GetBufferIfAvailable();

private:
size_t Size;
std::unordered_set<Buffer*> AvailableBuffers;
BufferPtr MakeBufferPtr(Buffer *underlyingBuffer = nullptr);
void ReturnToCollection(Buffer *buffer);
};
std::map<size_t, BufferCollection> CollectionsBySize;
};
26 changes: 13 additions & 13 deletions AirLib/include/common/common_utils/Utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ class Utils {
}
it++;
start = it;
for (; it != end; it++) {
for (; it != end; it++) {
if (*it == ch) {
break;
}
Expand Down Expand Up @@ -585,20 +585,20 @@ class Utils {
return std::numeric_limits<TReal>::epsilon();
}

//implements relative method - do not use for comparing with zero
//use this most of the time, tolerance needs to be meaningful in your context
template<typename TReal>
static bool isApproximatelyEqual(TReal a, TReal b, TReal tolerance = epsilon<TReal>())
{
TReal diff = std::fabs(a - b);
if (diff <= tolerance)
return true;
//implements relative method - do not use for comparing with zero
//use this most of the time, tolerance needs to be meaningful in your context
template<typename TReal>
static bool isApproximatelyEqual(TReal a, TReal b, TReal tolerance = epsilon<TReal>())
{
TReal diff = std::fabs(a - b);
if (diff <= tolerance)
return true;

if (diff < std::fmax(std::fabs(a), std::fabs(b)) * tolerance)
return true;
if (diff < std::fmax(std::fabs(a), std::fabs(b)) * tolerance)
return true;

return false;
}
return false;
}

//supply tolerance that is meaningful in your context
//for example, default tolerance may not work if you are comparing double with float
Expand Down
1 change: 1 addition & 0 deletions AirLib/src/api/RpcLibClientBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ vector<ImageCaptureBase::ImageResponse> RpcLibClientBase::simGetImages(vector<Im

return RpcLibAdapatorsBase::ImageResponse::to(response_adaptor);
}

vector<uint8_t> RpcLibClientBase::simGetImage(const std::string& camera_name, ImageCaptureBase::ImageType type, const std::string& vehicle_name)
{
vector<uint8_t> result = pimpl_->client.call("simGetImage", camera_name, type, vehicle_name).as<vector<uint8_t>>();
Expand Down
35 changes: 22 additions & 13 deletions AirLib/src/api/RpcLibServerBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,18 +122,27 @@ RpcLibServerBase::RpcLibServerBase(ApiProvider* api_provider, const std::string&
pimpl_->server.bind("armDisarm", [&](bool arm, const std::string& vehicle_name) -> bool {
return getVehicleApi(vehicle_name)->armDisarm(arm);
});
pimpl_->server.bind("simGetImages", [&](const std::vector<RpcLibAdapatorsBase::ImageRequest>& request_adapter, const std::string& vehicle_name) ->
vector<RpcLibAdapatorsBase::ImageResponse> {
const auto& response = getVehicleSimApi(vehicle_name)->getImages(RpcLibAdapatorsBase::ImageRequest::to(request_adapter));
return RpcLibAdapatorsBase::ImageResponse::from(response);

pimpl_->server.bind("simGetImages", [&](const std::vector<RpcLibAdapatorsBase::ImageRequest>& request_adapter, const std::string& vehicle_name) -> vector<RpcLibAdapatorsBase::ImageResponse> {
std::vector<ImageCaptureBase::ImageResponse> responses;
for (RpcLibAdapatorsBase::ImageRequest request : request_adapter)
responses.push_back(ImageCaptureBase::ImageResponse());
getVehicleSimApi(vehicle_name)->getImages(RpcLibAdapatorsBase::ImageRequest::to(request_adapter), responses);
return RpcLibAdapatorsBase::ImageResponse::from(responses);
});

pimpl_->server.bind("simGetImage", [&](const std::string& camera_name, ImageCaptureBase::ImageType type, const std::string& vehicle_name) -> vector<uint8_t> {
auto result = getVehicleSimApi(vehicle_name)->getImage(camera_name, type);
if (result.size() == 0) {
// rpclib has a bug with serializing empty vectors, so we return a 1 byte vector instead.
result.push_back(0);
}
return result;

ImageCaptureBase::ImageRequest request(camera_name, type);
ImageCaptureBase::ImageResponse response;

getVehicleSimApi(vehicle_name)->getImage(request, response);

// rpclib has a bug with serializing empty vectors, so we return a 1 byte vector instead.
if (response.image_data_uint8->size() == 0)
response.image_data_uint8->push_back(0);

return *response.image_data_uint8;
});

pimpl_->server.bind("simGetMeshPositionVertexBuffers", [&]() ->vector<RpcLibAdapatorsBase::MeshPositionVertexBuffersResponse> {
Expand Down Expand Up @@ -309,9 +318,9 @@ RpcLibServerBase::RpcLibServerBase(ApiProvider* api_provider, const std::string&
getVehicleApi(vehicle_name)->cancelLastTask();
});

pimpl_->server.bind("simSwapTextures", [&](const std::string tag, int tex_id, int component_id, int material_id) -> std::vector<string> {
return *getWorldSimApi()->swapTextures(tag, tex_id, component_id, material_id);
});
pimpl_->server.bind("simSwapTextures", [&](const std::string tag, int tex_id, int component_id, int material_id) -> std::vector<string> {
return *getWorldSimApi()->swapTextures(tag, tex_id, component_id, material_id);
});

//if we don't suppress then server will bomb out for exceptions raised by any method
pimpl_->server.suppress_exceptions(true);
Expand Down
55 changes: 55 additions & 0 deletions AirLib/src/common/common_utils/BufferPool.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

#include "common/common_utils/BufferPool.h"

BufferPtr BufferPool::GetBufferExactSize(size_t size)
{
if (CollectionsBySize.count(size) == 0)
CollectionsBySize.insert(std::pair<size_t, BufferCollection>(size, BufferCollection(size)));

return CollectionsBySize.at(size).DemandBuffer();
}

BufferPtr BufferPool::GetBufferAtLeastSize(size_t size, size_t max_size)
{
BufferPtr buffer = nullptr;
auto closestOffering = CollectionsBySize.lower_bound(size);
if (closestOffering != CollectionsBySize.end() && closestOffering->first < max_size)
buffer = closestOffering->second.GetBufferIfAvailable();

if (buffer != nullptr)
return buffer;
return GetBufferExactSize(size);
}

BufferPtr BufferPool::BufferCollection::DemandBuffer()
{
BufferPtr buf = GetBufferIfAvailable();
if (buf != nullptr)
return buf;
return MakeBufferPtr();
}

BufferPtr BufferPool::BufferCollection::GetBufferIfAvailable()
{
if (AvailableBuffers.size() == 0)
return nullptr;

Buffer *buffer = *AvailableBuffers.begin();
AvailableBuffers.erase(buffer);
return MakeBufferPtr(buffer);
}

BufferPtr BufferPool::BufferCollection::MakeBufferPtr(Buffer *underlyingBuffer)
{
if (underlyingBuffer == nullptr)
return std::unique_ptr<Buffer, Deleter>(new Buffer(Size), std::bind(&BufferPool::BufferCollection::ReturnToCollection, this, std::placeholders::_1));
else
return std::unique_ptr<Buffer, Deleter>(underlyingBuffer, std::bind(&BufferPool::BufferCollection::ReturnToCollection, this, std::placeholders::_1));
}

void BufferPool::BufferCollection::ReturnToCollection(Buffer *buffer)
{
AvailableBuffers.insert(buffer);
}
4 changes: 2 additions & 2 deletions Examples/DataCollection/DataCollectorSGM.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ class DataCollectorSGM {
counter++;
continue;
}
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];
}

//Get SGM disparity and confidence
Expand Down
6 changes: 3 additions & 3 deletions Examples/DataCollection/StereoImageGenerator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ class StereoImageGenerator {
static void processImages(common_utils::ProsumerQueue<ImagesResult>* results)
{
while (!results->getIsDone()) {
msr::airlib::ClockBase* clock = msr::airlib::ClockFactory::get();
//msr::airlib::ClockBase* clock = msr::airlib::ClockFactory::get();

ImagesResult result;
/*ImagesResult result;
if (!results->tryPop(result)) {
clock->sleep_for(1);
continue;
Expand Down Expand Up @@ -184,7 +184,7 @@ class StereoImageGenerator {
<< " ori:" << VectorMath::toString(result.orientation)
<< " render time " << result.render_time * 1E3f << "ms"
<< " process time " << clock->elapsedSince(process_time) * 1E3f << " ms"
<< std::endl;
<< std::endl;*/

}
}
Expand Down
4 changes: 2 additions & 2 deletions Examples/DepthNav/DepthNav.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ class DepthNav {
if (response.at(1).height != params_.depth_height || response.at(1).width != params_.depth_height)
throw DepthNavException("Image Dimension mismatch. Please check right camera in the AirSim config file.");
*/
const std::vector<uint8_t>& left_image = response.at(0).image_data_uint8;
const std::vector<uint8_t>& right_image = response.at(1).image_data_uint8;
const std::vector<uint8_t> left_image(response.at(0).image_data_uint8->begin(), response.at(0).image_data_uint8->end());
const std::vector<uint8_t> right_image(response.at(1).image_data_uint8->begin(), response.at(1).image_data_uint8->end());

//baseline * focal_length = depth * disparity
float f = params_.depth_width / (2 * tan(params_.fov/2));
Expand Down
4 changes: 2 additions & 2 deletions HelloCar/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ int main()
std::getline(std::cin, path);

for (const ImageResponse& image_info : response) {
std::cout << "Image uint8 size: " << image_info.image_data_uint8.size() << std::endl;
std::cout << "Image uint8 size: " << image_info.image_data_uint8->size() << std::endl;
std::cout << "Image float size: " << image_info.image_data_float.size() << std::endl;

if (path != "") {
Expand All @@ -55,7 +55,7 @@ int main()
}
else {
std::ofstream file(file_path + ".png", std::ios::binary);
file.write(reinterpret_cast<const char*>(image_info.image_data_uint8.data()), image_info.image_data_uint8.size());
file.write(reinterpret_cast<const char*>(image_info.image_data_uint8->data()), image_info.image_data_uint8->size());
file.close();
}
}
Expand Down
4 changes: 2 additions & 2 deletions HelloDrone/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ int main()
std::getline(std::cin, path);

for (const ImageResponse& image_info : response) {
std::cout << "Image uint8 size: " << image_info.image_data_uint8.size() << std::endl;
std::cout << "Image uint8 size: " << image_info.image_data_uint8->size() << std::endl;
std::cout << "Image float size: " << image_info.image_data_float.size() << std::endl;

if (path != "") {
Expand All @@ -49,7 +49,7 @@ int main()
}
else {
std::ofstream file(file_path + ".png", std::ios::binary);
file.write(reinterpret_cast<const char*>(image_info.image_data_uint8.data()), image_info.image_data_uint8.size());
file.write(reinterpret_cast<const char*>(image_info.image_data_uint8->data()), image_info.image_data_uint8->size());
file.close();
}
}
Expand Down
Loading