MD performance and misc changes #11600
Conversation
| LOG_DEBUG( "-----> JSON size = " << stream_header_message.json_data().dump().length() ); | ||
| LOG_DEBUG( "-----> CBOR size = " << json::to_cbor( stream_header_message.json_data() ).size() ); | ||
| auto json_string = slice( stream_header_message.custom_data< char const >(), stream_header_message._data.size() ); | ||
| LOG_DEBUG( "-----> JSON = " << shorten_json_string( json_string, 300 ) << " size " << json_string.length() ); |
There was a problem hiding this comment.
Why 300? I think that if we shorten it than about a line width, 120 chars, should be better
There was a problem hiding this comment.
We have lots of info, so this depends on the message, I found 300 to be not too long and long enough that you could see the top-level json elements and their values.
| @@ -117,11 +120,11 @@ static void on_discovery_stream_header( std::shared_ptr< dds_stream_server > con | |||
| { "stream-name", stream->name() }, | |||
| { "options" , stream_options }, | |||
| { "intrinsics" , intrinsics }, | |||
| { "recommended-filters", stream_filters }, | |||
| { "recommended-filters", std::move( stream_filters ) }, | |||
There was a problem hiding this comment.
Won't move be better also for intrinsics and options?
There was a problem hiding this comment.
You could do it for everything, but really the biggest value would be for compound json objects or objects that allocated memory of their own. I don't believe intrinsics fit, but you're right about options.
There was a problem hiding this comment.
Changed. Will be in next PR.
| @@ -4,6 +4,8 @@ Copyright(c) 2017 Intel Corporation. All Rights Reserved. */ | |||
| #include "pyrealsense2.h" | |||
| #include <librealsense2/rs.hpp> | |||
|
|
|||
| #include <src/image.cpp> // bad idea? for get_image_bpp | |||
There was a problem hiding this comment.
Including image.h won't work?
There was a problem hiding this comment.
No, the function is not exposed thru rs2. I needed the actual implementation as part of the pyd.
There was a problem hiding this comment.
OK. I don't like including cpp files but it is in the python wrapper so lesser evil.
Exposing it in the API is currently not necessary.
This is an interim PR, to avoid massive changes all together