From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id CBE7B4AEF7 for ; Thu, 21 Aug 2025 02:09:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 26BFF68C5D6; Thu, 21 Aug 2025 05:09:55 +0300 (EEST) Received: from c1ad6a1ecdc3 (code.ffmpeg.org [188.245.149.3]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id C234C68D620 for ; Thu, 21 Aug 2025 05:09:53 +0300 (EEST) MIME-Version: 1.0 To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] =?utf-8?q?=5BPATCH=5D_Assorted_Exif_changes_=28PR?= =?utf-8?q?_=2320297=29?= X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: James Almer via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: James Almer Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Message-Id: <20250821020955.26BFF68C5D6@ffbox0-bg.ffmpeg.org> Date: Thu, 21 Aug 2025 05:09:55 +0300 (EEST) Archived-At: List-Archive: List-Post: PR #20297 opened by James Almer (jamrial) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20297 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20297.patch >From 7fe00d8941072a62da0f21440bed8eda288d4db3 Mon Sep 17 00:00:00 2001 From: James Almer Date: Tue, 19 Aug 2025 13:18:18 -0300 Subject: [PATCH 1/2] avcodec/exif: use ff_frame_new_side_data() to export display matrix Otherwise, the user requested priority of packet side data will be ignored. For this, move the relevant functions to decode.c, as they need access to an AVCodecContext. Signed-off-by: James Almer --- libavcodec/decode.c | 105 ++++++++++++++++++++++++++++++++++++ libavcodec/decode.h | 29 ++++++++++ libavcodec/exif.c | 107 ------------------------------------- libavcodec/exif_internal.h | 30 +++-------- 4 files changed, 141 insertions(+), 130 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 2319e76e4b..f10427495e 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -47,6 +47,7 @@ #include "codec_desc.h" #include "codec_internal.h" #include "decode.h" +#include "exif_internal.h" #include "hwaccel_internal.h" #include "hwconfig.h" #include "internal.h" @@ -2245,3 +2246,107 @@ void ff_decode_internal_uninit(AVCodecContext *avctx) av_refstruct_unref(&dc->lcevc); } + +static int attach_displaymatrix(AVCodecContext *avctx, AVFrame *frame, int orientation) +{ + AVFrameSideData *sd = NULL; + int32_t *matrix; + int ret; + /* invalid orientation */ + if (orientation < 2 || orientation > 8) + return AVERROR_INVALIDDATA; + ret = ff_frame_new_side_data(avctx, frame, AV_FRAME_DATA_DISPLAYMATRIX, sizeof(int32_t) * 9, &sd); + if (ret < 0) { + av_log(avctx, AV_LOG_ERROR, "Could not allocate frame side data : %s\n", av_err2str(ret)); + return ret; + } + if (sd) { + matrix = (int32_t *) sd->data; + ret = av_exif_orientation_to_matrix(matrix, orientation); + } + + return ret; +} + +static int exif_attach_ifd(AVCodecContext *avctx, AVFrame *frame, const AVExifMetadata *ifd, AVBufferRef *og) +{ + const AVExifEntry *orient = NULL; + AVFrameSideData *sd; + AVExifMetadata *cloned = NULL; + AVBufferRef *written = NULL; + int ret; + + for (size_t i = 0; i < ifd->count; i++) { + const AVExifEntry *entry = &ifd->entries[i]; + if (entry->id == ORIENTATION_TAG && entry->count > 0 && entry->type == AV_TIFF_SHORT) { + orient = entry; + break; + } + } + + if (orient && orient->value.uint[0] > 1) { + av_log(avctx, AV_LOG_DEBUG, "found nontrivial EXIF orientation: %" PRIu64 "\n", orient->value.uint[0]); + ret = attach_displaymatrix(avctx, frame, orient->value.uint[0]); + if (ret < 0) { + av_log(avctx, AV_LOG_WARNING, "unable to attach displaymatrix from EXIF\n"); + } else { + const AVExifEntry *cloned_orient; + cloned = av_exif_clone_ifd(ifd); + if (!cloned) { + ret = AVERROR(ENOMEM); + goto end; + } + // will have the same offset in the clone as in the original + cloned_orient = &cloned->entries[orient - ifd->entries]; + cloned_orient->value.uint[0] = 1; + } + } + + ret = av_exif_ifd_to_dict(avctx, cloned ? cloned : ifd, &frame->metadata); + if (ret < 0) + return ret; + + if (cloned || !og) { + ret = av_exif_write(avctx, cloned ? cloned : ifd, &written, AV_EXIF_TIFF_HEADER); + if (ret < 0) + goto end; + } + + sd = av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_EXIF, written ? written : og); + if (!sd) { + if (written) + av_buffer_unref(&written); + ret = AVERROR(ENOMEM); + goto end; + } + + ret = 0; + +end: + if (og && written && ret >= 0) + av_buffer_unref(&og); // as though we called new_side_data on og; + av_exif_free(cloned); + av_free(cloned); + return ret; +} + +int ff_exif_attach_ifd(AVCodecContext *avctx, AVFrame *frame, const AVExifMetadata *ifd) +{ + return exif_attach_ifd(avctx, frame, ifd, NULL); +} + +int ff_exif_attach_buffer(AVCodecContext *avctx, AVFrame *frame, AVBufferRef *data, enum AVExifHeaderMode header_mode) +{ + int ret; + AVExifMetadata ifd = { 0 }; + + ret = av_exif_parse_buffer(avctx, data->data, data->size, &ifd, header_mode); + if (ret < 0) + goto end; + + ret = exif_attach_ifd(avctx, frame, &ifd, data); + +end: + av_exif_free(&ifd); + return ret; +} diff --git a/libavcodec/decode.h b/libavcodec/decode.h index 2c3719a8d0..1433731775 100644 --- a/libavcodec/decode.h +++ b/libavcodec/decode.h @@ -220,4 +220,33 @@ int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrame *frame, int ff_decode_content_light_new_ext(const AVCodecContext *avctx, AVFrameSideData ***sd, int *nb_sd, struct AVContentLightMetadata **clm); + +enum AVExifHeaderMode; + +/** + * Attach the data buffer to the frame. This is mostly a wrapper for + * av_side_data_new_from_buffer, but it checks if the orientation tag is + * present in the provided EXIF buffer. If it is, it zeroes it out and + * attaches that information as an AV_FRAME_DATA_DISPLAYMATRIX instead + * of including it in the AV_FRAME_DATA_EXIF side data buffer. + * + * On a success, the caller loses ownership of the data buffer. Either it is + * unrefed, or its ownership is transferred to the frame directly. On failure, + * the data buffer is left owned by the caller. + */ +int ff_exif_attach_buffer(AVCodecContext *avctx, AVFrame *frame, AVBufferRef *data, enum AVExifHeaderMode header_mode); + +struct AVExifMetadata; + +/** + * Attach an already-parsed EXIF metadata struct to the frame as a side data + * buffer. It writes the EXIF IFD into the buffer and attaches the buffer to + * the frame. + * + * If the metadata struct contains an orientation tag, it will be zeroed before + * writing, and instead, an AV_FRAME_DATA_DISPLAYMATRIX will be attached in + * addition to the AV_FRAME_DATA_EXIF side data. + */ +int ff_exif_attach_ifd(AVCodecContext *avctx, AVFrame *frame, const struct AVExifMetadata *ifd); + #endif /* AVCODEC_DECODE_H */ diff --git a/libavcodec/exif.c b/libavcodec/exif.c index 48959eb9b3..f767de6f34 100644 --- a/libavcodec/exif.c +++ b/libavcodec/exif.c @@ -46,13 +46,6 @@ #define IFD_EXTRA_SIZE 6 #define EXIF_TAG_NAME_LENGTH 32 -#define MAKERNOTE_TAG 0x927c -#define ORIENTATION_TAG 0x112 -#define EXIFIFD_TAG 0x8769 -#define IMAGE_WIDTH_TAG 0x100 -#define IMAGE_LENGTH_TAG 0x101 -#define PIXEL_X_TAG 0xa002 -#define PIXEL_Y_TAG 0xa003 struct exif_tag { const char name[EXIF_TAG_NAME_LENGTH]; @@ -805,23 +798,6 @@ int av_exif_parse_buffer(void *logctx, const uint8_t *buf, size_t size, return bytestream2_tell(&gbytes); } -static int attach_displaymatrix(void *logctx, AVFrame *frame, int orientation) -{ - AVFrameSideData *sd; - int32_t *matrix; - /* invalid orientation */ - if (orientation < 2 || orientation > 8) - return AVERROR_INVALIDDATA; - sd = av_frame_new_side_data(frame, AV_FRAME_DATA_DISPLAYMATRIX, sizeof(int32_t) * 9); - if (!sd) { - av_log(logctx, AV_LOG_ERROR, "Could not allocate frame side data\n"); - return AVERROR(ENOMEM); - } - matrix = (int32_t *) sd->data; - - return av_exif_orientation_to_matrix(matrix, orientation); -} - #define COLUMN_SEP(i, c) ((i) ? ((i) % (c) ? ", " : "\n") : "") static int exif_ifd_to_dict(void *logctx, const char *prefix, const AVExifMetadata *ifd, AVDictionary **metadata) @@ -934,68 +910,6 @@ int avpriv_exif_decode_ifd(void *logctx, const uint8_t *buf, int size, } #endif /* FF_API_OLD_EXIF */ -static int exif_attach_ifd(void *logctx, AVFrame *frame, const AVExifMetadata *ifd, AVBufferRef *og) -{ - const AVExifEntry *orient = NULL; - AVFrameSideData *sd; - AVExifMetadata *cloned = NULL; - AVBufferRef *written = NULL; - int ret; - - for (size_t i = 0; i < ifd->count; i++) { - const AVExifEntry *entry = &ifd->entries[i]; - if (entry->id == ORIENTATION_TAG && entry->count > 0 && entry->type == AV_TIFF_SHORT) { - orient = entry; - break; - } - } - - if (orient && orient->value.uint[0] > 1) { - av_log(logctx, AV_LOG_DEBUG, "found nontrivial EXIF orientation: %" PRIu64 "\n", orient->value.uint[0]); - ret = attach_displaymatrix(logctx, frame, orient->value.uint[0]); - if (ret < 0) { - av_log(logctx, AV_LOG_WARNING, "unable to attach displaymatrix from EXIF\n"); - } else { - const AVExifEntry *cloned_orient; - cloned = av_exif_clone_ifd(ifd); - if (!cloned) { - ret = AVERROR(ENOMEM); - goto end; - } - // will have the same offset in the clone as in the original - cloned_orient = &cloned->entries[orient - ifd->entries]; - cloned_orient->value.uint[0] = 1; - } - } - - ret = av_exif_ifd_to_dict(logctx, cloned ? cloned : ifd, &frame->metadata); - if (ret < 0) - return ret; - - if (cloned || !og) { - ret = av_exif_write(logctx, cloned ? cloned : ifd, &written, AV_EXIF_TIFF_HEADER); - if (ret < 0) - goto end; - } - - sd = av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_EXIF, written ? written : og); - if (!sd) { - if (written) - av_buffer_unref(&written); - ret = AVERROR(ENOMEM); - goto end; - } - - ret = 0; - -end: - if (og && written && ret >= 0) - av_buffer_unref(&og); // as though we called new_side_data on og; - av_exif_free(cloned); - av_free(cloned); - return ret; -} - #define EXIF_COPY(fname, srcname) do { \ size_t sz; \ if (av_size_mult(src->count, sizeof(*(fname)), &sz) < 0) { \ @@ -1231,27 +1145,6 @@ fail: return NULL; } -int ff_exif_attach_ifd(void *logctx, AVFrame *frame, const AVExifMetadata *ifd) -{ - return exif_attach_ifd(logctx, frame, ifd, NULL); -} - -int ff_exif_attach_buffer(void *logctx, AVFrame *frame, AVBufferRef *data, enum AVExifHeaderMode header_mode) -{ - int ret; - AVExifMetadata ifd = { 0 }; - - ret = av_exif_parse_buffer(logctx, data->data, data->size, &ifd, header_mode); - if (ret < 0) - goto end; - - ret = exif_attach_ifd(logctx, frame, &ifd, data); - -end: - av_exif_free(&ifd); - return ret; -} - static const int rotation_lut[2][4] = { {1, 8, 3, 6}, {4, 7, 2, 5}, }; diff --git a/libavcodec/exif_internal.h b/libavcodec/exif_internal.h index de1a26e3f9..565e747353 100644 --- a/libavcodec/exif_internal.h +++ b/libavcodec/exif_internal.h @@ -42,29 +42,13 @@ int avpriv_exif_decode_ifd(void *logctx, const uint8_t *buf, int size, int le, int depth, AVDictionary **metadata); #endif /* FF_API_OLD_EXIF */ -/** - * Attach the data buffer to the frame. This is mostly a wrapper for - * av_side_data_new_from_buffer, but it checks if the orientation tag is - * present in the provided EXIF buffer. If it is, it zeroes it out and - * attaches that information as an AV_FRAME_DATA_DISPLAYMATRIX instead - * of including it in the AV_FRAME_DATA_EXIF side data buffer. - * - * On a success, the caller loses ownership of the data buffer. Either it is - * unrefed, or its ownership is transferred to the frame directly. On failure, - * the data buffer is left owned by the caller. - */ -int ff_exif_attach_buffer(void *logctx, AVFrame *frame, AVBufferRef *data, enum AVExifHeaderMode header_mode); - -/** - * Attach an already-parsed EXIF metadata struct to the frame as a side data - * buffer. It writes the EXIF IFD into the buffer and attaches the buffer to - * the frame. - * - * If the metadata struct contains an orientation tag, it will be zeroed before - * writing, and instead, an AV_FRAME_DATA_DISPLAYMATRIX will be attached in - * addition to the AV_FRAME_DATA_EXIF side data. - */ -int ff_exif_attach_ifd(void *logctx, AVFrame *frame, const AVExifMetadata *ifd); +#define MAKERNOTE_TAG 0x927c +#define ORIENTATION_TAG 0x112 +#define EXIFIFD_TAG 0x8769 +#define IMAGE_WIDTH_TAG 0x100 +#define IMAGE_LENGTH_TAG 0x101 +#define PIXEL_X_TAG 0xa002 +#define PIXEL_Y_TAG 0xa003 /** * Compares values in the IFD with data in the provided AVFrame and sets the values -- 2.49.1 >From db06993b06785ea6937d89db24caacca43735e49 Mon Sep 17 00:00:00 2001 From: James Almer Date: Wed, 20 Aug 2025 13:03:21 -0300 Subject: [PATCH 2/2] avcodec/decode: remove Exif Orientation tag after adding it as display matrix Don't replace it with a conflicting value. Signed-off-by: James Almer --- libavcodec/decode.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libavcodec/decode.c b/libavcodec/decode.c index f10427495e..84cb507abc 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -2290,24 +2290,22 @@ static int exif_attach_ifd(AVCodecContext *avctx, AVFrame *frame, const AVExifMe if (ret < 0) { av_log(avctx, AV_LOG_WARNING, "unable to attach displaymatrix from EXIF\n"); } else { - const AVExifEntry *cloned_orient; cloned = av_exif_clone_ifd(ifd); if (!cloned) { ret = AVERROR(ENOMEM); goto end; } - // will have the same offset in the clone as in the original - cloned_orient = &cloned->entries[orient - ifd->entries]; - cloned_orient->value.uint[0] = 1; + av_exif_remove_entry(avctx, cloned, orient->id, 0); + ifd = cloned; } } - ret = av_exif_ifd_to_dict(avctx, cloned ? cloned : ifd, &frame->metadata); + ret = av_exif_ifd_to_dict(avctx, ifd, &frame->metadata); if (ret < 0) return ret; if (cloned || !og) { - ret = av_exif_write(avctx, cloned ? cloned : ifd, &written, AV_EXIF_TIFF_HEADER); + ret = av_exif_write(avctx, ifd, &written, AV_EXIF_TIFF_HEADER); if (ret < 0) goto end; } -- 2.49.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".