-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Enable runtime exposure update in HDR mode #12908
Enable runtime exposure update in HDR mode #12908
Conversation
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 } ); |
There was a problem hiding this comment.
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)
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
e57e4b9
to
98a443f
Compare
Enable runtime exposure update in HDR mode