[libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add function to convert PixelFormat to android format

[Hirokazu Honda]

Hi Laurent, thank you for reviewing.

On Wed, Dec 1, 2021 at 11:26 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> Thank you for the patch.
>
> On Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:
> > This adds a function to CameraCapabilities of converting PixelFormat
> > to android format.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> >  src/android/camera_capabilities.cpp | 18 ++++++++++++++++++
> >  src/android/camera_capabilities.h   |  2 ++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index f357902e..4c1dc87c 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()
> >                */
> >               if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {
> >                       formatsMap_[androidFormat] = formats::MJPEG;
> > +                     revFormatsMap_[formats::MJPEG] = androidFormat;
> >                       LOG(HAL, Debug) << "Mapped Android format "
> >                                       << camera3Format.name << " to "
> >                                       << formats::MJPEG.toString()
> > @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()
> >                * stream configurations map, by testing the image resolutions.
> >                */
> >               formatsMap_[androidFormat] = mappedFormat;
> > +             revFormatsMap_[mappedFormat] = androidFormat;
> >               LOG(HAL, Debug) << "Mapped Android format "
> >                               << camera3Format.name << " to "
> >                               << mappedFormat.toString();
> > @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const
> >       return it->second;
> >  }
> >
> > +/*
> > + * Translate libcamera pixel format to Android format code. -1 if no mapped
> > + * android format is found.
> > + */
> > +int CameraCapabilities::toAndroidFormat(PixelFormat format) const
> > +{
> > +     auto it = revFormatsMap_.find(format);
> > +     if (it == revFormatsMap_.end()) {
> > +             LOG(HAL, Error) << "Requested format " << format.toString()
> > +                             << " not supported";
> > +             return -1;
> > +     }
> > +
> > +     return it->second;
>
> I think there's a conceptual problem here. While a given Android format
> can always be translated to a PixelFormat, the mapping isn't 1:1. We
> already map different Android formats to NV21 for instance, and I expect
> this to continue as we support more formats. A reverse map is thus
> ill-defined, there's no guarantee that
>
>         toAndroidFormat(toPixelFormat(format)) == format
>
> which I think can lead to subtle bugs. Can't we instead, in the next
> patch, get the Android format from camera3Stream_->format ?
>

camera3Stream_->format is what Android HAL client requests.
>From camera3Stream_->format, we decide to request configuration to camera.
Since the created buffer is the input to camera, its format must be
configuration().pixelFormat.
Both PlatformFrameBufferAllocator (gralloc and
cros::CameraBufferManager) demands android format.
Further, pixelFormat resolved from android format is dependent on the
backend implementation.
Therefore it is probably correct to lookup a proper android format
from pixelFormat.
But there is no such API in gralloc API and cros::CameraBufferManager.

-Hiro
> > +}
> > +
> >  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() const
> >  {
> >       if (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {
> > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h
> > index 2cf97ae8..df6361f1 100644
> > --- a/src/android/camera_capabilities.h
> > +++ b/src/android/camera_capabilities.h
> > @@ -30,6 +30,7 @@ public:
> >
> >       CameraMetadata *staticMetadata() const { return staticMetadata_.get(); }
> >       libcamera::PixelFormat toPixelFormat(int format) const;
> > +     int toAndroidFormat(libcamera::PixelFormat format) const;
> >       unsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }
> >
> >       std::unique_ptr<CameraMetadata> requestTemplateManual() const;
> > @@ -77,6 +78,7 @@ private:
> >
> >       std::vector<Camera3StreamConfiguration> streamConfigurations_;
> >       std::map<int, libcamera::PixelFormat> formatsMap_;
> > +     std::map<libcamera::PixelFormat, int> revFormatsMap_;
> >       std::unique_ptr<CameraMetadata> staticMetadata_;
> >       unsigned int maxJpegBufferSize_;
> >
>
> --
> Regards,
>
> Laurent Pinchart