[libcamera-devel] [v2 PATCH] android: mm: cros_camera_buffer: Log failure error on cleanup

[Hirokazu Honda]

Hi Umang, thank you for the patch.

On Tue, Sep 7, 2021 at 11:59 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Failure can still happen by CameraBufferManager during Unlock() and/or
> Deregister() of camera3Buffer handles. We should be logging those
> errors as well.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> Changes in v2:
> - Remove a debug log that creeped in by mistake
>
> I have been able to spot one of the failure which is happening on my
> in-developement async post-processing. It is due a failure in
> Deregister(). It is intermittent and non-fatal as far as I can see.
>
> Having a failure log in HAL, apart from https://paste.debian.net/1210728/
> helps to know the exact place from where the error is originating from
> libcamera HAL.
> ---
>  src/android/mm/cros_camera_buffer.cpp | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> index ec45e04c..86770135 100644
> --- a/src/android/mm/cros_camera_buffer.cpp
> +++ b/src/android/mm/cros_camera_buffer.cpp
> @@ -73,10 +73,20 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
>
>  CameraBuffer::Private::~Private()
>  {
> -       if (mapped_)
> -               bufferManager_->Unlock(handle_);
> -       if (registered_)
> -               bufferManager_->Deregister(handle_);
> +       int ret;
> +       if (mapped_) {
> +               ret = bufferManager_->Unlock(handle_);
> +               if (ret != 0)
> +                       LOG(HAL, Error) << "Failed to unlock buffer: "
> +                                       << strerror(-ret);
> +       }
> +
> +       if (registered_) {
> +               ret = bufferManager_->Deregister(handle_);
> +               if (ret != 0)
> +                       LOG(HAL, Error) << "Failed to deregister buffer: "
> +                                       << strerror(-ret);
> +       }
>  }
>
>  unsigned int CameraBuffer::Private::numPlanes() const
> --
> 2.31.0
>