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

[Naushir Patuck]

Hi David,

Thank you for the patch.

On Wed, 2 Dec 2020 at 11:53, David Plowman <david.plowman at 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 at raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20201204/a26d5f70/attachment-0001.htm>