[FFmpeg-devel] [PATCH] lavfi/vf_libplacebo: fix side data stripping logic

[FFmpeg-devel] [PATCH] lavfi/vf_libplacebo: fix side data stripping logic

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This was accidentally comparing s->colorspace against out->colorspace,
which is wrong - the intent was to compare in->colorspace against
out->colorspace.

We also forgot to strip mastering metadata. Finally, the order is sort
of wrong - we should strip this side data *before* process_frames,
because otherwise it may end up being seen and used by libplacebo.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 libavfilter/vf_libplacebo.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c
index 1386aaeb3a..31ae28ac38 100644
--- a/libavfilter/vf_libplacebo.c
+++ b/libavfilter/vf_libplacebo.c
@@ -390,7 +390,7 @@ fail:
 
 static int filter_frame(AVFilterLink *link, AVFrame *in)
 {
-    int err, changed;
+    int err, changed_csp;
     AVFilterContext *ctx = link->dst;
     LibplaceboContext *s = ctx->priv;
     AVFilterLink *outlink = ctx->outputs[0];
@@ -426,22 +426,25 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
     if (s->color_primaries >= 0)
         out->color_primaries = s->color_primaries;
 
-    RET(process_frames(ctx, out, in));
-
-    int changed_csp = s->colorspace      != out->colorspace     ||
-                      s->color_range     != out->color_range    ||
-                      s->color_trc       != out->color_trc      ||
-                      s->color_primaries != out->color_primaries;
+    changed_csp = in->colorspace      != out->colorspace     ||
+                  in->color_range     != out->color_range    ||
+                  in->color_trc       != out->color_trc      ||
+                  in->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);
+    }
     if (s->apply_dovi || changed_csp) {
-        /* Strip side data if no longer relevant */
         av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_RPU_BUFFER);
         av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_METADATA);
     }
-
     if (s->apply_filmgrain)
         av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
 
+    RET(process_frames(ctx, out, in));
+
     av_frame_free(&in);
 
     return ff_filter_frame(outlink, out);
-- 
2.34.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".

Re: [FFmpeg-devel] [PATCH] lavfi/vf_libplacebo: fix side data stripping logic

[Lynne]

10 Jan 2022, 08:29 by ffmpeg@haasn.xyz:

> From: Niklas Haas <git@haasn.dev>
>
> This was accidentally comparing s->colorspace against out->colorspace,
> which is wrong - the intent was to compare in->colorspace against
> out->colorspace.
>
> We also forgot to strip mastering metadata. Finally, the order is sort
> of wrong - we should strip this side data *before* process_frames,
> because otherwise it may end up being seen and used by libplacebo.
>
> Signed-off-by: Niklas Haas <git@haasn.dev>
> ---
>  libavfilter/vf_libplacebo.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/libavfilter/vf_libplacebo.c b/libavfilter/vf_libplacebo.c
> index 1386aaeb3a..31ae28ac38 100644
> --- a/libavfilter/vf_libplacebo.c
> +++ b/libavfilter/vf_libplacebo.c
> @@ -390,7 +390,7 @@ fail:
>  
>  static int filter_frame(AVFilterLink *link, AVFrame *in)
>  {
> -    int err, changed;
> +    int err, changed_csp;
>  AVFilterContext *ctx = link->dst;
>  LibplaceboContext *s = ctx->priv;
>  AVFilterLink *outlink = ctx->outputs[0];
> @@ -426,22 +426,25 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>  if (s->color_primaries >= 0)
>  out->color_primaries = s->color_primaries;
>  
> -    RET(process_frames(ctx, out, in));
> -
> -    int changed_csp = s->colorspace      != out->colorspace     ||
> -                      s->color_range     != out->color_range    ||
> -                      s->color_trc       != out->color_trc      ||
> -                      s->color_primaries != out->color_primaries;
> +    changed_csp = in->colorspace      != out->colorspace     ||
> +                  in->color_range     != out->color_range    ||
> +                  in->color_trc       != out->color_trc      ||
> +                  in->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);
> +    }
>  if (s->apply_dovi || changed_csp) {
> -        /* Strip side data if no longer relevant */
>  av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_RPU_BUFFER);
>  av_frame_remove_side_data(out, AV_FRAME_DATA_DOVI_METADATA);
>  }
> -
>  if (s->apply_filmgrain)
>  av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
>  
> +    RET(process_frames(ctx, out, in));
> +
>  av_frame_free(&in);
>  
>  return ff_filter_frame(outlink, out);
>

Pushed to both master and release/5.0, thanks.
_______________________________________________
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".