[libcamera-devel] [PATCH v2 2/2] android: Check if Stream configurations were generated correctly

[Hirokazu Honda]

Hi Javier, thank you for the patch.

On Sat, Oct 2, 2021 at 6:16 AM Javier Martinez Canillas
<javierm at redhat.com> wrote:
>
> The libcamera Android Camera HAL generates camera configurations for the
> StillCapture, Raw and ViewFinder stream roles. But there is only a check
> if the configuration generation failed, for the StillCapture stream role.
>
> This could lead to a NULL pointer dereference if a pipeline handler fails
> to generate a default configuration for one of the other two stream roles.
>
> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
> ---
>
> Changes in v2:
> - Fix typos in commit message.
>
>  src/android/camera_capabilities.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 87a6e1c6f26..baeedc11500 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -408,6 +408,11 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>         std::vector<Size> supportedResolutions;
>         std::unique_ptr<CameraConfiguration> cameraConfig =
>                 camera_->generateConfiguration({ StreamRole::Viewfinder });
> +       if (!cameraConfig) {
> +               LOG(HAL, Error) << "Failed to get supported YUV resolutions";
> +               return supportedResolutions;
> +       }
> +
>         StreamConfiguration &cfg = cameraConfig->at(0);
>
>         for (const Size &res : resolutions) {
> @@ -431,11 +436,17 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
>  std::vector<Size>
>  CameraCapabilities::initializeRawResolutions(const PixelFormat &pixelFormat)
>  {
> +       std::vector<Size> supportedResolutions;
>         std::unique_ptr<CameraConfiguration> cameraConfig =
>                 camera_->generateConfiguration({ StreamRole::Raw });
> +       if (!cameraConfig) {
> +               LOG(HAL, Error) << "Failed to get supported Raw resolutions";
> +               return supportedResolutions;

return {} may work?
So declaring supportedResolution above is not necessary.

With the nit,
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

> +       }
> +
>         StreamConfiguration &cfg = cameraConfig->at(0);
>         const StreamFormats &formats = cfg.formats();
> -       std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
> +       supportedResolutions = formats.sizes(pixelFormat);
>
>         return supportedResolutions;
>  }
> --
> 2.31.1
>