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.

Sign up for GitHub

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

fix pointcloud map_to #2371

Merged
merged 2 commits into from
Sep 16, 2018
Merged

fix pointcloud map_to #2371

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab254a5
fix pointcloud map_to
matkatz Sep 9, 2018
613beae
pointcloud map_to, CR - review
matkatz Sep 16, 2018
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
4 changes: 2 additions & 2 deletions src/proc/colorizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ namespace librealsense
colorizer::colorizer()
: _min(0.f), _max(6.f), _equalize(true), _stream()
{
_stream_filter = RS2_STREAM_DEPTH;
_stream_format_filter = RS2_FORMAT_Z16;
_stream_filter.stream = RS2_STREAM_DEPTH;
_stream_filter.format = RS2_FORMAT_Z16;

_maps = { &jet, &classic, &grayscale, &inv_grayscale, &biomes, &cold, &warm, &quantized, &pattern };

Expand Down
4 changes: 2 additions & 2 deletions src/proc/decimation-filter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ namespace librealsense
_recalc_profile(false),
_options_changed(false)
{
_stream_filter = RS2_STREAM_DEPTH;
_stream_format_filter = RS2_FORMAT_Z16;
_stream_filter.stream = RS2_STREAM_DEPTH;
_stream_filter.format = RS2_FORMAT_Z16;

auto decimation_control = std::make_shared<ptr_option<uint8_t>>(
decimation_min_val,
Expand Down
4 changes: 2 additions & 2 deletions src/proc/hole-filling-filter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ namespace librealsense
_current_frm_size_pixels(0),
_hole_filling_mode(hole_fill_def)
{
_stream_filter = RS2_STREAM_DEPTH;
_stream_format_filter = RS2_FORMAT_Z16;
_stream_filter.stream = RS2_STREAM_DEPTH;
_stream_filter.format = RS2_FORMAT_Z16;

auto hole_filling_mode = std::make_shared<ptr_option<uint8_t>>(
hole_fill_min,
Expand Down
25 changes: 14 additions & 11 deletions src/proc/pointcloud.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ namespace librealsense
_depth_intrinsics = optional_value<rs2_intrinsics>();
_depth_units = optional_value<float>();
_extrinsics = optional_value<rs2_extrinsics>();
_other_stream = nullptr;
}

bool found_depth_intrinsics = false;
Expand Down Expand Up @@ -115,13 +114,19 @@ namespace librealsense

void pointcloud::inspect_other_frame(const rs2::frame& other)
{
if (_other_stream != nullptr && other.get_profile().as<rs2::video_stream_profile>() == *_other_stream.get())
if (_stream_filter != _prev_stream_filter)
_other_stream = nullptr;
_prev_stream_filter = _stream_filter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be encapsulated with the previous line under the "if.." statement


if (_extrinsics.has_value() && _other_stream != nullptr && other.get_profile().as<rs2::video_stream_profile>() == *_other_stream.get())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using "(!_extrinsics.has_value() || .." would be more readable, i.e. maintainable

return;

_other_stream = nullptr;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The "if" in the next line is redundant now, and should be removed to make it straight-forward: i.e:
_other_stream = <override_value>


if (!_other_stream.get())
{
auto osp = other.get_profile().as<rs2::video_stream_profile>();
_other_stream = std::make_shared<rs2::video_stream_profile>(osp.clone(osp.stream_type(), osp.stream_index(), osp.format()));
auto osp = other.get_profile().get();
_other_stream = std::make_shared<rs2::video_stream_profile>(rs2::stream_profile(osp).as<rs2::video_stream_profile>());
_other_intrinsics = optional_value<rs2_intrinsics>();
_extrinsics = optional_value<rs2_extrinsics>();
}
Expand Down Expand Up @@ -464,8 +469,6 @@ namespace librealsense
pointcloud::pointcloud() :
_other_stream(nullptr)
{
_stream_filter = RS2_STREAM_ANY;

_occlusion_filter = std::make_shared<occlusion_filter>();

auto occlusion_invalidation = std::make_shared<ptr_option<uint8_t>>(
Expand Down Expand Up @@ -500,10 +503,10 @@ namespace librealsense
if (set)
{
//process composite frame only if it contains both a depth frame and the requested texture frame
if (_stream_filter == RS2_STREAM_ANY)
if (_stream_filter.stream == RS2_STREAM_ANY)
return false;

auto tex = set.first_or_default(_stream_filter, _stream_format_filter);
auto tex = set.first_or_default(_stream_filter.stream, _stream_filter.format);
if (!tex)
return false;
auto depth = set.first_or_default(RS2_STREAM_DEPTH, RS2_FORMAT_Z16);
Expand All @@ -516,7 +519,7 @@ namespace librealsense
return true;

auto p = frame.get_profile();
if (p.stream_type() == _stream_filter && p.format() == _stream_format_filter && p.stream_index() == _stream_index_filter)
if (p.stream_type() == _stream_filter.stream && p.format() == _stream_filter.format && p.stream_index() == _stream_filter.index)
return true;
return false;

Expand All @@ -540,7 +543,7 @@ namespace librealsense
inspect_depth_frame(depth);
rv = process_depth_frame(source, depth);

auto texture = composite.first(_stream_filter);
auto texture = composite.first(_stream_filter.stream);
inspect_other_frame(texture);
}
else
Expand All @@ -551,7 +554,7 @@ namespace librealsense
inspect_depth_frame(f);
rv = process_depth_frame(source, f);
}
if (f.get_profile().stream_type() == _stream_filter && f.get_profile().format() == _stream_format_filter)
if (f.get_profile().stream_type() == _stream_filter.stream && f.get_profile().format() == _stream_filter.format)
{
inspect_other_frame(f);
}
Expand Down
2 changes: 2 additions & 0 deletions src/proc/pointcloud.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace librealsense
{
class occlusion_filter;

class pointcloud : public stream_filter_processing_block
{
public:
Expand Down Expand Up @@ -37,5 +38,6 @@ namespace librealsense
std::vector<float> _pre_compute_map_y;

void pre_compute_x_y_map();
stream_filter _prev_stream_filter;
};
}
4 changes: 2 additions & 2 deletions src/proc/spatial-filter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ namespace librealsense
_holes_filling_mode(holes_fill_def),
_holes_filling_radius(0)
{
_stream_filter = RS2_STREAM_DEPTH;
_stream_format_filter = RS2_FORMAT_Z16;
_stream_filter.stream = RS2_STREAM_DEPTH;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to be move to initialization list, so if one day one other class member become dependent on this struct, the initialization order will be preserved, or at least compiler warning will be generated

_stream_filter.format = RS2_FORMAT_Z16;

auto spatial_filter_alpha = std::make_shared<ptr_option<float>>(
alpha_min_val,
Expand Down
38 changes: 13 additions & 25 deletions src/proc/synthetic-stream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,12 @@ namespace librealsense
return source.allocate_composite_frame(results);
}

stream_filter_processing_block::stream_filter_processing_block() :
_stream_filter(RS2_STREAM_ANY),
_stream_format_filter(RS2_FORMAT_ANY),
_stream_index_filter(-1)
stream_filter_processing_block::stream_filter_processing_block()
{
register_option(RS2_OPTION_FRAMES_QUEUE_SIZE, _source.get_published_size_option());
_source.init(std::shared_ptr<metadata_parser_map>());

auto stream_selector = std::make_shared<ptr_option<int>>(RS2_STREAM_ANY, RS2_STREAM_FISHEYE, 1, RS2_STREAM_ANY, (int*)&_stream_filter, "Stream type");
auto stream_selector = std::make_shared<ptr_option<int>>(RS2_STREAM_ANY, RS2_STREAM_FISHEYE, 1, RS2_STREAM_ANY, (int*)&_stream_filter.stream, "Stream type");
for (int s = RS2_STREAM_ANY; s < RS2_STREAM_COUNT; s++)
{
stream_selector->set_description(s, "Process - " + std::string (rs2_stream_to_string((rs2_stream)s)));
Expand All @@ -139,10 +136,10 @@ namespace librealsense
throw invalid_value_exception(to_string()
<< "Unsupported stream filter, " << val << " is out of range.");

_stream_filter = static_cast<rs2_stream>((int)val);
_stream_filter.stream = static_cast<rs2_stream>((int)val);
});

auto format_selector = std::make_shared<ptr_option<int>>(RS2_FORMAT_ANY, RS2_FORMAT_DISPARITY32, 1, RS2_FORMAT_ANY, (int*)&_stream_format_filter, "Stream format");
auto format_selector = std::make_shared<ptr_option<int>>(RS2_FORMAT_ANY, RS2_FORMAT_DISPARITY32, 1, RS2_FORMAT_ANY, (int*)&_stream_filter.format, "Stream format");
for (int f = RS2_FORMAT_ANY; f < RS2_FORMAT_COUNT; f++)
{
format_selector->set_description(f, "Process - " + std::string(rs2_format_to_string((rs2_format)f)));
Expand All @@ -155,10 +152,10 @@ namespace librealsense
throw invalid_value_exception(to_string()
<< "Unsupported stream format filter, " << val << " is out of range.");

_stream_format_filter = static_cast<rs2_format>((int)val);
_stream_filter.format = static_cast<rs2_format>((int)val);
});

auto index_selector = std::make_shared<ptr_option<int>>(0, std::numeric_limits<int>::max(), 1, -1, &_stream_index_filter, "Stream index");
auto index_selector = std::make_shared<ptr_option<int>>(0, std::numeric_limits<int>::max(), 1, -1, &_stream_filter.index, "Stream index");
index_selector->on_set([this, index_selector](float val)
{
std::lock_guard<std::mutex> lock(_mutex);
Expand All @@ -167,7 +164,7 @@ namespace librealsense
throw invalid_value_exception(to_string()
<< "Unsupported stream index filter, " << val << " is out of range.");

_stream_index_filter = (int)val;
_stream_filter.index = (int)val;
});

register_option(RS2_OPTION_STREAM_FILTER, stream_selector);
Expand All @@ -191,20 +188,20 @@ namespace librealsense
rs2_format format = profile.format();
int index = profile.stream_index();

if (_stream_filter != RS2_STREAM_ANY && _stream_filter != stream)
if (_stream_filter.stream != RS2_STREAM_ANY && _stream_filter.stream != stream)
return false;
if (is_z_or_disparity(_stream_format_filter))
if (is_z_or_disparity(_stream_filter.format))
{
if (_stream_format_filter != RS2_FORMAT_ANY && !is_z_or_disparity(format))
if (_stream_filter.format != RS2_FORMAT_ANY && !is_z_or_disparity(format))
return false;
}
else
{
if (_stream_format_filter != RS2_FORMAT_ANY && _stream_format_filter != format)
if (_stream_filter.format != RS2_FORMAT_ANY && _stream_filter.format != format)
return false;
}

if (_stream_index_filter != -1 && _stream_index_filter != index)
if (_stream_filter.index != -1 && _stream_filter.index != index)
return false;
return true;
}
Expand All @@ -217,16 +214,7 @@ namespace librealsense
rs2_stream stream = profile.stream_type();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Those three lines seem to be redundant after refactoring

rs2_format format = profile.format();
int index = profile.stream_index();

if (_stream_filter != RS2_STREAM_ANY && _stream_filter != stream)
return false;

if (_stream_format_filter != RS2_FORMAT_ANY && _stream_format_filter != format)
return false;

if (_stream_index_filter != -1 && _stream_index_filter != index)
return false;
return true;
return _stream_filter.match(frame);
}

void synthetic_source::frame_ready(frame_holder result)
Expand Down
54 changes: 51 additions & 3 deletions src/proc/synthetic-stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,64 @@ namespace librealsense
virtual rs2::frame process_frame(const rs2::frame_source& source, const rs2::frame& f) = 0;
};

struct stream_filter
{
rs2_stream stream;
rs2_format format;
int index;

stream_filter() : stream(RS2_STREAM_ANY), format(RS2_FORMAT_ANY), index(-1) {}
stream_filter(rs2_stream s, rs2_format f, int i) : stream(s), format(f), index(i) {}

bool match(const rs2::frame& frame)
{
stream_filter filter(frame.get_profile().stream_type(), frame.get_profile().format(), frame.get_profile().stream_index());
return match(filter);
}

bool match(const stream_filter& other)
{
if (stream != RS2_STREAM_ANY && stream != other.stream)
return false;
if (format != RS2_FORMAT_ANY && format != other.format)
return false;
if (index != -1 && index != other.index)
return false;
return true;
}

bool operator!=(const stream_filter& other)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the == and ~= operator be swapped, i.e to implement != by terms of == ?
The (!(!(something)) is really counter-intuitive and hard to maintain

{
if (stream != other.stream)
return true;
if (format != other.format)
return true;
if (index != other.index)
return true;
return false;
}

bool operator==(const stream_filter& other)
{
return !(*this != other);
}

void operator=(const stream_filter& other)
{
stream = other.stream;
format = other.format;
index = other.index;
}
};

class stream_filter_processing_block : public generic_processing_block
{
public:
stream_filter_processing_block();
virtual ~stream_filter_processing_block() { _source.flush(); }

protected:
rs2_stream _stream_filter;
rs2_format _stream_format_filter;
int _stream_index_filter;
stream_filter _stream_filter;

bool should_process(const rs2::frame& frame) override;
};
Expand Down
4 changes: 2 additions & 2 deletions src/proc/temporal-filter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ namespace librealsense
_extension_type(RS2_EXTENSION_DEPTH_FRAME),
_current_frm_size_pixels(0)
{
_stream_filter = RS2_STREAM_DEPTH;
_stream_format_filter = RS2_FORMAT_Z16;
_stream_filter.stream = RS2_STREAM_DEPTH;
_stream_filter.format = RS2_FORMAT_Z16;

auto temporal_persistence_control = std::make_shared<ptr_option<uint8_t>>(
persistence_min,
Expand Down