| Message ID | 20210514092745.814353-1-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> > Signed-off-by: Paul Elder <paul.elder@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(); > + 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__ */ >
Hi Paul, On Fri, May 14, 2021 at 6:32 PM Kieran Bingham < kieran.bingham@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@ideasonboard.com> > > > > Signed-off-by: Paul Elder <paul.elder@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@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
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(); + 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__ */
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. Signed-off-by: Paul Elder <paul.elder@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(-)