On Tue, Aug 19, 2025 at 06:33:03PM +0300, [email protected] wrote: > The branch, master has been updated > via 4c3f94f2651ce4d6834fbef10e5c287f25720ac9 (commit) > via fe496b0308f1911138698b92bbafac582fa455d7 (commit) > via 535a07d14e16eb2930d2b1e16605eafa95959139 (commit) > via 8132ee046d1a03c5913759c50fce7beda8d3d627 (commit) > via 5d4f873ff31f86f9ae1927f37ca0fcb78da70512 (commit) > via 1e816ebefee0ef479fa76bd2a1fb9ec5d5e807df (commit) > via 93a8091015978c44462626409af71789080bbef4 (commit) <------- we are > here > via 303f60684f25ce63951480ef0f759acf41aac938 (commit) > via 5caaadee7954f2014d9e0a17d1edfb961893bcf1 (commit) > via d3190a64c366a79091fe47fddf93c09a7d988802 (commit) > via 44af3829796427b89c4b76b56652cc5932ada616 (commit) > via f5ad1c910c168d05cb01315773ab0bd094c9372f (commit) > via e3aa1154aab2656c91ce61915f79516d9b563b61 (commit) > via c6cc2115f45ac26ae42442f8796996eb410f4028 (commit) > via 52dba25661305e3c4a6209d46aea43cd327c960e (commit) > via bfb17d26306592c85cf0c4e909099c621177b062 (commit) > via ba2ea285e0f270a0a885b414cafaace6a89b9a91 (commit) > via ad77345a5d14862f4701e5ad422b03b14934a5b9 (commit) > via bb90b262d6d23f1bca3587a48abc15b951cbbf05 (commit) > via a99fff4e2d4058c57599ba0b968862af82bdad5b (commit) > from a6b5a382dd7ecdd27c5d0ebba688e1db409d18fd (commit)
> commit 93a8091015978c44462626409af71789080bbef4 > Author: Leo Izen <[email protected]> > AuthorDate: Wed Apr 16 10:33:04 2025 -0400 > Commit: Leo Izen <[email protected]> > CommitDate: Tue Aug 19 11:26:48 2025 -0400 > > avcodec/exif: add av_exif_get_entry > > Add an API function to retrieve am AVExifEntry struct with a given TIFF > tag ID from the AVExifMetadata ifd struct. > > Signed-off-by: Leo Izen <[email protected]> > > diff --git a/libavcodec/exif.c b/libavcodec/exif.c > index 993a931d42..15bbe7749b 100644 > --- a/libavcodec/exif.c > +++ b/libavcodec/exif.c > @@ -1080,6 +1080,36 @@ end: > return ret; > } > > +static int exif_get_entry(void *logctx, AVExifMetadata *ifd, uint16_t id, > int depth, AVExifEntry **value) > +{ > + int offset = 1; > + > + if (!ifd || ifd->entries && !ifd->count || ifd->count && !ifd->entries > || !value) > + return AVERROR(EINVAL); > + > + for (size_t i = 0; i < ifd->count; i++) { > + if (ifd->entries[i].id == id) { > + *value = &ifd->entries[i]; > + return i + offset; > + } > + if (ifd->entries[i].type == AV_TIFF_IFD) { > + if (depth < 3) { > + int ret = exif_get_entry(logctx, &ifd->entries[i].value.ifd, > id, depth + 1, value); > + if (ret) > + return ret < 0 ? ret : ret + offset; > + } > + offset += ifd->entries[i].value.ifd.count; > + } > + } > + > + return 0; > +} > + > +int av_exif_get_entry(void *logctx, AVExifMetadata *ifd, uint16_t id, int > recursive, AVExifEntry **value) > +{ > + return exif_get_entry(logctx, ifd, id, recursive ? 0 : INT_MAX, value); > +} The docuemntation only promises "positive number" yet this is not just 1, so it risks that the user becomes dependant on undocuemneted behavior > + > int av_exif_set_entry(void *logctx, AVExifMetadata *ifd, uint16_t id, enum > AVTiffDataType type, > uint32_t count, const uint8_t *ifd_lead, uint32_t ifd_offset, const void > *value) > { > @@ -1093,12 +1123,9 @@ int av_exif_set_entry(void *logctx, AVExifMetadata > *ifd, uint16_t id, enum AVTif > || !value || ifd->count == 0xFFFFu) > return AVERROR(EINVAL); > > - for (size_t i = 0; i < ifd->count; i++) { > - if (ifd->entries[i].id == id) { > - entry = &ifd->entries[i]; > - break; > - } > - } > + ret = av_exif_get_entry(logctx, ifd, id, 0, &entry); > + if (ret < 0) > + return ret; > > if (entry) { > exif_free_entry(entry); > @@ -1292,7 +1319,6 @@ int ff_exif_get_buffer(void *logctx, const AVFrame > *frame, AVBufferRef **buffer_ > AVExifEntry *ih = NULL; > AVExifEntry *pw = NULL; > AVExifEntry *ph = NULL; > - AVExifMetadata *exif = NULL; > uint64_t orientation = 1; > uint64_t w = frame->width; > uint64_t h = frame->height; > @@ -1333,7 +1359,7 @@ int ff_exif_get_buffer(void *logctx, const AVFrame > *frame, AVBufferRef **buffer_ > continue; > } > if (entry->id == EXIFIFD_TAG && entry->type == AV_TIFF_IFD) { > - exif = &entry->value.ifd; > + AVExifMetadata *exif = &entry->value.ifd; > for (size_t j = 0; j < exif->count; j++) { > AVExifEntry *exifentry = &exif->entries[j]; > if (exifentry->id == PIXEL_X_TAG && exifentry->count > 0 && > exifentry->type == AV_TIFF_SHORT) { > @@ -1387,15 +1413,15 @@ int ff_exif_get_buffer(void *logctx, const AVFrame > *frame, AVBufferRef **buffer_ > goto end; > } > if (!pw && w && w < 0xFFFFu || !ph && h && h < 0xFFFFu) { > + AVExifMetadata *exif; > + AVExifEntry *exif_entry; > + int exif_found = av_exif_get_entry(logctx, &ifd, EXIFIFD_TAG, 0, > &exif_entry); > rewrite = 1; > - exif = NULL; > - for (size_t i = 0; i < ifd.count; i++) { > - if (ifd.entries[i].id == EXIFIFD_TAG && ifd.entries[i].type == > AV_TIFF_IFD) { > - exif = &ifd.entries[i].value.ifd; > - break; > - } > - } > - if (!exif) { > + if (exif_found < 0) > + goto end; > + if (exif_found > 0) { > + exif = &exif_entry->value.ifd; > + } else { previously this checked the type, now it doesnt, it seems like this can lead to accessing entries as the wrong type and thus causing memory corruption I think this code is just worse than before > AVExifMetadata exif_new = { 0 }; > ret = av_exif_set_entry(logctx, &ifd, EXIFIFD_TAG, AV_TIFF_IFD, > 1, NULL, 0, &exif_new); > if (ret < 0) { > diff --git a/libavcodec/exif.h b/libavcodec/exif.h > index 9f9f707495..b6d1ffd91d 100644 > --- a/libavcodec/exif.h > +++ b/libavcodec/exif.h > @@ -144,6 +144,17 @@ int32_t av_exif_get_tag_id(const char *name); > int av_exif_set_entry(void *logctx, AVExifMetadata *ifd, uint16_t id, enum > AVTiffDataType type, > uint32_t count, const uint8_t *ifd_lead, uint32_t > ifd_offset, const void *value); > > +/** > + * Get an entry with the tagged ID from the EXIF metadata struct. A pointer > to the entry > + * will be written into *value. If the recursive flag is set to true, this > function will check > + * subdirectories as well. > + * > + * If the entry was present and returned successfully, a positive number is > returned. > + * If the entry was not found, *value is left untouched and zero is returned. > + * If an error occurred, a negative number is returned. > + */ > +int av_exif_get_entry(void *logctx, AVExifMetadata *ifd, uint16_t id, int > recursive, AVExifEntry **value); The new public API is also under-specified. AVExifEntry can itself contain nested AVExifMetadata via the value.ifd union member, and the new function supports recursive descent into subdirectories, but lookup is keyed only by a raw numeric tag ID and returns the first depth-first hit. The header comment does not define what happens when multiple recursive matches exist, nor which one is preferred. For hierarchical EXIF data, "get entry by ID" is not a very strong identifier unless the directory/path semantics are also part of the contract. "*value is left untouched" is also akward design and easy to result in bugs. when nothing is found its safer to set it to NULL > + > /** > * Remove an entry from the provided EXIF metadata struct. If the recursive > flag is set > * to true, then this function will check subdirectories as well. > -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
