Re: [FFmpeg-devel] [PATCH 1/6] avutil/frame: add av_frame_remove_side_data_changed

[epirat07@gmail.com]

On 26 Apr 2024, at 14:27, Niklas Haas wrote:

> From: Niklas Haas <git@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-request@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-request@ffmpeg.org with subject "unsubscribe".