On 3/25/26 13:45, Michael Niedermayer via ffmpeg-devel wrote:
Hi all
I think the EXIF code would beneift from a review
I would do this on https://code.ffmpeg.org/ but
https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20041
does not show the actual diff of the patches so i cannot
do it that way at least
This is review of patch #2 of 20 from 20041
ill probably simply reply to cvslog for the others
but this was still from the old log script which threw everything
in one mail making taht a pain too
index fa2a2c2a13b..8df117478ae 100644
--- a/libavutil/side_data.c
+++ b/libavutil/side_data.c
@@ -53,6 +53,7 @@ static const AVSideDataDescriptor sd_props[] = {
[AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT] = { "Ambient viewing
environment", AV_SIDE_DATA_PROP_GLOBAL },
[AV_FRAME_DATA_SPHERICAL] = { "Spherical Mapping",
AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_SIZE_DEPENDENT },
[AV_FRAME_DATA_ICC_PROFILE] = { "ICC profile",
AV_SIDE_DATA_PROP_GLOBAL | AV_SIDE_DATA_PROP_COLOR_DEPENDENT },
+ [AV_FRAME_DATA_EXIF] = { "EXIF metadata",
AV_SIDE_DATA_PROP_GLOBAL },
IIUC EXIF can contain orientation, dimensions and colorspace-related fields, so
this single monolithic struct with just AV_SIDE_DATA_PROP_GLOBAL may be bad
If this is your criticism of the patch then you haven't really looked at
any of the functionality in any of the code. There's already existing
tools in FFmpeg for orientation (notably AV_SIDE_DATA_DISPLAYMATRIX)
which we pop off of the EXIF metadata struct and attach as its own
separate side data on read, and which we read and re-attach on write.
Dimensions are also sanitized to match the dimensions of the AVFrame
it's attached to. There's quite a bit of code to reconcile these sorts
of issues.
- Leo Izen
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
