[libcamera-devel] [PATCH v3 02/11] android: camera_device: Generate JPEG sizes

[Jacopo Mondi]

Hi Niklas,

On Thu, Sep 10, 2020 at 12:33:06PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-09-08 15:41:33 +0200, Jacopo Mondi wrote:
> > When producing the list of image resolutions to claim as supported by the
> > camera HAL, the JPEG stream was assumed to be 'always valid' as, at the
> > time, there was no JPEG support in place at all.
> >
> > With the introduction of support for JPEG compression, reporting
> > non-valid sizes as supported obviously causes troubles.
> >
> > In order to avoid reporting non-supported resolutions as supported,
> > produce the list of available JPEG sizes by using the ones supported
> > by the YCbCr_420_888 format, from which the JPEG stream is encoded.
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 40 ++++++++++++++++++++++-------------
> >  1 file changed, 25 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 28fb3868c082..1b2e12d6d33c 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -363,17 +363,21 @@ int CameraDevice::initializeStreamConfigurations()
> >  		const std::vector<PixelFormat> &libcameraFormats =
> >  			camera3Format.libcameraFormats;
> >
> > +		/*
> > +		 * JPEG is always supported, either produced directly by the
> > +		 * camera, or encoded in the HAL.
> > +		 */
> > +		if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
> > +			formatsMap_[androidFormat] = formats::MJPEG;
> > +			continue;
> > +		}
> > +
> >  		/*
> >  		 * Test the libcamera formats that can produce images
> >  		 * compatible with the format defined by Android.
> >  		 */
> >  		PixelFormat mappedFormat;
> >  		for (const PixelFormat &pixelFormat : libcameraFormats) {
> > -			/* \todo Fixed mapping for JPEG. */
> > -			if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
> > -				mappedFormat = formats::MJPEG;
> > -				break;
> > -			}
> >
> >  			/*
> >  			 * The stream configuration size can be adjusted,
> > @@ -416,19 +420,25 @@ int CameraDevice::initializeStreamConfigurations()
> >  			cfg.size = res;
> >
> >  			CameraConfiguration::Status status = cameraConfig->validate();
> > -			/*
> > -			 * Unconditionally report we can produce JPEG.
> > -			 *
> > -			 * \todo The JPEG stream will be implemented as an
> > -			 * HAL-only stream, but some cameras can produce it
> > -			 * directly. As of now, claim support for JPEG without
> > -			 * inspecting where the JPEG stream is produced.
> > -			 */
> > -			if (androidFormat != HAL_PIXEL_FORMAT_BLOB &&
> > -			    status != CameraConfiguration::Valid)
> > +			if (status != CameraConfiguration::Valid)
> >  				continue;
> >
> >  			streamConfigurations_.push_back({ res, androidFormat });
> > +
> > +			/*
> > +			 * If the format is HAL_PIXEL_FORMAT_YCbCr_420_888
> > +			 * from which JPEG is produced, add an entry for
> > +			 * the JPEG stream.
> > +			 *
> > +			 * \todo Wire the JPEG encoder to query the supported
> > +			 * sizes provided a list of formats it can encode.
>
> nit: Is this needed? Are we expecting the JPEG encoder to reject some
> resolutions?

It's more about alignments and limitations in the supported
resolutions.

>
> In any case,
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > +			 *
> > +			 * \todo Support JPEG streams produced by the Camera
> > +			 * natively.
> > +			 */
> > +			if (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888)
> > +				streamConfigurations_.push_back(
> > +					{ res, HAL_PIXEL_FORMAT_BLOB });
> >  		}
> >  	}
> >
> > --
> > 2.28.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund