Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] Assorted Exif changes (PR #20297)
@ 2025-08-21  2:09 James Almer via ffmpeg-devel
  0 siblings, 0 replies; only message in thread
From: James Almer via ffmpeg-devel @ 2025-08-21  2:09 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: James Almer

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 <jamrial@gmail.com>
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 <jamrial@gmail.com>
---
 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 <jamrial@gmail.com>
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 <jamrial@gmail.com>
---
 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".

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-08-21  2:09 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-21  2:09 [FFmpeg-devel] [PATCH] Assorted Exif changes (PR #20297) James Almer via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git