[PATCH v3 5/6] libcamera: camera_sensor_properties: Add sensor control delays

[Kieran Bingham]

Quoting Dan Scally (2024-11-15 13:35:54)
> Hi Kieran
> 
> On 15/11/2024 12:10, Kieran Bingham wrote:
> > Quoting Daniel Scally (2024-11-15 07:46:27)
> >> Add properties covering the sensor control application delays to both
> >> the static CameraSensorProperties definitions. The values used are the
> >> defaults that're in use across the library, with deviations from that
> >> taken from Raspberry Pi's CamHelper class definitions.
> >>
> >> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> >> ---
> >> Changes in v3:
> >>
> >>          - Rebased on top of the CameraSensorFactory introduction
> >>          - Some rephrasing
> >>          - Defined the sensorDelays member as empty where Raspberry Pi didn't
> >>            have any specific values. Check for the empty struct in
> >>            getSensorDelays() and return the defaults from there with a warning.
> > Sounds resonable to me.
> >
> >
> >> Changes in v2:
> >>
> >>          - Rather than adding the delays to the properties ControlList, added a
> >>            new function in CameraSensor that allows PipelineHandlers to retreive
> >>            the delay values.
> >>
> >>   include/libcamera/internal/camera_sensor.h    |  2 +
> >>   .../internal/camera_sensor_properties.h       |  9 +++
> >>   src/libcamera/sensor/camera_sensor.cpp        | 13 +++
> >>   src/libcamera/sensor/camera_sensor_legacy.cpp | 33 ++++++++
> >>   .../sensor/camera_sensor_properties.cpp       | 79 +++++++++++++++++++
> >>   5 files changed, 136 insertions(+)
> >>
> >> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> >> index 8aafd82e..a9b2d494 100644
> >> --- a/include/libcamera/internal/camera_sensor.h
> >> +++ b/include/libcamera/internal/camera_sensor.h
> >> @@ -73,6 +73,8 @@ public:
> >>          virtual const std::vector<controls::draft::TestPatternModeEnum> &
> >>          testPatternModes() const = 0;
> >>          virtual int setTestPatternMode(controls::draft::TestPatternModeEnum mode) = 0;
> >> +       virtual void getSensorDelays(uint8_t &exposureDelay, uint8_t &gainDelay,
> >> +                                    uint8_t &vblankDelay, uint8_t &hblankDelay) = 0;
> > Could/Should this return a const pointer to a struct SensorDelays now ?
> >
> Yeah maybe...that would add a requirement for the struct to be available to CameraSensor (which is a 
> Factory class now) - I don't really have any strong feelings either way, so as long as that's fine 
> then I'll make the change.

I think it's reasonable....

--
Kieran