[libcamera-devel] [PATCH v1 2/6] pipeline: raspberrypi: Move freeBuffers() to the RPiCameraData class

[David Plowman]

Hi Naush

Thanks for this patch!

On Mon, 7 Mar 2022 at 12:46, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> This function used to clear the camera buffers does not belong in the
> PipelineHandlerRPi class as it only access members of the RPiCameraData,
> so move it.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp         | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index d604c473c26c..3781e4e0e3c4 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -189,6 +189,7 @@ public:
>         {
>         }
>
> +       void freeBuffers();
>         void frameStarted(uint32_t sequence);
>
>         int loadIPA(ipa::RPi::SensorConfig *sensorConfig);
> @@ -329,7 +330,6 @@ private:
>         int registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity);
>         int queueAllBuffers(Camera *camera);
>         int prepareBuffers(Camera *camera);
> -       void freeBuffers(Camera *camera);
>         void mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask);
>  };
>
> @@ -1049,7 +1049,7 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)
>         /* Stop the IPA. */
>         data->ipa_->stop();
>
> -       freeBuffers(camera);
> +       data->freeBuffers();
>  }
>
>  int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
> @@ -1445,16 +1445,14 @@ void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffer
>         data->ipa_->mapBuffers(ipaBuffers);
>  }
>
> -void PipelineHandlerRPi::freeBuffers(Camera *camera)
> +void RPiCameraData::freeBuffers()
>  {
> -       RPiCameraData *data = cameraData(camera);
> -
>         /* Copy the buffer ids from the unordered_set to a vector to pass to the IPA. */
> -       std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end());
> -       data->ipa_->unmapBuffers(ipaBuffers);
> -       data->ipaBuffers_.clear();
> +       std::vector<unsigned int> ipaBuffers(ipaBuffers_.begin(), ipaBuffers_.end());
> +       ipa_->unmapBuffers(ipaBuffers);
> +       ipaBuffers_.clear();
>
> -       for (auto const stream : data->streams_)
> +       for (auto const stream : streams_)
>                 stream->releaseBuffers();
>  }
>
> --
> 2.25.1
>

I agree this is a bit tidier, and of course is being tested as part of
picamera2.

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Tested-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David