Enable rotate frames for RGB camera #13733
Conversation
|
Please check CI failures |
src/proc/rotation-filter.cpp
Outdated
| auto src = f.as< rs2::video_frame >(); | ||
| rs2::stream_profile profile = f.get_profile(); | ||
| auto format = f.get_profile().format(); |
There was a problem hiding this comment.
We already have the profile from 1 line above, you can do profile.format() here right?
src/proc/rotation-filter.cpp
Outdated
| if( ! frame || frame.is< rs2::frameset >() ) | ||
| return false; | ||
| auto type = frame.get_profile().stream_type(); | ||
| /// change extrinsics |
| return false; | ||
| auto type = frame.get_profile().stream_type(); | ||
| /// change extrinsics | ||
| for( auto & stream_type : _streams_to_rotate ){ |
There was a problem hiding this comment.
No need for { } for one line if/for. Where you are using, { should be placed on a new line.
| @@ -52,32 +53,51 @@ namespace librealsense { | |||
| register_option( RS2_OPTION_ROTATION, rotation_control ); | |||
| } | |||
|
|
|||
| rotation_filter::rotation_filter( std::vector< rs2_stream > streams_to_rotate ) | |||
There was a problem hiding this comment.
Code duplication. Is default constructor needed? If so have it call the other constructor with an empty vector {}.
src/proc/rotation-filter.cpp
Outdated
| if( ! strong_rotation_control ) | ||
| return; | ||
|
|
||
| std::lock_guard< std::mutex > lock( _mutex ); |
There was a problem hiding this comment.
Lock after validating if valid value.
src/proc/rotation-filter.cpp
Outdated
|
|
||
| register_option( RS2_OPTION_ROTATION, rotation_control ); | ||
| } | ||
|
|
||
| rs2::frame rotation_filter::process_frame(const rs2::frame_source& source, const rs2::frame& f) | ||
| { | ||
| if( _value == rotation_default_val ) |
There was a problem hiding this comment.
Should lock _mutex or place _value in a local variable and use it all over this function.
There was a problem hiding this comment.
Same for update_output_profile and rotate_frame.
src/proc/rotation-filter.cpp
Outdated
| { | ||
| if( _value == 90 || _value == -90 ) | ||
| { | ||
| LOG_ERROR( "Rotating YUYV format is disabled for 90 or -90 degrees" ); |
There was a problem hiding this comment.
Do we intend to support this at a future PR?
There was a problem hiding this comment.
No support for 90 and 270 degrees rotation for YUYV format
src/proc/rotation-filter.cpp
Outdated
| @@ -220,6 +279,38 @@ namespace librealsense { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| void rotation_filter::rotate_YUYV_frame(uint8_t* const out, const uint8_t* source, int width, int height) { | |||
There was a problem hiding this comment.
Please use formatter. Spaces, { at new line, etc...
src/proc/rotation-filter.cpp
Outdated
| break; | ||
| } | ||
| case 3: { | ||
| rotate_frame< 3 >( static_cast< uint8_t * >( const_cast< void * >( tgt.get_data() ) ), |
There was a problem hiding this comment.
Seems that using bpp as a normal function parameter instead of template will reduce code duplication.
Tracked on RSDSO-19999