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

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

[Niklas Haas]

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
+};
+
+/**
+ * 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] [PATCH 2/6] avfilter/vf_scale*: strip metadata on size change

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

---
 libavfilter/vf_scale.c        | 2 ++
 libavfilter/vf_scale_cuda.c   | 3 +++
 libavfilter/vf_scale_npp.c    | 3 +++
 libavfilter/vf_scale_vaapi.c  | 3 +++
 libavfilter/vf_scale_vt.c     | 3 +++
 libavfilter/vf_scale_vulkan.c | 3 +++
 6 files changed, 17 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 1c07daeddf..7b9be039ad 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -870,6 +870,8 @@ scale:
     out->height = outlink->h;
     out->color_range = outlink->color_range;
     out->colorspace = outlink->colorspace;
+    if (out->width != in->width || out->height != in->height)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_SIZE);
 
     if (scale->output_is_pal)
         avpriv_set_systematic_pal2((uint32_t*)out->data[1], outlink->format == AV_PIX_FMT_PAL8 ? AV_PIX_FMT_BGR8 : outlink->format);
diff --git a/libavfilter/vf_scale_cuda.c b/libavfilter/vf_scale_cuda.c
index 5571a52b1e..1e381d6e52 100644
--- a/libavfilter/vf_scale_cuda.c
+++ b/libavfilter/vf_scale_cuda.c
@@ -525,6 +525,9 @@ static int cudascale_scale(AVFilterContext *ctx, AVFrame *out, AVFrame *in)
     if (ret < 0)
         return ret;
 
+    if (out->width != in->width || out->height != in->height)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_SIZE);
+
     return 0;
 }
 
diff --git a/libavfilter/vf_scale_npp.c b/libavfilter/vf_scale_npp.c
index 27e5e584ae..ed09f4f301 100644
--- a/libavfilter/vf_scale_npp.c
+++ b/libavfilter/vf_scale_npp.c
@@ -892,6 +892,9 @@ scale:
     if (ret < 0)
         return ret;
 
+    if (out->width != in->width || out->height != in->height)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_SIZE);
+
     return 0;
 }
 
diff --git a/libavfilter/vf_scale_vaapi.c b/libavfilter/vf_scale_vaapi.c
index 5f20b8a43c..cf7af5c68e 100644
--- a/libavfilter/vf_scale_vaapi.c
+++ b/libavfilter/vf_scale_vaapi.c
@@ -137,6 +137,9 @@ static int scale_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
     if (err < 0)
         goto fail;
 
+    if (output_frame->width != input_frame->width || output_frame->height != input_frame->height)
+        av_frame_remove_side_data_changed(output_frame, AV_FRAME_CHANGED_SIZE);
+
     if (ctx->colour_primaries != AVCOL_PRI_UNSPECIFIED)
         output_frame->color_primaries = ctx->colour_primaries;
     if (ctx->colour_transfer != AVCOL_TRC_UNSPECIFIED)
diff --git a/libavfilter/vf_scale_vt.c b/libavfilter/vf_scale_vt.c
index af4a8b32c6..085f14d5fe 100644
--- a/libavfilter/vf_scale_vt.c
+++ b/libavfilter/vf_scale_vt.c
@@ -141,6 +141,9 @@ static int scale_vt_filter_frame(AVFilterLink *link, AVFrame *in)
     if (ret < 0)
         goto fail;
 
+    if (out->width != in->width || out->height != in->height)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_SIZE);
+
     av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den,
               (int64_t)in->sample_aspect_ratio.num * outlink->h * link->w,
               (int64_t)in->sample_aspect_ratio.den * outlink->w * link->h,
diff --git a/libavfilter/vf_scale_vulkan.c b/libavfilter/vf_scale_vulkan.c
index 7210509de3..957f99a4d5 100644
--- a/libavfilter/vf_scale_vulkan.c
+++ b/libavfilter/vf_scale_vulkan.c
@@ -288,6 +288,9 @@ static int scale_vulkan_filter_frame(AVFilterLink *link, AVFrame *in)
     if (err < 0)
         goto fail;
 
+    if (out->width != in->width || out->height != in->height)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_SIZE);
+
     if (s->out_range != AVCOL_RANGE_UNSPECIFIED)
         out->color_range = s->out_range;
     if (s->vkctx.output_format != s->vkctx.input_format)
-- 
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] [PATCH 3/6] avfilter/vf_zscale: strip metadata on change

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Required for both size chnages and color volume changes (as a result of
changing primaries/transfer).
---
 libavfilter/vf_zscale.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 45f1bd25ce..c012464615 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -783,7 +783,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
     const AVPixFmtDescriptor *odesc = av_pix_fmt_desc_get(outlink->format);
     char buf[32];
-    int ret = 0;
+    int ret = 0, changed = 0;
     AVFrame *out = NULL;
     ThreadData td;
 
@@ -880,6 +880,12 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
                   (int64_t)in->sample_aspect_ratio.den * outlink->w * link->h,
                   INT_MAX);
 
+        if (out->width != in->width || out->height != in->height)
+            changed |= AV_FRAME_CHANGED_SIZE;
+        if (out->color_trc != in->color_trc || out->color_primaries != in->color_primaries)
+            changed |= AV_FRAME_CHANGED_COLOR_VOLUME;
+        av_frame_remove_side_data_changed(out, changed);
+
         td.in = in;
         td.out = out;
         td.desc = desc;
-- 
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] [PATCH 4/6] avfilter/vf_libplacebo: update metadata stripping logic

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Switches to av_frame_remove_side_data_changed(), covering a number of
cases that we previously ignored. Additionally, stop stripping metadata
when merely changing colorspace or color range, since these do not
affect the actual color volume of the image data, only the encoding.
---
 libavfilter/vf_libplacebo.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c
index be9000aa8e..17da6fc71d 100644
--- a/libavfilter/vf_libplacebo.c
+++ b/libavfilter/vf_libplacebo.c
@@ -806,7 +806,7 @@ static void update_crops(AVFilterContext *ctx, LibplaceboInput *in,
 /* Construct and emit an output frame for a given timestamp */
 static int output_frame(AVFilterContext *ctx, int64_t pts)
 {
-    int err = 0, ok, changed_csp;
+    int err = 0, ok, changed;
     LibplaceboContext *s = ctx->priv;
     pl_options opts = s->opts;
     AVFilterLink *outlink = ctx->outputs[0];
@@ -842,6 +842,7 @@ static int output_frame(AVFilterContext *ctx, int64_t pts)
          * output colorspace defaults */
         out->color_primaries = AVCOL_PRI_BT2020;
         out->color_trc = AVCOL_TRC_SMPTE2084;
+        changed |= AV_FRAME_CHANGED_COLOR_VOLUME;
     }
 
     if (s->color_trc >= 0)
@@ -849,21 +850,13 @@ static int output_frame(AVFilterContext *ctx, int64_t pts)
     if (s->color_primaries >= 0)
         out->color_primaries = s->color_primaries;
 
-    changed_csp = ref->colorspace      != out->colorspace     ||
-                  ref->color_range     != out->color_range    ||
-                  ref->color_trc       != out->color_trc      ||
-                  ref->color_primaries != out->color_primaries;
-
     /* Strip side data if no longer relevant */
-    if (changed_csp) {
-        av_frame_remove_side_data(out, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
-        av_frame_remove_side_data(out, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
-        av_frame_remove_side_data(out, AV_FRAME_DATA_ICC_PROFILE);
-    }
-    if (s->apply_dovi || changed_csp) {
-        av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_RPU_BUFFER);
-        av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_METADATA);
-    }
+    if (out->width != ref->width || out->height != ref->height)
+        changed |= AV_FRAME_CHANGED_SIZE;
+    if (ref->color_trc != out->color_trc || ref->color_primaries != out->color_primaries)
+        changed |= AV_FRAME_CHANGED_COLOR_VOLUME;
+    av_frame_remove_side_data_changed(out, changed);
+
     if (s->apply_filmgrain)
         av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
 
-- 
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] [PATCH 5/6] avfilter/vf_colorspace: strip color volume metadata

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

---
 libavfilter/vf_colorspace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index 7bacd7892a..46c442f3fb 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -750,6 +750,10 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
     } else {
         out->color_trc   = s->user_trc;
     }
+
+    if (out->color_primaries != in->color_primaries || out->color_trc != in->color_trc)
+        av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_COLOR_VOLUME);
+
     if (rgb_sz != s->rgb_sz) {
         const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format);
         int uvw = in->width >> desc->log2_chroma_w;
-- 
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] [PATCH 6/6] avfilter/vf_lut*: strip color volume metadata

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

These filters, in general, will apply some arbitrary color volume
transformation. Strip corresponding metadata to be conservative/safe.
---
 libavfilter/vf_lut.c   | 2 ++
 libavfilter/vf_lut2.c  | 1 +
 libavfilter/vf_lut3d.c | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/libavfilter/vf_lut.c b/libavfilter/vf_lut.c
index 01df8f287d..89884dd315 100644
--- a/libavfilter/vf_lut.c
+++ b/libavfilter/vf_lut.c
@@ -537,6 +537,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
         av_frame_copy_props(out, in);
     }
 
+    av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_COLOR_VOLUME);
+
     if (s->is_rgb && s->is_16bit && !s->is_planar) {
         /* packed, 16-bit */
         PACKED_THREAD_DATA
diff --git a/libavfilter/vf_lut2.c b/libavfilter/vf_lut2.c
index 1f0661a0f5..123bfb2e17 100644
--- a/libavfilter/vf_lut2.c
+++ b/libavfilter/vf_lut2.c
@@ -610,6 +610,7 @@ static int tlut2_filter_frame(AVFilterLink *inlink, AVFrame *frame)
             }
 
             av_frame_copy_props(out, frame);
+            av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_COLOR_VOLUME);
 
             td.out  = out;
             td.srcx = frame;
diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
index b3ddd3e69f..84c1405b26 100644
--- a/libavfilter/vf_lut3d.c
+++ b/libavfilter/vf_lut3d.c
@@ -1175,6 +1175,8 @@ static AVFrame *apply_lut(AVFilterLink *inlink, AVFrame *in)
         av_frame_copy_props(out, in);
     }
 
+    av_frame_remove_side_data_changed(out, AV_FRAME_CHANGED_COLOR_VOLUME);
+
     td.in  = in;
     td.out = out;
     ff_filter_execute(ctx, lut3d->interp, &td, NULL,
-- 
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".

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

[Niklas Haas]

On Fri, 26 Apr 2024 14:27:58 +0200 Niklas Haas <ffmpeg@haasn.xyz> 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
> +};

We almost surely also want to do something similar for audio-related
metadata. I'm not too familiar with audio, does something like this seem
right?

On changing channel layout:
- strip AV_FRAME_DATA_MATRIXENCODING
- strip AV_FRAME_DATA_DOWNMIX_INFO
- strip AV_FRAME_DATA_REPLAYGAIN (maybe?)

On changing sample rate:
- strip AV_FRAME_DATA_SKIP_SAMPLES

Anything else?

What about S12M_TIMECODE / GOP_TIMECODE? Shouldn't we strip these if the
pts / timebase changes?

> +
> +/**
> + * 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".

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

[epirat07]

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".

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

[James Almer]

On 4/26/2024 9:27 AM, 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.

Maybe instead of "changed", which is a concept that doesn't belong to a 
frame but to a process, use a name that references side data that 
depends on specific properties (dimensions, color props, etc).

Also, don't make it depend on AVFrame, but AVFrameSideData instead, so 
it can be reused in other contexts (Use the av_frame_side_data_* 
namespace for it).

> ---
>   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
> +};
> +
> +/**
> + * 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, \
_______________________________________________
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".

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

[Niklas Haas]

On Fri, 26 Apr 2024 16:29:03 -0300 James Almer <jamrial@gmail.com> wrote:
> On 4/26/2024 9:27 AM, 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.
> 
> Maybe instead of "changed", which is a concept that doesn't belong to a 
> frame but to a process, use a name that references side data that 
> depends on specific properties (dimensions, color props, etc).
> 
> Also, don't make it depend on AVFrame, but AVFrameSideData instead, so 
> it can be reused in other contexts (Use the av_frame_side_data_* 
> namespace for it).

Do you have a proposed name?

I can think of:
- av_frame_side_data_remove_aspect()
- av_frame_side_data_remove_dependent()
- av_frame_side_data_remove_category()

Not sure if I like any more (which is why I went with _changed(), which
tells you exactly when this function was intended to be used).
_______________________________________________
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".