| Message ID | 20210403131828.1591652-1-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thanks for the patch. 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@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"; Other than that: Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > + return -EINVAL; > + } > + > /* > * Generate an empty configuration, and construct a StreamConfiguration > * for each camera3_stream to add to it.
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@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@ideasonboard.com> > Other than that: > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > > + return -EINVAL; > > + } > > + > > /* > > * Generate an empty configuration, and construct a StreamConfiguration > > * for each camera3_stream to add to it.
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"; + return -EINVAL; + } + /* * Generate an empty configuration, and construct a StreamConfiguration * for each camera3_stream to add to it.
This checks if the number of streams is zero on configuration and then returns -EINVAL. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/android/camera_device.cpp | 5 +++++ 1 file changed, 5 insertions(+)