[libcamera-devel] [PATCH] android: camera_device: Add pipeline max depth static metadata

[Ren-Pei Zeng]

Thanks Jacopo for clarifying. I misthought this value will limit the
available buffers on the consumer side (chrome capture stack in this case),
but it should not.
I'm not familiar with how frame control affects this, and the value is for
this moment, so it's up to you.

Thanks!

On Wed, Aug 28, 2019 at 11:41 PM Jacopo Mondi <jacopo at jmondi.org> wrote:

> Hello Ren-Pei,
>
> On Wed, Aug 28, 2019 at 11:16:20PM +0800, Ren-Pei Zeng wrote:
> > Thanks for working on this!
> >
> > +       uint8_t maxPipelineDepth = 1;
> > Should this keep the original value, i.e. = 5?
>
> My understanding is that as long as the HAL reports to support LEGACY
> mode this should not be a big issue, as the maxDepth is relevant only
> in regards to the dynamic control android.sync.frameNumber, which we
> currently don't report and should be probably set to UNKNOWN as long
> as we work in backward compatibility mode.
>
> Is my understanding correct here in your opinion?
>
> I chose 1 because we actually don't apply any control at the moment,
> and the value there should depend on the platform specific pipeline
> handler characteristics.
>
> Why would you prefer to keep a value of 5 here ? I'm not opposed to
> it, just for my better understanding!
>
> Thanks
>    j
>
> >
> > On Wed, Aug 28, 2019 at 10:12 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> >
> > > The ANDROID_REQUEST_PIPELINE_MAX_DEPTH metadata tag was wrongly
> > > reported in the capture settings template, while it is actually a
> static
> > > camera metadata.
> > >
> > > As of Chromium R78 the absence of this specific metadata tag causes a
> > > system crash. Fix this by reporting the maximum pipeline depth in the
> > > static metadata pack and set its value to 1 as currently no control is
> > > applied to the image capture pipeline.
> > >
> > > Reported-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > Suggested-by: Ren-Pei Zeng <kamesan at google.com>
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > >
> > > ---
> > > Laurent, as you're running R78 and first reported the crash, could you
> > > please
> > > give this a spin and report if the issue is still present?
> > >
> > > ---
> > >  src/android/camera_device.cpp | 13 +++++++------
> > >  1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp
> b/src/android/camera_device.cpp
> > > index c27175ac090d..7c69d0810eee 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -281,6 +281,13 @@ camera_metadata_t
> *CameraDevice::getStaticMetadata()
> > >                         ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> > >                         &supportedHWLevel, 1);
> > >
> > > +       /* Request static metadata. */
> > > +       uint8_t maxPipelineDepth = 1;
> > > +       ret = add_camera_metadata_entry(staticMetadata_,
> > > +                       ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > > +                       &maxPipelineDepth, 1);
> > > +       METADATA_ASSERT(ret);
> > > +
> > >         return staticMetadata_;
> > >  }
> > >
> > > @@ -340,12 +347,6 @@ const camera_metadata_t
> > > *CameraDevice::constructDefaultRequestSettings(int type)
> > >                         maxOutStream, 3);
> > >         METADATA_ASSERT(ret);
> > >
> > > -       uint8_t maxPipelineDepth = 5;
> > > -       ret = add_camera_metadata_entry(requestTemplate_,
> > > -                       ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > > -                       &maxPipelineDepth, 1);
> > > -       METADATA_ASSERT(ret);
> > > -
> > >         int32_t inputStreams = 0;
> > >         ret = add_camera_metadata_entry(requestTemplate_,
> > >                         ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,
> > > --
> > > 2.23.0
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190829/3b780a74/attachment.html>