Skip to content

Enable runtime exposure update in HDR mode - #12908

Merged
Nir-Az merged 4 commits into
realsenseai:developmentfrom
noacoohen:enable_runtime_exposure_update_in_HDR_mode
May 12, 2024
Merged

Nir-Az merged 4 commits into
realsenseai:developmentfrom
noacoohen:enable_runtime_exposure_update_in_HDR_mode

Conversation

@noacoohen

Copy link
Copy Markdown
Contributor

Enable runtime exposure update in HDR mode

@noacoohen
noacoohen requested review from Nir-Az and OhadMeir May 8, 2024 10:56
Comment thread src/ds/ds-options.cpp Resolved
Comment thread src/ds/ds-options.cpp Outdated
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread src/ds/ds-options.cpp Outdated
{
// 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
Copy Markdown
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
Copy Markdown
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
Comment thread src/ds/ds-options.cpp Resolved
Comment thread unit-tests/live/d400/test-hdr-long.py Resolved
@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