[libcamera-devel] [PATCH] Revert "android: camera_device: Configure one stream for identical stream requests"

[Hirokazu Honda]

Hi Jacopo, thank you for the patch.

On Tue, Oct 19, 2021 at 3:10 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, Oct 18, 2021 at 05:25:24PM +0200, Jacopo Mondi wrote:
> > Commit d165f7da34b8 ("android: camera_device: Configure one stream for
> > identical stream requests") introduced the ability to generate through
> > post-processing YUV streams of identical size and format.
> >
> > Howver the change didn't fully take into account the situation

s/Howver/However/

> > where only mapped streams are contained in the request submitted by
> > the camera service to the HAL. In this case the Request will be queued
> > with no buffers and refused by the Camera.
> >
> > Even if this seems a corner case it causes a few CTS to fail, and more
> > problematically it triggers out-of-order completion of requests, causing
> > the camera service to abort.
> >
> > ERROR Camera camera.cpp:1031 Request contains no buffers
> > ERROR HAL camera_device.cpp:1109 '\_SB_.PCI0.I2C2.CAM0': Out-of-order completion for request 0x00007a1f1800ccd0
> > ERROR cros_camera_service[15706:15711]: [camera_device_adapter.cc(744)] (15711) Notify(): Fatal device error; aborting the camera service
> >
> > Revert the commit until a proper solution is implemented.
>
> Sounds fine with me, as there's no urgency to support this use case.
>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

> > ---
> >  src/android/camera_device.cpp | 29 -----------------------------
> >  1 file changed, 29 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 901867105085..bd34188809da 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -619,35 +619,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >                       continue;
> >               }
> >
> > -             /*
> > -              * While gralloc usage flags are supposed to report usage
> > -              * patterns to select a suitable buffer allocation strategy, in
> > -              * practice they're also used to make other decisions, such as
> > -              * selecting the actual format for the IMPLEMENTATION_DEFINED
> > -              * HAL pixel format. To avoid issues, we thus have to set the
> > -              * GRALLOC_USAGE_HW_CAMERA_WRITE flag unconditionally, even for
> > -              * streams that will be produced in software.
> > -              */
> > -             stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
> > -
> > -             /*
> > -              * If a CameraStream with the same size and format of the
> > -              * current stream has already been requested, associate the two.
> > -              */
> > -             auto iter = std::find_if(
> > -                     streamConfigs.begin(), streamConfigs.end(),
> > -                     [size, format](const Camera3StreamConfig &streamConfig) {
> > -                             return streamConfig.config.size == size &&
> > -                                    streamConfig.config.pixelFormat == format;
> > -                     });
> > -             if (iter != streamConfigs.end()) {
> > -                     /* Add usage to copy the buffer in streams[0] to stream. */
> > -                     iter->streams[0].stream->usage |= GRALLOC_USAGE_SW_READ_OFTEN;
> > -                     stream->usage |= GRALLOC_USAGE_SW_WRITE_OFTEN;
> > -                     iter->streams.push_back({ stream, CameraStream::Type::Mapped });
> > -                     continue;
> > -             }
> > -
> >               Camera3StreamConfig streamConfig;
> >               streamConfig.streams = { { stream, CameraStream::Type::Direct } };
> >               streamConfig.config.size = size;
>
> --
> Regards,
>
> Laurent Pinchart