[FFmpeg-devel] [PATCH v2 1/3] various: change EXIF metadata into AVFrameSideData
James Almer
jamrial at gmail.com
Mon Sep 30 01:42:32 EEST 2024
On 9/16/2024 11:43 AM, Leo Izen wrote:
> This patch centralizes much of the EXIF parsing and handling code for
> libavcodec, and delegates its own AVFrameSideData type to containing
> the buffer that holds EXIF metadata. This patch also adds exposes the
> exif parsing routing so it can be called by ffprobe, and updates the
> corresponding FATE tests to read the keys from the side data instead of
> from the main frame metadata.
>
> This commit also deprecates an avpriv_ function in exif.h, exposing the
> parsing functionality as a public API in the exported libavcodec/exif.h
> header file.
>
> Signed-off-by: Leo Izen <leo.izen at gmail.com>
> ---
> fftools/ffprobe.c | 27 +-
> libavcodec/Makefile | 1 +
> libavcodec/exif.c | 394 +++++++++++++++++++++++++++--
> libavcodec/exif.h | 32 ++-
> libavcodec/exif_internal.h | 55 ++++
> libavcodec/mjpegdec.c | 91 +------
> libavcodec/mjpegdec.h | 2 +-
> libavcodec/tiff.c | 19 +-
> libavcodec/tiff.h | 1 +
> libavcodec/version.h | 2 +-
> libavcodec/webp.c | 38 +--
> libavformat/avidec.c | 4 +-
> libavutil/frame.c | 2 +
> libavutil/frame.h | 6 +
> tests/ref/fate/exif-image-embedded | 5 +-
> tests/ref/fate/exif-image-jpg | 91 +++----
> tests/ref/fate/exif-image-webp | 91 +++----
> 17 files changed, 631 insertions(+), 230 deletions(-)
> create mode 100644 libavcodec/exif_internal.h
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index bf5ebe3ce0..cdd7af6e82 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -32,6 +32,7 @@
> #include "libavformat/avformat.h"
> #include "libavformat/version.h"
> #include "libavcodec/avcodec.h"
> +#include "libavcodec/exif.h"
> #include "libavcodec/version.h"
> #include "libavutil/ambient_viewing_environment.h"
> #include "libavutil/avassert.h"
> @@ -2040,19 +2041,30 @@ static void writer_register_all(void)
> memset( (ptr) + (cur_n), 0, ((new_n) - (cur_n)) * sizeof(*(ptr)) ); \
> }
>
> -static inline int show_tags(WriterContext *w, AVDictionary *tags, int section_id)
> +static inline int show_dict(WriterContext *w, const AVDictionary *tags)
> {
> const AVDictionaryEntry *tag = NULL;
> int ret = 0;
> -
> if (!tags)
> return 0;
> - writer_print_section_header(w, NULL, section_id);
> -
> while ((tag = av_dict_iterate(tags, tag))) {
> - if ((ret = print_str_validate(tag->key, tag->value)) < 0)
> + ret = print_str_validate(tag->key, tag->value);
> + if (ret < 0)
> break;
> }
> + return ret;
> +}
> +
> +static inline int show_tags(WriterContext *w, const AVDictionary *tags, int section_id)
> +{
> + int ret;
> +
> + if (!tags)
> + return 0;
> + writer_print_section_header(w, NULL, section_id);
> +
> + ret = show_dict(w, tags);
> +
> writer_print_section_footer(w);
>
> return ret;
> @@ -2920,6 +2932,11 @@ static void print_frame_side_data(WriterContext *w,
> } else if (sd->type == AV_FRAME_DATA_FILM_GRAIN_PARAMS) {
> AVFilmGrainParams *fgp = (AVFilmGrainParams *)sd->data;
> print_film_grain_params(w, fgp);
> + } else if (sd->type == AV_FRAME_DATA_EXIF) {
> + AVDictionary *dict = NULL;
> + int ret = av_exif_parse_buffer(NULL, sd->data, sd->size, &dict, AV_EXIF_PARSE_TIFF_HEADER);
> + if (ret >= 0)
> + show_dict(w, dict);
> }
> writer_print_section_footer(w);
> }
Support in ffprobe should be in a separate patch.
> diff --git a/libavcodec/exif.c b/libavcodec/exif.c
> index 959d114d09..8442e9f174 100644
> --- a/libavcodec/exif.c
> +++ b/libavcodec/exif.c
[...]
> +attribute_deprecated
No need for this. It's not a public function.
What you should do is wrap it in a (LIBAVCODEC_VERSION_MAJOR < 62)
preprocessor check.
> +int avpriv_exif_decode_ifd(void *logctx, const uint8_t *buf, int size,
> + int le, int depth, AVDictionary **metadata)
> +{
> + return av_exif_parse_buffer(logctx, buf, size, metadata, le ? AV_EXIF_ASSUME_LE : AV_EXIF_ASSUME_BE);
> }
> diff --git a/libavcodec/exif.h b/libavcodec/exif.h
> index f70d21391a..cf54be68ba 100644
> --- a/libavcodec/exif.h
> +++ b/libavcodec/exif.h
> @@ -1,6 +1,7 @@
> /*
> * EXIF metadata parser
> * Copyright (c) 2013 Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + * Copyright (c) 2024 Leo Izen <leo.izen at gmail.com>
> *
> * This file is part of FFmpeg.
> *
> @@ -23,21 +24,40 @@
> * @file
> * EXIF metadata parser
> * @author Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + * @author Leo Izen <leo.izen at gmail.com>
> */
>
> #ifndef AVCODEC_EXIF_H
> #define AVCODEC_EXIF_H
>
> +#include <stddef.h>
> #include <stdint.h>
> +
> +#include "libavutil/attributes.h"
No need. see below.
> #include "libavutil/dict.h"
> -#include "bytestream.h"
>
> -/** Recursively decodes all IFD's and
> - * adds included TAGS into the metadata dictionary. */
> +enum AVExifParseMode {
> + AV_EXIF_PARSE_TIFF_HEADER,
> + AV_EXIF_ASSUME_LE,
> + AV_EXIF_ASSUME_BE,
> +};
> +
> +/**
> + * Parse an EXIF metadata buffer into the AVDictionary **metadata.
> + *
> + * @param logctx A log context for error logging.
> + * @param buf This buffer contains an EXIF blob.
> + * @param size The size of the buffer.
> + * @param metadata A metadata dictionary, into which the key/value tags are written.
> + * @param parse_mode An enum indicating whether to parse the TIFF header,
> + * or to assume it's LE/BE and skip it.
> + * @return negative upon failure, otherwise total bytes read from the buffer.
> + */
> +int av_exif_parse_buffer(void *logctx, const uint8_t *data, size_t size,
> + AVDictionary **metadata, enum AVExifParseMode parse_mode);
> +
> +attribute_deprecated
> int avpriv_exif_decode_ifd(void *logctx, const uint8_t *buf, int size,
> int le, int depth, AVDictionary **metadata);
This must not be in a public header. Put it in exif_internal.h, and also
wrapped in a (LIBAVCODEC_VERSION_MAJOR < 62) preprocessor check, with no
attribute_deprecated attribute.
>
> -int ff_exif_decode_ifd(void *logctx, GetByteContext *gbytes, int le,
> - int depth, AVDictionary **metadata);
> -
> #endif /* AVCODEC_EXIF_H */
> diff --git a/libavcodec/exif_internal.h b/libavcodec/exif_internal.h
> new file mode 100644
> index 0000000000..2c788856d6
> --- /dev/null
> +++ b/libavcodec/exif_internal.h
> @@ -0,0 +1,55 @@
> +/*
> + * EXIF metadata parser - internal functions
> + * Copyright (c) 2013 Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + * Copyright (c) 2024 Leo Izen <leo.izen at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * EXIF metadata parser - internal functions
> + * @author Thilo Borgmann <thilo.borgmann _at_ mail.de>
> + * @author Leo Izen <leo.izen at gmail.com>
> + */
> +
> +#ifndef AVCODEC_EXIF_INTERNAL_H
> +#define AVCODEC_EXIF_INTERNAL_H
> +
> +#include "libavutil/buffer.h"
> +#include "libavutil/frame.h"
> +#include "bytestream.h"
> +#include "exif.h"
> +
> +/**
> + * Attach an AVBufferRef containing EXIF metadata to a frame.
> + * This function allocates the necessary AVFrameSideData and attaches it,
> + * and also attaches any necessary other sidedata that can be read from the,
> + * EXIF metadata, such as a display matrix.
> + */
> +int ff_exif_attach(void *logctx, AVFrame *frame, AVBufferRef **data);
> +
> +/**
> + * Collects an IFD into a buffer. **buffer points to an AVBufferRef *, which will
> + * be allocated by this function. The caller needs to unref the buffer when it is
> + * done with it. This function also writes the EXIF/TIFF header into the buffer
> + * based on the endianness, so encoders can pass the buffer as-is. This function can be
> + * used by TIFF or other codecs that have non-zero IFD offsets on their EXIF metadata.
> + */
> +int ff_exif_collect_ifd(void *logctx, GetByteContext *gb, int le, AVBufferRef **buffer);
> +
> +#endif /* AVCODEC_EXIF_INTERNAL_H */
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index 86ec58713c..9ed71976df 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -43,6 +43,7 @@
> #include "codec_internal.h"
> #include "copy_block.h"
> #include "decode.h"
> +#include "exif_internal.h"
> #include "hwaccel_internal.h"
> #include "hwconfig.h"
> #include "idctdsp.h"
> @@ -2039,8 +2040,6 @@ static int mjpeg_decode_app(MJpegDecodeContext *s)
>
> /* EXIF metadata */
> if (s->start_code == APP1 && id == AV_RB32("Exif") && len >= 2) {
> - GetByteContext gbytes;
> - int ret, le, ifd_offset, bytes_read;
> const uint8_t *aligned;
>
> skip_bits(&s->gb, 16); // skip padding
> @@ -2048,27 +2047,12 @@ static int mjpeg_decode_app(MJpegDecodeContext *s)
>
> // init byte wise reading
> aligned = align_get_bits(&s->gb);
> - bytestream2_init(&gbytes, aligned, len);
> -
> - // read TIFF header
> - ret = ff_tdecode_header(&gbytes, &le, &ifd_offset);
> - if (ret) {
> - av_log(s->avctx, AV_LOG_ERROR, "mjpeg: invalid TIFF header in EXIF data\n");
> - } else {
> - bytestream2_seek(&gbytes, ifd_offset, SEEK_SET);
> -
> - // read 0th IFD and store the metadata
> - // (return values > 0 indicate the presence of subimage metadata)
> - ret = ff_exif_decode_ifd(s->avctx, &gbytes, le, 0, &s->exif_metadata);
> - if (ret < 0) {
> - av_log(s->avctx, AV_LOG_ERROR, "mjpeg: error decoding EXIF data\n");
> - }
> - }
> -
> - bytes_read = bytestream2_tell(&gbytes);
> - skip_bits(&s->gb, bytes_read << 3);
> - len -= bytes_read;
> -
> + s->exif_buffer = av_buffer_alloc(len);
> + if (!s->exif_buffer)
> + return AVERROR(ENOMEM);
> + memcpy(s->exif_buffer->data, aligned, len);
> + skip_bits(&s->gb, len << 3);
> + len = 0;
> goto out;
> }
>
> @@ -2380,13 +2364,12 @@ int ff_mjpeg_decode_frame_from_buf(AVCodecContext *avctx, AVFrame *frame,
> int index;
> int ret = 0;
> int is16bit;
> - AVDictionaryEntry *e = NULL;
>
> s->force_pal8 = 0;
>
> s->buf_size = buf_size;
>
> - av_dict_free(&s->exif_metadata);
> + av_buffer_unref(&s->exif_buffer);
> av_freep(&s->stereo3d);
> s->adobe_transform = -1;
>
> @@ -2853,60 +2836,12 @@ the_end:
> }
> }
>
> - if (e = av_dict_get(s->exif_metadata, "Orientation", e, AV_DICT_IGNORE_SUFFIX)) {
> - char *value = e->value + strspn(e->value, " \n\t\r"), *endptr;
> - int orientation = strtol(value, &endptr, 0);
> -
> - if (!*endptr) {
> - AVFrameSideData *sd = NULL;
> -
> - if (orientation >= 2 && orientation <= 8) {
> - int32_t *matrix;
> -
> - sd = av_frame_new_side_data(frame, AV_FRAME_DATA_DISPLAYMATRIX, sizeof(int32_t) * 9);
> - if (!sd) {
> - av_log(avctx, AV_LOG_ERROR, "Could not allocate frame side data\n");
> - return AVERROR(ENOMEM);
> - }
> -
> - matrix = (int32_t *)sd->data;
> -
> - switch (orientation) {
> - case 2:
> - av_display_rotation_set(matrix, 0.0);
> - av_display_matrix_flip(matrix, 1, 0);
> - break;
> - case 3:
> - av_display_rotation_set(matrix, 180.0);
> - break;
> - case 4:
> - av_display_rotation_set(matrix, 180.0);
> - av_display_matrix_flip(matrix, 1, 0);
> - break;
> - case 5:
> - av_display_rotation_set(matrix, 90.0);
> - av_display_matrix_flip(matrix, 1, 0);
> - break;
> - case 6:
> - av_display_rotation_set(matrix, 90.0);
> - break;
> - case 7:
> - av_display_rotation_set(matrix, -90.0);
> - av_display_matrix_flip(matrix, 1, 0);
> - break;
> - case 8:
> - av_display_rotation_set(matrix, -90.0);
> - break;
> - default:
> - av_assert0(0);
> - }
> - }
> - }
> + if (s->exif_buffer) {
> + ret = ff_exif_attach(s->avctx, frame, &s->exif_buffer);
> + if (ret < 0)
> + return ret;
> }
>
> - av_dict_copy(&frame->metadata, s->exif_metadata, 0);
> - av_dict_free(&s->exif_metadata);
> -
> if (avctx->codec_id != AV_CODEC_ID_SMVJPEG &&
> (avctx->codec_tag == MKTAG('A', 'V', 'R', 'n') ||
> avctx->codec_tag == MKTAG('A', 'V', 'D', 'J')) &&
> @@ -2961,7 +2896,7 @@ av_cold int ff_mjpeg_decode_end(AVCodecContext *avctx)
> av_freep(&s->blocks[i]);
> av_freep(&s->last_nnz[i]);
> }
> - av_dict_free(&s->exif_metadata);
> + av_buffer_unref(&s->exif_buffer);
>
> reset_icc_profile(s);
>
> diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
> index 13c524d597..780381445f 100644
> --- a/libavcodec/mjpegdec.h
> +++ b/libavcodec/mjpegdec.h
> @@ -138,7 +138,7 @@ typedef struct MJpegDecodeContext {
> unsigned int ljpeg_buffer_size;
>
> int extern_huff;
> - AVDictionary *exif_metadata;
> + AVBufferRef *exif_buffer;
>
> AVStereo3D *stereo3d; ///!< stereoscopic information (cached, since it is read before frame allocation)
>
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index 37b56e9757..45c3ae8c97 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -46,6 +46,7 @@
> #include "bytestream.h"
> #include "codec_internal.h"
> #include "decode.h"
> +#include "exif_internal.h"
> #include "faxcompr.h"
> #include "lzw.h"
> #include "tiff.h"
> @@ -1268,7 +1269,7 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
> s->last_tag = tag;
>
> off = bytestream2_tell(&s->gb);
> - if (count == 1) {
> + if (count == 1 && tag != TIFF_EXIFTAG) {
> switch (type) {
> case TIFF_BYTE:
> case TIFF_SHORT:
> @@ -1770,6 +1771,22 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
> case TIFF_SOFTWARE_NAME:
> ADD_METADATA(count, "software", NULL);
> break;
> + case TIFF_EXIFTAG: {
> + AVBufferRef *exif = NULL;
> + int next;
> + gb_temp = s->gb;
> + next = ff_exif_collect_ifd(s->avctx, &gb_temp, s->le, &exif);
> + if (next < 0)
> + av_log(s->avctx, AV_LOG_ERROR, "Error parsing TIFF exif tags: %d\n", next);
> + else if (next)
> + bytestream2_seek(&s->gb, next, SEEK_SET);
> + if (exif) {
> + ret = ff_exif_attach(s->avctx, frame, &exif);
> + if (ret < 0)
> + return ret;
> + }
> + break;
> + }
> case DNG_VERSION:
> if (count == 4) {
> unsigned int ver[4];
> diff --git a/libavcodec/tiff.h b/libavcodec/tiff.h
> index 12afcfa6e5..2628f9885f 100644
> --- a/libavcodec/tiff.h
> +++ b/libavcodec/tiff.h
> @@ -94,6 +94,7 @@ enum TiffTags {
> TIFF_GEO_KEY_DIRECTORY = 0x87AF,
> TIFF_GEO_DOUBLE_PARAMS = 0x87B0,
> TIFF_GEO_ASCII_PARAMS = 0x87B1,
> + TIFF_EXIFTAG = 0x8769,
> };
>
> /** abridged list of DNG tags */
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 7531c6c42a..9b8c267529 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>
> #include "version_major.h"
>
> -#define LIBAVCODEC_VERSION_MINOR 14
> +#define LIBAVCODEC_VERSION_MINOR 15
> #define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> index 7c2a5f0111..659f641652 100644
> --- a/libavcodec/webp.c
> +++ b/libavcodec/webp.c
> @@ -35,6 +35,9 @@
> * Exif metadata
> * ICC profile
> *
> + * @author Leo Izen <leo.izen at gmail.com>
> + * Exif metadata
> + *
> * Unimplemented:
> * - Animation
> * - XMP metadata
> @@ -48,7 +51,7 @@
> #include "bytestream.h"
> #include "codec_internal.h"
> #include "decode.h"
> -#include "exif.h"
> +#include "exif_internal.h"
> #include "get_bits.h"
> #include "thread.h"
> #include "tiff_common.h"
> @@ -1452,13 +1455,12 @@ static int webp_decode_frame(AVCodecContext *avctx, AVFrame *p,
> break;
> }
> case MKTAG('E', 'X', 'I', 'F'): {
> - int le, ifd_offset, exif_offset = bytestream2_tell(&gb);
> - AVDictionary *exif_metadata = NULL;
> - GetByteContext exif_gb;
> + AVBufferRef *buf;
>
> if (s->has_exif) {
> av_log(avctx, AV_LOG_VERBOSE, "Ignoring extra EXIF chunk\n");
> - goto exif_end;
> + bytestream2_skip(&gb, chunk_size);
> + break;
> }
> if (!(vp8x_flags & VP8X_FLAG_EXIF_METADATA))
> av_log(avctx, AV_LOG_WARNING,
> @@ -1466,25 +1468,13 @@ static int webp_decode_frame(AVCodecContext *avctx, AVFrame *p,
> "VP8X header\n");
>
> s->has_exif = 1;
> - bytestream2_init(&exif_gb, avpkt->data + exif_offset,
> - avpkt->size - exif_offset);
> - if (ff_tdecode_header(&exif_gb, &le, &ifd_offset) < 0) {
> - av_log(avctx, AV_LOG_ERROR, "invalid TIFF header "
> - "in Exif data\n");
> - goto exif_end;
> - }
> -
> - bytestream2_seek(&exif_gb, ifd_offset, SEEK_SET);
> - if (ff_exif_decode_ifd(avctx, &exif_gb, le, 0, &exif_metadata) < 0) {
> - av_log(avctx, AV_LOG_ERROR, "error decoding Exif data\n");
> - goto exif_end;
> - }
> -
> - av_dict_copy(&p->metadata, exif_metadata, 0);
> -
> -exif_end:
> - av_dict_free(&exif_metadata);
> - bytestream2_skip(&gb, chunk_size);
> + buf = av_buffer_alloc(chunk_size);
> + if (!buf)
> + return AVERROR(ENOMEM);
> + bytestream2_get_buffer(&gb, buf->data, chunk_size);
> + ret = ff_exif_attach(avctx, p, &buf);
> + if (ret < 0)
> + return ret;
> break;
> }
> case MKTAG('I', 'C', 'C', 'P'): {
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index 1ae09efc15..1cea72471f 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -433,8 +433,8 @@ static int avi_extract_stream_metadata(AVFormatContext *s, AVStream *st)
> offset = bytestream2_tell(&gb);
>
> // decode EXIF tags from IFD, AVI is always little-endian
> - return avpriv_exif_decode_ifd(s, data + offset, data_size - offset,
> - 1, 0, &st->metadata);
> + return av_exif_parse_buffer(s, data + offset, data_size - offset,
> + &st->metadata, AV_EXIF_ASSUME_LE);
> break;
> case MKTAG('C', 'A', 'S', 'I'):
> avpriv_request_sample(s, "RIFF stream data tag type CASI (%u)", tag);
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 5cbfc6a48b..f668ed87fd 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -56,6 +56,8 @@ static const AVSideDataDescriptor sd_props[] = {
> [AV_FRAME_DATA_SPHERICAL] = { "Spherical Mapping", AV_SIDE_DATA_PROP_GLOBAL },
> [AV_FRAME_DATA_ICC_PROFILE] = { "ICC profile", AV_SIDE_DATA_PROP_GLOBAL },
> [AV_FRAME_DATA_SEI_UNREGISTERED] = { "H.26[45] User Data Unregistered SEI message", AV_SIDE_DATA_PROP_MULTI },
> + [AV_FRAME_DATA_EXIF] = { "EXIF metadata",
> + AV_SIDE_DATA_PROP_GLOBAL },
nit: put it above AV_FRAME_DATA_SEI_UNREGISTERED, so all the multi prop
entries are together at the end.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240929/431a13f7/attachment.sig>
More information about the ffmpeg-devel
mailing list