| Message ID | 20200724142120.95538-5-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, On 24/07/2020 15:21, Jacopo Mondi wrote: > Add a method to update an existing metadata tag entry, by wrapping > the update_metadata_entry() function provided by the Android > metadata library. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_metadata.cpp | 27 +++++++++++++++++++++++++++ > src/android/camera_metadata.h | 1 + > 2 files changed, 28 insertions(+) > > diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp > index ea33e9c2de25..e48c987fb0fb 100644 > --- a/src/android/camera_metadata.cpp > +++ b/src/android/camera_metadata.cpp > @@ -46,6 +46,33 @@ bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count) > return false; > } > > +bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count) > +{ > + if (!valid_) > + return false; > + > + const char *name = get_camera_metadata_tag_name(tag); Depending upon how 'fast' this is, as it's a lookup, and only for debug, should it only happen in the debug/fail paths? Even if it's at the expense of duplicating the call in both uses below? > + > + camera_metadata_entry_t entry; > + int ret = find_camera_metadata_entry(metadata_, tag, &entry); > + if (ret) { > + LOG(CameraMetadata, Error) > + << "Failed to update tag " > + << (name ? name : "unknown") << ": not present"; > + return false; > + } > + > + ret = update_camera_metadata_entry(metadata_, entry.index, data, > + count, nullptr); > + if (ret) { > + LOG(CameraMetadata, Error) > + << "Failed to update tag " << (name ? name : "unknown"); > + return false; > + } > + > + return true; > +} > + > camera_metadata_t *CameraMetadata::get() > { > return valid_ ? metadata_ : nullptr; > diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h > index f16dd27bbf44..9d047b1bb534 100644 > --- a/src/android/camera_metadata.h > +++ b/src/android/camera_metadata.h > @@ -19,6 +19,7 @@ public: > > bool isValid() const { return valid_; } > bool addEntry(uint32_t tag, const void *data, size_t data_count); > + bool updateEntry(uint32_t tag, const void *data, size_t data_count); > > camera_metadata_t *get(); > const camera_metadata_t *get() const; >
Hi Jacopo, Thank you for the patch. On Fri, Jul 24, 2020 at 04:21:18PM +0200, Jacopo Mondi wrote: > Add a method to update an existing metadata tag entry, by wrapping > the update_metadata_entry() function provided by the Android > metadata library. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_metadata.cpp | 27 +++++++++++++++++++++++++++ > src/android/camera_metadata.h | 1 + > 2 files changed, 28 insertions(+) > > diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp > index ea33e9c2de25..e48c987fb0fb 100644 > --- a/src/android/camera_metadata.cpp > +++ b/src/android/camera_metadata.cpp > @@ -46,6 +46,33 @@ bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count) > return false; > } > > +bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count) > +{ > + if (!valid_) > + return false; > + > + const char *name = get_camera_metadata_tag_name(tag); As pointed out by Kieran, I would move this to the error cases below to avoid unnecessary lookups. > + > + camera_metadata_entry_t entry; > + int ret = find_camera_metadata_entry(metadata_, tag, &entry); > + if (ret) { > + LOG(CameraMetadata, Error) > + << "Failed to update tag " > + << (name ? name : "unknown") << ": not present"; s/"unknown"/"<unknown>"/ or something similar, otherwise the message will be Failed to update tag unknown: not present and will be confusing. > + return false; > + } > + > + ret = update_camera_metadata_entry(metadata_, entry.index, data, > + count, nullptr); > + if (ret) { > + LOG(CameraMetadata, Error) > + << "Failed to update tag " << (name ? name : "unknown"); Same here. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + return false; > + } > + > + return true; > +} > + > camera_metadata_t *CameraMetadata::get() > { > return valid_ ? metadata_ : nullptr; > diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h > index f16dd27bbf44..9d047b1bb534 100644 > --- a/src/android/camera_metadata.h > +++ b/src/android/camera_metadata.h > @@ -19,6 +19,7 @@ public: > > bool isValid() const { return valid_; } > bool addEntry(uint32_t tag, const void *data, size_t data_count); > + bool updateEntry(uint32_t tag, const void *data, size_t data_count); > > camera_metadata_t *get(); > const camera_metadata_t *get() const;
diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp index ea33e9c2de25..e48c987fb0fb 100644 --- a/src/android/camera_metadata.cpp +++ b/src/android/camera_metadata.cpp @@ -46,6 +46,33 @@ bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count) return false; } +bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count) +{ + if (!valid_) + return false; + + const char *name = get_camera_metadata_tag_name(tag); + + camera_metadata_entry_t entry; + int ret = find_camera_metadata_entry(metadata_, tag, &entry); + if (ret) { + LOG(CameraMetadata, Error) + << "Failed to update tag " + << (name ? name : "unknown") << ": not present"; + return false; + } + + ret = update_camera_metadata_entry(metadata_, entry.index, data, + count, nullptr); + if (ret) { + LOG(CameraMetadata, Error) + << "Failed to update tag " << (name ? name : "unknown"); + return false; + } + + return true; +} + camera_metadata_t *CameraMetadata::get() { return valid_ ? metadata_ : nullptr; diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h index f16dd27bbf44..9d047b1bb534 100644 --- a/src/android/camera_metadata.h +++ b/src/android/camera_metadata.h @@ -19,6 +19,7 @@ public: bool isValid() const { return valid_; } bool addEntry(uint32_t tag, const void *data, size_t data_count); + bool updateEntry(uint32_t tag, const void *data, size_t data_count); camera_metadata_t *get(); const camera_metadata_t *get() const;
Add a method to update an existing metadata tag entry, by wrapping the update_metadata_entry() function provided by the Android metadata library. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_metadata.cpp | 27 +++++++++++++++++++++++++++ src/android/camera_metadata.h | 1 + 2 files changed, 28 insertions(+)