[libcamera-devel] [PATCH] android: camera_device: Report supported JPEG sizes
Tomasz Figa
tfiga at google.com
Thu Aug 6 17:26:34 CEST 2020
On Thu, Aug 6, 2020 at 3:19 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Tomasz,
>
> On Thu, Aug 06, 2020 at 01:04:47PM +0200, Tomasz Figa wrote:
> > Hi Jacopo,
> >
> > On Wed, Aug 5, 2020 at 11:30 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> > >
> > > Hi Laurent,
> > > +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)
> > > +Kieran for the Compressor interface part
> > >
> > > On Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Wed, Aug 05, 2020 at 05:37:45PM +0200, Jacopo Mondi wrote:
> > > > > When producing the list of image resolution 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.
> > > > >
> > > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > > ---
> > > > >
> > > > > Patch to be applied on top of Kieran's JPEG work.
> > > > > ---
> > > > > src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------
> > > > > 1 file changed, 27 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > > > index ec8ca934842a..6a9a038a2b53 100644
> > > > > --- a/src/android/camera_device.cpp
> > > > > +++ b/src/android/camera_device.cpp
> > > > > @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()
> > > > > */
> > > > > formatsMap_[androidFormat] = mappedFormat;
> > > > >
> > > > > + /*
> > > > > + * Stop here for JPEG streams: the JPEG supported sizes will
> > > > > + * be tested later using the here recorded non-blob stream sizes.
> > > > > + */
> > > > > + if (androidFormat == HAL_PIXEL_FORMAT_BLOB)
> > > > > + continue;
> > > > > +
> > > > > for (const Size &res : cameraResolutions) {
> > > > > cfg.pixelFormat = mappedFormat;
> > > > > 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 (cameraConfig->validate() != CameraConfiguration::Valid)
> > > > > continue;
> > > > >
> > > > > streamConfigurations_.push_back({ res, androidFormat });
> > > > > }
> > > > > }
> > > > >
> > > > > + /*
> > > > > + * Insert the JPEG sizes by using the ones recorded for YUV streams
> > > > > + * from which JPEG is produced.
> > > > > + */
> > > > > + std::vector<Camera3StreamConfiguration> jpegConfigurations;
> > > > > + jpegConfigurations.reserve(cameraResolutions.size());
> > > > > +
> > > > > + for (const auto &config : streamConfigurations_) {
> > > > > + /* \todo JPEG can be produced from other formats too! */
> > > >
> > > > Another todo item, the android.scaler.availableStreamConfigurations
> > > > documentation lists required resolutions (see
> > > > https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html
> > > > which very annoyingly googlesource.com can't display as html in a web browser).
> > > >
> > > > JPEG android.sensor.info.activeArraySize Any
> > > > JPEG 1920x1080 (1080p) Any if 1080p <= activeArraySize
> > > > JPEG 1280x720 (720) Any if 720p <= activeArraySize
> > > > JPEG 640x480 (480p) Any if 480p <= activeArraySize
> > > > JPEG 320x240 (240p) Any if 240p <= activeArraySize
> > > >
> > >
> > > Those are already mandatory to be supported if I'm not mistaken.
> > >
> > > we feed cameraResolutions for all the non-jpeg formats here:
> > >
> > > for (const Size &res : cameraResolutions) {
> > > cfg.pixelFormat = mappedFormat;
> > > cfg.size = res;
> > >
> > > if (cameraConfig->validate() != CameraConfiguration::Valid)
> > > continue;
> > >
> > > streamConfigurations_.push_back({ res, androidFormat });
> > > }
> > >
> > > Then we use the sizes associated with the mandatory
> > > HAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed
> > > something on this part ?
> > >
> > > Looking at the code now, we could create an initJpeg() functions, as
> > > all the jpeg streams are basically just ignored now and
> > > initialized at the end of the function.
> > >
> > > Maybe it's really time to break this file apart, it's already quite
> > > big and terse. What if we delegate the jpeg initalization part to the
> > > Compressor interface (looking at it now, we could have gone for
> > > Encoder as a name maybe).
> > >
> > > As of now it would basically require two
> > > more methods: a way to get the a metadata pack with the values of
> > > ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE
> > > initalized, and a vector of supported sizes given the vector of the
> > > ones so far collected.
> > >
> > > Then we'll probably need a factory to create the right Compressor, but
> > > that's for later and we should pobably first discuss how to decide
> > > "which" encoder to initialize before getting to the "how".
> > >
> > > In example, how does it work today in CrOS and what are the
> > > requirements going forward ?
> >
> > If you mean JPEG resolution advertisement, I think it depends on the
> > camera hardware.
> >
> > If it's a UVC camera, it often has higher resolution capture
> > capabilities with MJPEG rather than YUV. At the same time, Android
> > requires YUV capture at "RECORD" resolutions, which are typically
> > higher (e.g. 1920x1080) than what a UVC camera can output at
> > reasonable framerate without resorting to MJPEG, so we have to
> > advertise any resolution supported by MJPEG as supported for YUV
> > streams and decode the MJPEG frames in the HAL to provide YUV frames
> > to the user.
> >
> > For RAW cameras, the JPEG streams are usually provided from YUV
> > streams anyway, so I believe reporting all the resolutions supported
> > by YUV as supported by JPEG should be correct.
>
> Thanks, that's usefull to know.
Also note that for MJPEG output from UVC cameras, it would be a waste
to re-encode the YUV back into JPEG, so we instead just perform a
lightweight conversion of the MJPEG frame into a proper JPEG (AFAIR
just some headers need to be added).
>
> >
> > If you mean JPEG encoder selection, we have the encoding abstracted
> > using a generic userspace interface exposed from Chrome (called JEA -
> > JpegEncodeAccelerator), which automatically uses whatever available in
> > the system. Currently it provides 3 implementations - V4L2, VAAPI and
> > software.
>
> Yep, I meant this part. My first thought was a factory in our Camera
> HAL, but the criteria to decide which encode to instantiate should be
> defined first. Otherwise the alternative is for downstream to maintain
> a version "which does the right thing" and I think we want to avoid
> it.
In Chrome OS, the camera service may not have access to the JPEG
encoder hardware (which could be exposed via VAAPI), so we would
always want to delegate the encoding to JEA.
Best regards,
Tomasz
More information about the libcamera-devel
mailing list