options: prevent null pointer access to FrameDurationLimits #428
Conversation
|
@christianrauch thanks for the submission! Can I ask for some more context on this please? FrameDurationLimits should always be present for each camera mode, so this should never be null. Are you seeing it missing? |
|
I was experimenting with the app for a regular USB webcam without frame durations. I think it is always a good idea to check that the accessed elements are also available to avoid invalid access. |
| @@ -155,7 +155,7 @@ bool Options::Parse(int argc, char *argv[]) | |||
|
|
|||
| auto fd_ctrl = cam->controls().find(&controls::FrameDurationLimits); | |||
| auto crop_ctrl = cam->properties().get(properties::ScalerCropMaximum); | |||
| double fps = 1e6 / fd_ctrl->second.min().get<int64_t>(); | |||
| double fps = fd_ctrl==cam->controls().end() ? NAN : (1e6 / fd_ctrl->second.min().get<int64_t>()); | |||
| std::cout << std::fixed << std::setprecision(2) << "[" | |||
There was a problem hiding this comment.
I think I would probably prefer 0 instead of NAN. What do you think?
There was a problem hiding this comment.
Since 0 is a valid number, this could be interpreted as you can only take a single or no images. "NAN" signals for me that the framerate is not available or unknown. As a bonus, if someone decides to do math with it, the NAN will propagate and it will be easier visible that this operation should not be done.
There was a problem hiding this comment.
I don't think fps == 0.0 is possible for a valid configuration. But I am ok with NAN either way.
|
The CI questions the style of some code lines that I did not touch and it changes the only line I touched to something that looks weird with a large gap on the next line. Shall I really apply those suggestions to satisfy the CI? |
No need to fix those, seems the CI got something wrong when scanning the patch. Although, looking closely, it has flagged one issue: Would you be able to fix that up, then I can merge this. |
This fixes a null-pointer access when FrameDurationLimits is not available. In this case, the framerate is set to an invalid NAN.