[libcamera-devel] [PATCH] android: camera_metadata: Add functions for instrumenting resizing

[Hirokazu Honda]

Hi Paul,

On Fri, May 14, 2021 at 6:32 PM Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Hi Paul,
>
> On 14/05/2021 10:27, Paul Elder wrote:
> > Add utility functions to CameraMetadata to check if it has been resized,
> > and for outputting the actual entry and data count. This is meant to be
> > used to output information on resizing, to assist developers in
> > choosing proper initial sizes to avoid resizing. Also make CameraDevice
> > use these functions for static and result metadata.
>
> Great, that's exactly what I had envisioned, and allows developers to
> initialise the static metadata reservations with exact sizes when they
> get adjusted, while still allowing the system to perform correctly if
> the initial reservations were too low.
>
> For metadata which is used more dynamically, it will be harder to get an
> exact size - but that's up to the developers to identify and size
> correctly to balance the initial reservation against expected usages.
>
> Thanks!
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp   | 16 ++++++++++++++++
> >  src/android/camera_metadata.cpp | 14 +++++++++++++-
> >  src/android/camera_metadata.h   |  4 ++++
> >  3 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/android/camera_device.cpp
> b/src/android/camera_device.cpp
> > index b32e8be5..15f81b04 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1391,6 +1391,14 @@ const camera_metadata_t
> *CameraDevice::getStaticMetadata()
> >               return nullptr;
> >       }
> >
> > +     if (staticMetadata_->resized()) {
> > +             size_t entryCount, dataCount;
> > +             std::tie(entryCount, dataCount) = staticMetadata_->usage();
>

nit: You can write these two lines as auto [entryCount, dataCount] =
staticMetadata->usage() since C++17.

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

> +             LOG(HAL, Info)
> > +                     << "Static metadata resized: " << entryCount
> > +                     << " entries and " << dataCount << " bytes used";
> > +     }
> > +
> >       return staticMetadata_->get();
> >  }
> >
> > @@ -2269,5 +2277,13 @@ CameraDevice::getResultMetadata(const
> Camera3RequestDescriptor &descriptor) cons
> >               LOG(HAL, Error) << "Failed to construct result metadata";
> >       }
> >
> > +     if (resultMetadata->resized()) {
> > +             size_t entryCount, dataCount;
> > +             std::tie(entryCount, dataCount) = resultMetadata->usage();
> > +             LOG(HAL, Info)
> > +                     << "Result metadata resized: " << entryCount
> > +                     << " entries and " << dataCount << " bytes used";
> > +     }
> > +
> >       return resultMetadata;
> >  }
> > diff --git a/src/android/camera_metadata.cpp
> b/src/android/camera_metadata.cpp
> > index bf8d2781..0588ea4e 100644
> > --- a/src/android/camera_metadata.cpp
> > +++ b/src/android/camera_metadata.cpp
> > @@ -14,17 +14,19 @@ using namespace libcamera;
> >  LOG_DEFINE_CATEGORY(CameraMetadata)
> >
> >  CameraMetadata::CameraMetadata()
> > -     : metadata_(nullptr), valid_(false)
> > +     : metadata_(nullptr), valid_(false), resized_(false)
> >  {
> >  }
> >
> >  CameraMetadata::CameraMetadata(size_t entryCapacity, size_t
> dataCapacity)
> > +     : resized_(false)
> >  {
> >       metadata_ = allocate_camera_metadata(entryCapacity, dataCapacity);
> >       valid_ = metadata_ != nullptr;
> >  }
> >
> >  CameraMetadata::CameraMetadata(const camera_metadata_t *metadata)
> > +     : resized_(false)
> >  {
> >       metadata_ = clone_camera_metadata(metadata);
> >       valid_ = metadata_ != nullptr;
> > @@ -55,6 +57,14 @@ CameraMetadata &CameraMetadata::operator=(const
> CameraMetadata &other)
> >       return *this;
> >  }
> >
> > +std::tuple<size_t, size_t> CameraMetadata::usage() const
> > +{
> > +     size_t currentEntryCount =
> get_camera_metadata_entry_count(metadata_);
> > +     size_t currentDataCount =
> get_camera_metadata_data_count(metadata_);
> > +
> > +     return { currentEntryCount, currentDataCount };
> > +}
> > +
> >  bool CameraMetadata::getEntry(uint32_t tag, camera_metadata_ro_entry_t
> *entry) const
> >  {
> >       if (find_camera_metadata_ro_entry(metadata_, tag, entry))
> > @@ -104,6 +114,8 @@ bool CameraMetadata::resize(size_t count, size_t
> size)
> >
> >               append_camera_metadata(metadata_, oldMetadata);
> >               free_camera_metadata(oldMetadata);
> > +
> > +             resized_ = true;
> >       }
> >
> >       return true;
> > diff --git a/src/android/camera_metadata.h
> b/src/android/camera_metadata.h
> > index 07afd4b2..b291fbf9 100644
> > --- a/src/android/camera_metadata.h
> > +++ b/src/android/camera_metadata.h
> > @@ -23,6 +23,9 @@ public:
> >
> >       CameraMetadata &operator=(const CameraMetadata &other);
> >
> > +     std::tuple<size_t, size_t> usage() const;
> > +     bool resized() const { return resized_; }
> > +
> >       bool isValid() const { return valid_; }
> >       bool resize(size_t count, size_t size);
> >       bool getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry)
> const;
> > @@ -81,6 +84,7 @@ public:
> >  private:
> >       camera_metadata_t *metadata_;
> >       bool valid_;
> > +     bool resized_;
> >  };
> >
> >  #endif /* __ANDROID_CAMERA_METADATA_H__ */
> >
>
> --
> Regards
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210517/6f486e10/attachment.htm>