[libcamera-devel] [PATCH v6 2/7] android: post_processor_jpeg: Replace encoder_ nullptr check

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Oct 25 06:58:03 CEST 2021


Hi Umang,

Thank you for the patch.

On Sat, Oct 23, 2021 at 04:02:57PM +0530, Umang Jain wrote:
> Instead of simply returning if encoder_ is nullptr, fail hard
> via an assertion. It is quite unlikely that encoder_ will be
> nullptr in PostProcessorJpeg::process() so, be loud about
> the failure.

If it was only quite unlikely an assertion wouldn't be acceptable. It
has to be impossible. That's supposed to be the case, so the change is
fine, but I'd write the commit message as "encoder_ could only be null
as a result of a fatal bug in the code, so ...".

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/jpeg/post_processor_jpeg.cpp | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 699576ef..49483836 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -102,9 +102,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  			       CameraBuffer *destination,
>  			       Camera3RequestDescriptor *request)
>  {
> -	if (!encoder_)
> -		return 0;
> -
> +	ASSERT(encoder_);
>  	ASSERT(destination->numPlanes() == 1);
>  
>  	const CameraMetadata &requestMetadata = request->settings_;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list