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

Fix D405 VGA resolution intrinsics calculation #13060

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

OhadMeir
Copy link
Contributor

Tracked on [RSDSO-18999]

@OhadMeir OhadMeir requested a review from Nir-Az June 20, 2024 09:31
//D405 needs special calculation because the ISP crops the full sensor image using non linear transformation.
rs2_intrinsics get_d405_color_stream_intrinsic(const std::vector<uint8_t>& raw_data, uint32_t width, uint32_t height)
{
// Convert normalized focal lenght and principal point to pixel units (K matrix format)
Copy link
Collaborator

Choose a reason for hiding this comment

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

length

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

k( 2, 1 ) = ( k( 2, 1 ) + 1 ) * resolutions_list[res].y / 2.f; // ppy
};

// Scale focal lenght and principal point in pixel units from one resolution to another (K matrix format)
Copy link
Collaborator

Choose a reason for hiding this comment

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

length

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

};

auto table = check_calib< ds::d400_rgb_calibration_table >( raw_data );
auto raw_res = width_height_to_ds_rect_resolutions( 1280, 800 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain why we take this resolution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the raw sensor resolution.
These values where also hard coded in the matlab algorithm I was based on. Special for D405 handling.

rs2_intrinsics get_d405_color_stream_intrinsic(const std::vector<uint8_t>& raw_data, uint32_t width, uint32_t height)
{
// Convert normalized focal length and principal point to pixel units (K matrix format)
auto k_to_pixels = [&]( float3x3 & k, ds_rect_resolutions res )
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename k_to_pixels to normalized_k_to_pixels

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

k( 2, 0 ) = k( 2, 0 ) - ( resolutions_list[scale_res].x - resolutions_list[output_res].x ) / 2; // ppx
k( 2, 1 ) = k( 2, 1 ) - ( resolutions_list[scale_res].y - resolutions_list[output_res].y ) / 2; // ppy
}
else
{
k_to_pixels( k, calibration_res );
normalized_k_to_pixels( k, calibration_res );
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the difference between normalized_k_to_pixels and k_to_pixels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No difference, just renaming based on Remi's comment.
The code always assumes a "normalized k" input and changes it to pixel units based on the requested resolution.

@OhadMeir OhadMeir closed this Jun 26, 2024
@OhadMeir OhadMeir reopened this Jun 26, 2024
@OhadMeir
Copy link
Contributor Author

Close and reopen to trigger CI

@OhadMeir OhadMeir merged commit fcea8fb into IntelRealSense:development Jun 26, 2024
42 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants