[libcamera-devel] [PATCH 2/2] libcamera: pipeline: raspberrypi: Free buffers when a camera is released

[David Plowman]

Hi Naush

Thanks for the review.

On Fri, 11 Nov 2022 at 14:19, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Hi David,
>
> Thank you for fixing this!
>
>
> On Fri, 11 Nov 2022 at 13:30, David Plowman via libcamera-devel <libcamera-devel at lists.libcamera.org> wrote:
>>
>> Implement the PipelineHandlerRPi::releaseDevice method which allows
>> us to free any allocated buffers when a camera is released.
>>
>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>> ---
>>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> index f15fa28b..785eddf9 100644
>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> @@ -337,6 +337,8 @@ public:
>>
>>         bool match(DeviceEnumerator *enumerator) override;
>>
>> +       void releaseDevice(Camera *camera) override;
>> +
>>  private:
>>         RPiCameraData *cameraData(Camera *camera)
>>         {
>> @@ -1193,6 +1195,12 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
>>         return !!numCameras;
>>  }
>>
>> +void PipelineHandlerRPi::releaseDevice(Camera *camera)
>> +{
>> +       RPiCameraData *data = cameraData(camera);
>> +       data->freeBuffers();
>> +}
>
>
> My head's spinning with all these related changes now :-)
>
> Presumably for this to work correctly, RPiCameraData::freeBuffers() must not do the
> if (!buffersAllocated_) test, correct? So this change relies on [1] to behave correctly?

Well, I think this is only a problem when the camera was started (and
stopped and released), otherwise we never allocate buffers in the
first place. So I think it's good even regardless of the
buffersAllocated_ test. But yeah, my head is spinning too. Just merge
everything and it'll be fine!!

David

>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
> [1] https://patchwork.libcamera.org/patch/17759/
>
>
>> +
>>  int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, MediaEntity *sensorEntity)
>>  {
>>         std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
>> --
>> 2.30.2
>>