diff mbox series

[FFmpeg-devel] avfilter/vf_colorspace: use colorspace negotiation API

Message ID 20240325175836.301657-1-nicolas.gaullier@cji.paris
State New
Headers show
Series [FFmpeg-devel] avfilter/vf_colorspace: use colorspace negotiation API | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Nicolas Gaullier March 25, 2024, 5:58 p.m. UTC 
Fixes a regression due to the fact that the colorspace filter does
not use the new API introduced by 8c7934f73ab6c568acaa.
The scale filter uses it since 45e09a30419cc2a7251e, and the setparams
filter since 3bf80df3ccd32aed23f0.

Example:
ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg:
color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
,showinfo -f null -frames 1 -

Before:
  color_range:unknown color_space:unknown color_primaries:bt709 ...
After:
  color_range:tv color_space:bt709 color_primaries:bt709 ...

There is still an issue with the color_range when it is not specified:
the documentation states the output defaults to the same value as the
input, but it does not seem possible to implement currently.

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 libavfilter/vf_colorspace.c | 62 +++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 26 deletions(-)

Comments

Nicolas Gaullier March 28, 2024, 9:50 a.m. UTC | #1 
>De : Nicolas Gaullier <nicolas.gaullier@cji.paris> 
>Envoyé : lundi 25 mars 2024 18:59
>
>Fixes a regression due to the fact that the colorspace filter does not use the new API introduced by 8c7934f73ab6c568acaa.
>The scale filter uses it since 45e09a30419cc2a7251e, and the setparams filter since 3bf80df3ccd32aed23f0.
>
>Example:
>ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg:
>color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
>,showinfo -f null -frames 1 -
>
>Before:
>  color_range:unknown color_space:unknown color_primaries:bt709 ...
>After:
>  color_range:tv color_space:bt709 color_primaries:bt709 ...
>
>There is still an issue with the color_range when it is not specified:
>the documentation states the output defaults to the same value as the input, but it does not seem possible to implement currently.

Ping ? I think it should be fixed one way or another, and backported to 7.0...

Thanks
Nicolas
Nicolas Gaullier March 28, 2024, 10:01 a.m. UTC | #2 
>>Example:
>>ffmpeg -f lavfi -i testsrc -vf setparams=color_primaries=bt470bg:
>>color_trc=smpte170m:colorspace=bt470bg,colorspace=bt709:range=tv,scale
>>,showinfo -f null -frames 1 -
>>
>>Before:
>>  color_range:unknown color_space:unknown color_primaries:bt709 ...
>>After:
>>  color_range:tv color_space:bt709 color_primaries:bt709 ...
>>
>>There is still an issue with the color_range when it is not specified:
>>the documentation states the output defaults to the same value as the input, but it does not seem possible to implement currently.
>
>Ping ? I think it should be fixed one way or another, and backported to 7.0...

Forget about it,
It has been fixed late yesterday with b89ee26539

Nicolas
diff mbox series

Patch

diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c
index f367ce17c6..b1e1b9b719 100644
--- a/libavfilter/vf_colorspace.c
+++ b/libavfilter/vf_colorspace.c
@@ -432,8 +432,7 @@  static int create_filtergraph(AVFilterContext *ctx,
     if (out->color_trc       != s->out_trc) s->out_txchr     = NULL;
     if (in->colorspace       != s->in_csp ||
         in->color_range      != s->in_rng)  s->in_lumacoef   = NULL;
-    if (out->colorspace      != s->out_csp ||
-        out->color_range     != s->out_rng) s->out_lumacoef  = NULL;
+    if (out->color_range     != s->out_rng) s->rgb2yuv       = NULL;
 
     if (!s->out_primaries || !s->in_primaries) {
         s->in_prm = in->color_primaries;
@@ -562,26 +561,8 @@  static int create_filtergraph(AVFilterContext *ctx,
         redo_yuv2rgb = 1;
     }
 
-    if (!s->out_lumacoef) {
-        s->out_csp = out->colorspace;
+    if (!s->rgb2yuv) {
         s->out_rng = out->color_range;
-        s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
-        if (!s->out_lumacoef) {
-            if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
-                if (s->user_all == CS_UNSPECIFIED) {
-                    av_log(ctx, AV_LOG_ERROR,
-                           "Please specify output colorspace\n");
-                } else {
-                    av_log(ctx, AV_LOG_ERROR,
-                           "Unsupported output color property %d\n", s->user_all);
-                }
-            } else {
-                av_log(ctx, AV_LOG_ERROR,
-                       "Unsupported output colorspace %d (%s)\n", s->out_csp,
-                       av_color_space_name(s->out_csp));
-            }
-            return AVERROR(EINVAL);
-        }
         redo_rgb2yuv = 1;
     }
 
@@ -686,6 +667,26 @@  static av_cold int init(AVFilterContext *ctx)
 {
     ColorSpaceContext *s = ctx->priv;
 
+    s->out_csp  = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
+                  default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
+    s->out_lumacoef = av_csp_luma_coeffs_from_avcsp(s->out_csp);
+    if (!s->out_lumacoef) {
+        if (s->out_csp == AVCOL_SPC_UNSPECIFIED) {
+            if (s->user_all == CS_UNSPECIFIED) {
+                av_log(ctx, AV_LOG_ERROR,
+                       "Please specify output colorspace\n");
+            } else {
+                av_log(ctx, AV_LOG_ERROR,
+                       "Unsupported output color property %d\n", s->user_all);
+            }
+        } else {
+            av_log(ctx, AV_LOG_ERROR,
+                   "Unsupported output colorspace %d (%s)\n", s->out_csp,
+                   av_color_space_name(s->out_csp));
+        }
+        return AVERROR(EINVAL);
+    }
+
     ff_colorspacedsp_init(&s->dsp);
 
     return 0;
@@ -745,10 +746,10 @@  static int filter_frame(AVFilterLink *link, AVFrame *in)
     } else {
         out->color_trc   = s->user_trc;
     }
-    out->colorspace      = s->user_csp == AVCOL_SPC_UNSPECIFIED ?
-                           default_csp[FFMIN(s->user_all, CS_NB)] : s->user_csp;
-    out->color_range     = s->user_rng == AVCOL_RANGE_UNSPECIFIED ?
-                           in->color_range : s->user_rng;
+    // FIXME color_range must be set in query_formats for negotiation.
+    // The value updated here will not be propagated further in the graph.
+    if (s->user_rng == AVCOL_RANGE_UNSPECIFIED)
+        out->color_range = outlink->color_range = in->color_range;
     if (rgb_sz != s->rgb_sz) {
         const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(out->format);
         int uvw = in->width >> desc->log2_chroma_w;
@@ -838,10 +839,19 @@  static int query_formats(AVFilterContext *ctx)
         AV_PIX_FMT_YUVJ420P,  AV_PIX_FMT_YUVJ422P,  AV_PIX_FMT_YUVJ444P,
         AV_PIX_FMT_NONE
     };
-    int res;
+    int res, ret;
     ColorSpaceContext *s = ctx->priv;
+    AVFilterLink *outlink = ctx->outputs[0];
     AVFilterFormats *formats = ff_make_format_list(pix_fmts);
 
+    if (ret = ff_formats_ref(ff_make_formats_list_singleton(s->out_csp),
+                             &outlink->incfg.color_spaces) < 0)
+        return ret;
+    if (s->user_rng != AVCOL_RANGE_UNSPECIFIED
+        && (ret = ff_formats_ref(ff_make_formats_list_singleton(s->user_rng),
+                             &outlink->incfg.color_ranges) < 0))
+        return ret;
+
     if (!formats)
         return AVERROR(ENOMEM);
     if (s->user_format == AV_PIX_FMT_NONE)