[libcamera-devel] [PATCH] android: CameraDevice: Deny if the streams is empty

[Laurent Pinchart]

Hi Hiro,

Thanks for the patch.

On Sat, Apr 03, 2021 at 11:01:42PM +0530, Umang Jain wrote:
> On 4/3/21 6:48 PM, Hirokazu Honda wrote:
> > This checks if the number of streams is zero on configuration
> > and then returns -EINVAL.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> >   src/android/camera_device.cpp | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index eb327978..f175ce53 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1554,6 +1554,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >   		running_ = false;
> >   	}
> >   
> > +	if (stream_list->num_streams == 0) {
> > +		LOG(HAL, Error) << "Empty stream configuration";
>
> The error message can be better, something like:
> 
> +		LOG(HAL, Error) << "Failed to find any stream for configuration";

The original message is indeed slightly confusing as it means the
configuration of one stream is empty. "Empty streams configuration"
would address that. It really doesn't matter much, as this error should
really never happen (it would signal a major bug in the camera service,
I sometimes wonder if we're not too defensive against incorrect
parameters), but nonetheless if we have an error message let's make it
as clear as possible. I'll go for "No streams in configuration" and
apply.

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

> Other than that:
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> > +		return -EINVAL;
> > +	}
> > +
> >   	/*
> >   	 * Generate an empty configuration, and construct a StreamConfiguration
> >   	 * for each camera3_stream to add to it.

-- 
Regards,

Laurent Pinchart