On 26 Apr 2024, at 14:27, Niklas Haas wrote:
> From: Niklas Haas <g...@haasn.dev>
>
> Many filters modify certain aspects of frame data, e.g. through resizing
> (vf_*scale* family), color volume mapping (vf_lut*, vf_tonemap*), or
> possibly others.
>
> When this happens, we should strip all frame side data that will no
> longer be correct/relevant after the operation. For example, changing
> the image size should invalidate AV_FRAME_DATA_PANSCAN because the crop
> window (given in pixels) no longer corresponds to the actual image size.
> For another example, tone-mapping filters (e.g. from HDR to SDR) should
> strip all of the dynamic HDR related metadata.
>
> Since there are a lot of similar with basically similar operations, it
> make sense to consolidate this stripping logic into a common helper
> function. I decided to put it into libavutil as it may be useful for API
> users as well, who often have their own internal processing and
> filtering.
> ---
> doc/APIchanges | 3 +++
> libavutil/frame.c | 31 +++++++++++++++++++++++++++++++
> libavutil/frame.h | 14 ++++++++++++++
> libavutil/version.h | 2 +-
> 4 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 0566fcdcc5..26af725528 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>
> API changes, most recent first:
>
> +2024-04-xx - xxxxxxxxxx - lavu 59.17.100 - frame.h
> + Add av_frame_remove_side_data_changed().
> +
> 2024-04-24 - 8616cfe0890 - lavu 59.16.100 - opt.h
> Add AV_OPT_SERIALIZE_SEARCH_CHILDREN.
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 0775e2abd9..d5482c258e 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -1015,6 +1015,37 @@ void av_frame_remove_side_data(AVFrame *frame, enum
> AVFrameSideDataType type)
> remove_side_data(&frame->side_data, &frame->nb_side_data, type);
> }
>
> +static const struct {
> + enum AVFrameSideDataType type;
> + int aspect;
> +} side_data_aspects[] = {
> + { AV_FRAME_DATA_PANSCAN, AV_FRAME_CHANGED_SIZE },
> + { AV_FRAME_DATA_MOTION_VECTORS, AV_FRAME_CHANGED_SIZE },
> + { AV_FRAME_DATA_MASTERING_DISPLAY_METADATA,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> +
> + { AV_FRAME_DATA_SPHERICAL, AV_FRAME_CHANGED_SIZE },
> + { AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_ICC_PROFILE,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_REGIONS_OF_INTEREST, AV_FRAME_CHANGED_SIZE },
> + { AV_FRAME_DATA_DETECTION_BBOXES, AV_FRAME_CHANGED_SIZE },
> + { AV_FRAME_DATA_DOVI_RPU_BUFFER,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_DOVI_METADATA,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_DYNAMIC_HDR_VIVID,
> AV_FRAME_CHANGED_COLOR_VOLUME },
> + { AV_FRAME_DATA_VIDEO_HINT, AV_FRAME_CHANGED_SIZE },
> +};
> +
> +void av_frame_remove_side_data_changed(AVFrame *frame, int changed_aspects)
> +{
> + if (!changed_aspects)
> + return;
> +
> + for (int i = 0; i < FF_ARRAY_ELEMS(side_data_aspects); i++) {
> + if (changed_aspects & side_data_aspects[i].aspect)
> + av_frame_remove_side_data(frame, side_data_aspects[i].type);
> + }
> +}
> +
> const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType
> type)
> {
> unsigned t = type;
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 60bb966f8b..7e07ecf91f 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -983,6 +983,20 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame
> *frame,
> */
> void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType
> type);
>
> +/**
> + * Flags for stripping side data based on changed aspects.
> + */
> +enum {
> + /* Video only */
> + AV_FRAME_CHANGED_SIZE = 1 << 0, ///< changed dimensions / crop
> + AV_FRAME_CHANGED_COLOR_VOLUME = 1 << 1, ///< changed color volume
> +};
Maybe give the enum a name so you can properly link to it in the docs?
I guess using the enum type instead of int in the fuction parameter
is not really allowed because it can be other values than the enum
has, by combining flags, which I guess is not allowed by the C standard?
> +
> +/**
> + * Remove all relevant side data after a corresponding change to the frame
> + * data, based on the given bitset of frame aspects.
> + */
> +void av_frame_remove_side_data_changed(AVFrame *frame, int changed_aspects);
>
> /**
> * Flags for frame cropping.
> diff --git a/libavutil/version.h b/libavutil/version.h
> index ea289c406f..3b5a2e7aaa 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 59
> -#define LIBAVUTIL_VERSION_MINOR 16
> +#define LIBAVUTIL_VERSION_MINOR 17
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> --
> 2.44.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
