[libcamera-devel] [PATCH v4 3/3] android: camera_device: Configure one stream for identical stream requests

[Hirokazu Honda]

Hi Laurent,

On Fri, Sep 3, 2021 at 5:36 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> Thank you for the patch.
>
> On Wed, Sep 01, 2021 at 05:03:02PM +0900, Hirokazu Honda wrote:
> > An Android HAL client may request identical stream requests. It is
>
> s/identical stream requests/multiple identical streams/
>
> > redundant that a native camera device produces a separate stream for
> > each of the identical requests.
> > Configure camera one stream configuration for the identical stream
> > requests.
>
> And here I'd write
>
> Configure the camera with a single stream in that case. The other
> identical HAL streams will be produced by the YUV post-processor.
>
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 324b997f..51d5370e 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -618,12 +618,38 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >                       continue;
> >               }
> >
> > +             /*
> > +              * Always add GRALLOC_USAGE_HW_CAMERA_WRITE to the usage on
> > +              * ChromeOS because cros::CameraBufferManager imports the stream
> > +              * buffer with the usage.
>
> I'm still not sure to follow you. What happens if you don't set
> GRALLOC_USAGE_HW_CAMERA_WRITE ?

This gmb_bo_import call fails. That is, the buffer cannot be mapped
and even constructing CameraBuffer fails.
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/camera/common/camera_buffer_manager_impl.cc;l=669;drc=6cd1330d01ad2fe4122faa0b2708ecf6267eb58e

Well, GBM_BO_USE_CAMERA_WRITE may be unnecessary, but I can understand
they want to add it for safety.

-Hiro
>
> > +              */
> > +#if defined(OS_CHROMEOS)
> > +             stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
> > +#endif
> > +
> > +             /*
> > +              * 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;
> >               streamConfig.config.pixelFormat = format;
> >               streamConfigs.push_back(std::move(streamConfig));
> > -
>
> This isn't needed.
>
> >               /* This stream will be produced by hardware. */
> >               stream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;
> >       }
>
> --
> Regards,
>
> Laurent Pinchart