[libcamera-devel] [PATCH v2 10/13] android: camera_device: Report PIPELINE_MAX_DEPTH

[Jacopo Mondi]

Hi Kieran

On Wed, Oct 21, 2020 at 02:10:19PM +0100, Kieran Bingham wrote:
> On 20/10/2020 19:05, Jacopo Mondi wrote:
> > Register the ANDROID_REQUEST_PIPELINE_MAX_DEPTH static property
> > inspecting the value reported by the pipeline handler.
> >
> > If the Camera does not report any suitable value, default the
> > static property to 2.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index e9404c2f3004..8166b09bb69a 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -13,6 +13,7 @@
> >  #include <tuple>
> >  #include <vector>
> >
> > +#include <libcamera/control_ids.h>
> >  #include <libcamera/controls.h>
> >  #include <libcamera/formats.h>
> >  #include <libcamera/property_ids.h>
> > @@ -591,6 +592,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		return nullptr;
> >  	}
> >
> > +	const ControlInfoMap &controlsInfo = camera_->controls();
> > +
> >  	/* Color correction static metadata. */
> >  	std::vector<uint8_t> aberrationModes = {
> >  		ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF,
> > @@ -882,9 +885,15 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
> >  				  &partialResultCount, 1);
> >
> > -	uint8_t maxPipelineDepth = 2;
> > -	staticMetadata_->addEntry(ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > -				  &maxPipelineDepth, 1);
> > +	{
> > +		/* Default the value to 2 if not reported by the camera. */
> > +		uint8_t maxPipelineDepth = 2;
> > +		const auto &infoMap = controlsInfo.find(&controls::draft::PipelineDepth);
> > +		if (infoMap != controlsInfo.end())
> > +			maxPipelineDepth = infoMap->second.max().get<int32_t>();
>
> I would certainly put a new line here, and possibly/probably before the
> if statement too. Otherwise it's disguising the scope of the condition IMO.
>
>
> Should we print any kind of a warning if the pipeline handler has not
> reported this control?

I don't think so. According to the discussion we had on the previous
series pipeline handler should not be concerned with android
specificities.

>
> I could foresee this being a repeated pattern. Not sure how to best
> handle that yet though.
>
> But for now,
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > +		staticMetadata_->addEntry(ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
> > +					  &maxPipelineDepth, 1);
> > +	}
> >
> >  	/* LIMITED does not support reprocessing. */
> >  	uint32_t maxNumInputStreams = 0;
> >
>
> --
> Regards
> --
> Kieran