Skip to content

Conversation

@noacoohen
Copy link
Contributor

Enable runtime exposure update in HDR mode

@noacoohen noacoohen requested review from Nir-Az and OhadMeir May 8, 2024 10:56
auto _current_hdr_sequence_index = _hdr_cfg->get( RS2_OPTION_SEQUENCE_ID );
// In order to enable runtime exposure update in HDR mode we first send disable HDR, then set the value
// and at last enable HDR again.
_hdr_cfg->set( RS2_OPTION_HDR_ENABLED, 0, { 0, 1, 1 } );
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 {0,1,1} part?
Can't we use LibRS option instead?
I mean replace for example
_hdr_cfg->set( RS2_OPTION_HDR_ENABLED, 0, { 0, 1, 1 } );
with
set_option(HDR_ENABLE, false)
?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that set_option is another level of abstract and that it is better to keep it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

The interface requires a range, we supply { 0, 1, 1 } means on or off

{
// keep the index
auto _current_hdr_sequence_index = _hdr_cfg->get( RS2_OPTION_SEQUENCE_ID );
// In order to enable runtime exposure update in HDR mode we first send disable HDR, then set the value
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more clean if all the comments in this block will be replaced with one comment in the block head.
Something like this:
Changing exposure while HDR is enabled was requested by customers. It is currently disabled by D400 FW, so we workaround it by disabling HDR, changing exposure and enabling again. Disabling/Enabling resets the sequence index so we need to keep and restore it.

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

@noacoohen noacoohen force-pushed the enable_runtime_exposure_update_in_HDR_mode branch from e57e4b9 to 98a443f Compare May 8, 2024 12:58
@Nir-Az Nir-Az merged commit 998aaa1 into realsenseai:development May 12, 2024
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.

3 participants