Fix formats-converter bug #12029
Fix formats-converter bug #12029
Conversation
…same converted profile
src/proc/formats-converter.cpp
Outdated
|
|
||
| if( profile ) | ||
| { | ||
| res += std::string( "(" ) + rs2_stream_to_string( profile->get_stream_type() ) + std::string( ")" ); |
There was a problem hiding this comment.
Wouldn't it be cleaner to just use ostringstream instead of all the std::string conversions? Or even an operator<<?
There was a problem hiding this comment.
Updated to operator<<
| @@ -57,38 +57,34 @@ void formats_converter::drop_non_basic_formats() | |||
| } | |||
| } | |||
|
|
|||
| std::string to_string( std::shared_ptr< stream_profile_interface > profile ) | |||
| std::ostream & operator<<( std::ostream & os, const std::shared_ptr< stream_profile_interface > & profile ) | |||
There was a problem hiding this comment.
Can you please add the required include on top?
So we will not be depending on other headers?
| @@ -57,6 +59,24 @@ void formats_converter::drop_non_basic_formats() | |||
| } | |||
| } | |||
|
|
|||
| std::ostream & operator<<( std::ostream & os, const std::shared_ptr< stream_profile_interface > & profile ) | |||
There was a problem hiding this comment.
This is a very useful function and we keep re-implementing it in one way or another all over the place.
I suggest it be placed in a central place and not just here.
src/core/streaming.h is where stream_profile_interface is defined, I suggest the signature there and the body in a .cpp somwhere.
There was a problem hiding this comment.
p.s. we have to choose whether the syntax would be:
... << stm_ptr;
or
... << *stm_ptr;
I.e., do we pass the (SMART) POINTER to the stream interface, or a reference to it? I think we're not always going to have a smart ptr. I vote for a reference, if possible.
There was a problem hiding this comment.
A reference will be problematic since it is an abstract class. Maybe we can pass a raw pointer but I don't think that we use it.
There was a problem hiding this comment.
A general operator for stream_profile_interface is implemented here. It does not print all the details I wanted so I added specifically here. Do you think we need it also elsewhere? No other implementations/uses that I was able to find.
There was a problem hiding this comment.
A reference will be problematic since it is an abstract class. Maybe we can pass a raw pointer but I don't think that we use it.
A ref is just syntax, it doesn't mean anything. You can take its address, compare it to null, etc., same as any pointer.
There was a problem hiding this comment.
A general operator for
stream_profile_interfaceis implemented here.
That's for stream_profiles. I vote for yours, and change that one to use it.
|
We are waiting for this fix in several other branches, we cannot merge D457 changes for ADL-P safely |
@maloel note that we are blind in GHA DDS tests, we will not rerun until it will pass.. |
¯\_(ツ)_/¯ I'm just commenting on something minor, up to you... |
The bug happened when more than one raw profile have mapped to the same converted profile.
Saved shared_ptr object in map instead of actual profile values. Different shared_ptr objects did not map as the same key even when pointing to profiles of same values.