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

[Jacopo Mondi]

Hi Ren-Pei

On Thu, Aug 29, 2019 at 12:23:57AM +0800, Ren-Pei Zeng wrote:
> 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.

I'm not actually sure either here :)
My understanding comes from reading the android.sync.frameNumber
documentation
https://jmondi.org/android_metadata_tags/docs.html#dynamic_android.sync.frameNumber
but you have more experience than me in android/cros related things,
so please have a look there too :)

Thanks
  j

>
> 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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190828/9e985519/attachment.sig>