[libcamera-devel] [PATCH v2 05/12] android: camera_device: Generate RAW resolutions

[Jacopo Mondi]

Hi Laurent,

On Sat, Sep 05, 2020 at 09:07:49PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Sep 02, 2020 at 05:22:29PM +0200, Jacopo Mondi wrote:
> > The resolutions supported for the RAW formats cannot be tested from
> > a list of known sizes like the processed ones. This is mainly due to the
> > fact RAW streams are produced by capturing frames at the CSI-2 receiver
> > output and their size corresponds to the sensor's native sizes.
>
> This isn't always the case though. Most sensors can bin and skip, so
> half of the native size is usually a valid resolution. There's also a

It's "native sizes" not "native size" :)
I meant all the sizes produced natively by the sensor.

Then indeed we can have CSI-2 receivers capable of cropping or even
scaling the sensor produced frames, but it's up to the pipeline
handler to report this.

> dependency between raw and processed sizes. If a binned/skipped raw size
> is desired, the available processed sizes will be reduced. And the other
> way around, when capturing a lower resolution processed frame, the
> pipeline handler may decide to bin/skip on the sensor to reduce the
> bandwidth and increase the possible frame rates.
>
> This doesn't need to be taken into account in this patch, but I'd like
> to make sure you've considered the issue in the design, to avoid
> cornering ourselves in a dead end.

I don't see many ways around than reporting all the available per-stream sizes
if not considered in isolation. We cannot produce lists of all valid
stream combinations, because the Android's stream configuration map
reprots sizes per formats, not combinations of valid configuration.

In example
StreamConfiguration
android.scaler.availableStreamConfigurations = {
        { raw, 2092, 1944, 0},
        { raw, 1988, 1112, 0},
        { raw, 724, 512, 0},
        { YUV, 2056, 1920, 0},
        { YUV, 1920, 1080, 0},
        { YUV, 720, 480, 0},
};

How would you express here "if you ask for a 724x512 frame, you can't
have 2056x1920 YUV" if not by refusing that combination at stream
configuration time ? And if the pipeline handler is capable of doing
so, it could even be capable of scaling down the 2092x1944 raw frame
to 724x512 after it has been used to produce the 2056x1920 YUV frame.

I guess there's not easy way out if not failing at configuration time.

>
> > In order to obtain the RAW frame size generate a temporary
> > CameraConfiguration for the Role::StillCaptureRAW role and inspect the
> > map of StreamFormats returned by the pipeline handler.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 21 +++++++++++++++++----
> >  src/android/camera_device.h   |  2 ++
> >  2 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 58b2ad27c5e2..b110bfe43ece 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::filterYUVResolutions(CameraConfiguration *camera
> >  	return supportedResolutions;
> >  }
> >
> > +std::vector<Size> CameraDevice::filterRawResolutions(const libcamera::PixelFormat &pixelFormat)
> > +{
> > +	std::unique_ptr<CameraConfiguration> cameraConfig =
> > +		camera_->generateConfiguration({ StillCaptureRaw });
> > +	StreamConfiguration &cfg = cameraConfig->at(0);
> > +	const StreamFormats &formats = cfg.formats();
> > +	std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
> > +
> > +	return supportedResolutions;
> > +}
> > +
> >  /*
> >   * Initialize the format conversion map to translate from Android format
> >   * identifier to libcamera pixel formats and fill in the list of supported
> > @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations()
> >  				<< camera3Format.name << " to: "
> >  				<< mappedFormat.toString();
> >
> > +		std::vector<Size> resolutions;
> >  		const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);
> >  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > -			continue;
> > +			resolutions = filterRawResolutions(mappedFormat);
> > +		else
> > +			resolutions = filterYUVResolutions(cameraConfig.get(),
> > +							   mappedFormat,
> > +							   cameraResolutions);
>
> Usage of cameraConfig here isn't very nice, but that's something to be
> addressed later.
>

I don't particularly like it either. It breaks a bit ownership rules
and feels a bit like an hack. Not a big deal though, isn't it ?

> >
> > -		std::vector<Size> resolutions = filterYUVResolutions(cameraConfig.get(),
> > -								     mappedFormat,
> > -								     cameraResolutions);
> >  		for (const Size &res : resolutions) {
> >  			streamConfigurations_.push_back({ res, androidFormat });
> >
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 359a163ebab9..dc0ee664d443 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -97,6 +97,8 @@ private:
> >  	filterYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
> >  			     const libcamera::PixelFormat &pixelFormat,
> >  			     const std::vector<libcamera::Size> &resolutions);
> > +	std::vector<libcamera::Size>
> > +	filterRawResolutions(const libcamera::PixelFormat &pixelFormat);
> >
> >  	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
> >  	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
>
> --
> Regards,
>
> Laurent Pinchart