[libcamera-devel,2/5] src: raspberrypi: Pass the drop frame count in start, not configure

10543 diff mbox series

The number of frames to drop (not display) is passed back now from the
start method, not configure. This means applications have a chance to
set fixed exposure/gain before starting the camera and this can affect
the frame drop count that is returned.

Both the IPA implementation file and the pipeline handler need
matching modifications.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/raspberrypi.cpp           | 38 +++++++++----------
 .../pipeline/raspberrypi/raspberrypi.cpp      | 25 ++++++------
 2 files changed, 32 insertions(+), 31 deletions(-)

Hi David,

Thank you for the patch.

On Wed, 2 Dec 2020 at 11:53, David Plowman <david.plowman@raspberrypi.com>
wrote:

> The number of frames to drop (not display) is passed back now from the
> start method, not configure. This means applications have a chance to
> set fixed exposure/gain before starting the camera and this can affect
> the frame drop count that is returned.
>
> Both the IPA implementation file and the pipeline handler need
> matching modifications.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  src/ipa/raspberrypi/raspberrypi.cpp           | 38 +++++++++----------
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 25 ++++++------
>  2 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index b8298768..0300b8d9 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -182,6 +182,25 @@ int IPARPi::start(const IPAOperationData &ipaConfig,
> IPAOperationData *result)
>                 result->operation |= RPi::IPA_CONFIG_SENSOR;
>         }
>
> +       /*
> +        * Initialise frame counts, and decide how many frames must be
> hidden or
> +        *"mistrusted", which depends on whether this is a startup from
> cold,
> +        * or merely a mode switch in a running system.
> +        */
> +       frameCount_ = 0;
> +       checkCount_ = 0;
> +       unsigned int dropFrame = 0;
> +       if (firstStart_) {
> +               dropFrame = helper_->HideFramesStartup();
> +               mistrustCount_ = helper_->MistrustFramesStartup();
> +       } else {
> +               dropFrame = helper_->HideFramesModeSwitch();
> +               mistrustCount_ = helper_->MistrustFramesModeSwitch();
> +       }
> +
> +       result->data.push_back(dropFrame);
> +       result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
> +
>         firstStart_ = false;
>
>         return 0;
> @@ -298,25 +317,6 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
>         /* Pass the camera mode to the CamHelper to setup algorithms. */
>         helper_->SetCameraMode(mode_);
>
> -       /*
> -        * Initialise frame counts, and decide how many frames must be
> hidden or
> -        *"mistrusted", which depends on whether this is a startup from
> cold,
> -        * or merely a mode switch in a running system.
> -        */
> -       frameCount_ = 0;
> -       checkCount_ = 0;
> -       unsigned int dropFrame = 0;
> -       if (controllerInit_) {
> -               dropFrame = helper_->HideFramesModeSwitch();
> -               mistrustCount_ = helper_->MistrustFramesModeSwitch();
> -       } else {
> -               dropFrame = helper_->HideFramesStartup();
> -               mistrustCount_ = helper_->MistrustFramesStartup();
> -       }
> -
> -       result->data.push_back(dropFrame);
> -       result->operation |= RPi::IPA_CONFIG_DROP_FRAMES;
> -
>         if (!controllerInit_) {
>                 /* Load the tuning file for this sensor. */
>                 controller_.Read(tuningFile_.c_str());
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 89a44763..5ae56628 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -745,13 +745,6 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
>                 return ret;
>         }
>
> -       ret = queueAllBuffers(camera);
> -       if (ret) {
> -               LOG(RPI, Error) << "Failed to queue buffers";
> -               stop(camera);
> -               return ret;
> -       }
> -
>         /* Check if a ScalerCrop control was specified. */
>         if (controls)
>                 data->applyScalerCrop(*controls);
> @@ -778,6 +771,19 @@ int PipelineHandlerRPi::start(Camera *camera,
> [[maybe_unused]] ControlList *cont
>                         LOG(RPI, Error) << "V4L2 staggered set failed";
>         }
>
> +       if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {
> +               /* Configure the number of dropped frames required on
> startup. */
> +               data->dropFrameCount_ = result.data[0];
> +       }
> +
> +       /* We need to set the dropFrameCount_ before queueing buffers. */
> +       ret = queueAllBuffers(camera);
> +       if (ret) {
> +               LOG(RPI, Error) << "Failed to queue buffers";
> +               stop(camera);
> +               return ret;
> +       }
> +
>         /*
>          * IPA configure may have changed the sensor flips - hence the
> bayer
>          * order. Get the sensor format and set the ISP input now.
> @@ -1231,11 +1237,6 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
>                         LOG(RPI, Error) << "V4L2 staggered set failed";
>         }
>
> -       if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {
> -               /* Configure the number of dropped frames required on
> startup. */
> -               dropFrameCount_ = result.data[resultIdx++];
> -       }
> -
>         /*
>          * Configure the H/V flip controls based on the combination of
>          * the sensor and user transform.
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>