diff mbox series

[FFmpeg-devel,v3,6/8] avfilter/vf_scale: simplify color matrix parsing logic

Message ID 20231031145450.94975-6-ffmpeg@haasn.xyz
State Accepted
Commit 5d5bb77af11eb6e9bea8b5eb5f1ecba10c72dcfd
Headers show
Series [FFmpeg-devel,v3,1/8] swscale: fix sws_setColorspaceDetails after sws_init_context | 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

Niklas Haas Oct. 31, 2023, 2:54 p.m. UTC 
From: Niklas Haas <git@haasn.dev>

No need to write a custom string parser when we can just use an integer
option with preset values. The various bits of fallback logic are wholly
redundant with equivalent logic already inside sws_getCoefficients.

Note: I disallowed setting 'out_color_matrix=auto', because this does
not do anything meaningful in the current code (just hard-codes
AVCOL_SPC_BT470BG fallback).
---
 libavfilter/vf_scale.c | 66 ++++++++++++++----------------------------
 1 file changed, 22 insertions(+), 44 deletions(-)

Comments

Michael Niedermayer Nov. 2, 2023, 10:46 p.m. UTC | #1 
On Tue, Oct 31, 2023 at 03:54:48PM +0100, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> No need to write a custom string parser when we can just use an integer
> option with preset values. The various bits of fallback logic are wholly
> redundant with equivalent logic already inside sws_getCoefficients.
> 
> Note: I disallowed setting 'out_color_matrix=auto', because this does
> not do anything meaningful in the current code (just hard-codes
> AVCOL_SPC_BT470BG fallback).
> ---
>  libavfilter/vf_scale.c | 66 ++++++++++++++----------------------------
>  1 file changed, 22 insertions(+), 44 deletions(-)

probably ok

thx

[...]
diff mbox series

Patch

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 23335cef4b..fb8e6a2b76 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -139,8 +139,8 @@  typedef struct ScaleContext {
 
     char *flags_str;
 
-    char *in_color_matrix;
-    char *out_color_matrix;
+    int in_color_matrix;
+    int out_color_matrix;
 
     int in_range;
     int in_frame_range;
@@ -410,30 +410,6 @@  static int query_formats(AVFilterContext *ctx)
     return 0;
 }
 
-static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
-{
-    if (!s)
-        s = "bt601";
-
-    if (s && strstr(s, "bt709")) {
-        colorspace = AVCOL_SPC_BT709;
-    } else if (s && strstr(s, "fcc")) {
-        colorspace = AVCOL_SPC_FCC;
-    } else if (s && strstr(s, "smpte240m")) {
-        colorspace = AVCOL_SPC_SMPTE240M;
-    } else if (s && (strstr(s, "bt601") || strstr(s, "bt470") || strstr(s, "smpte170m"))) {
-        colorspace = AVCOL_SPC_BT470BG;
-    } else if (s && strstr(s, "bt2020")) {
-        colorspace = AVCOL_SPC_BT2020_NCL;
-    }
-
-    if (colorspace < 1 || colorspace > 10 || colorspace == 8) {
-        colorspace = AVCOL_SPC_BT470BG;
-    }
-
-    return sws_getCoefficients(colorspace);
-}
-
 static int scale_eval_dimensions(AVFilterContext *ctx)
 {
     ScaleContext *scale = ctx->priv;
@@ -554,7 +530,7 @@  static int config_props(AVFilterLink *outlink)
     scale->isws[0] = scale->isws[1] = scale->sws = NULL;
     if (inlink0->w == outlink->w &&
         inlink0->h == outlink->h &&
-        !scale->out_color_matrix &&
+        scale->out_color_matrix == AVCOL_SPC_UNSPECIFIED &&
         scale->in_range == scale->out_range &&
         inlink0->format == outlink->format)
         ;
@@ -840,8 +816,8 @@  scale:
 
     in_range = in->color_range;
 
-    if (   scale->in_color_matrix
-        || scale->out_color_matrix
+    if (   scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED
+        || scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED
         || scale-> in_range != AVCOL_RANGE_UNSPECIFIED
         || in_range != AVCOL_RANGE_UNSPECIFIED
         || scale->out_range != AVCOL_RANGE_UNSPECIFIED) {
@@ -852,11 +828,13 @@  scale:
                                  (int **)&table, &out_full,
                                  &brightness, &contrast, &saturation);
 
-        if (scale->in_color_matrix)
-            inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace);
-        if (scale->out_color_matrix)
-            table     = parse_yuv_type(scale->out_color_matrix, AVCOL_SPC_UNSPECIFIED);
-        else if (scale->in_color_matrix)
+        if (scale->in_color_matrix == -1 /* auto */)
+            inv_table = sws_getCoefficients(in->colorspace);
+        else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
+            inv_table = sws_getCoefficients(scale->in_color_matrix);
+        if (scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
+            table     = sws_getCoefficients(scale->out_color_matrix);
+        else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
             table = inv_table;
 
         if (scale-> in_range != AVCOL_RANGE_UNSPECIFIED)
@@ -1003,16 +981,16 @@  static const AVOption scale_options[] = {
     { "interl", "set interlacing", OFFSET(interlaced), AV_OPT_TYPE_BOOL, {.i64 = 0 }, -1, 1, FLAGS },
     { "size",   "set video size",          OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
     { "s",      "set video size",          OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
-    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_STRING, { .str = "auto" }, .flags = FLAGS, "color" },
-    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS,  "color"},
-        { "auto",        NULL, 0, AV_OPT_TYPE_CONST, { .str = "auto" },      0, 0, FLAGS, "color" },
-        { "bt601",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt601" },     0, 0, FLAGS, "color" },
-        { "bt470",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt470" },     0, 0, FLAGS, "color" },
-        { "smpte170m",   NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte170m" }, 0, 0, FLAGS, "color" },
-        { "bt709",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt709" },     0, 0, FLAGS, "color" },
-        { "fcc",         NULL, 0, AV_OPT_TYPE_CONST, { .str = "fcc" },       0, 0, FLAGS, "color" },
-        { "smpte240m",   NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte240m" }, 0, 0, FLAGS, "color" },
-        { "bt2020",      NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt2020" },    0, 0, FLAGS, "color" },
+    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_INT, { .i64 = -1 }, -1, AVCOL_SPC_NB-1, .flags = FLAGS, "color" },
+    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color"},
+        { "auto",        NULL, 0, AV_OPT_TYPE_CONST, { .i64 = -1 },                     0, 0, FLAGS, "color" },
+        { "bt601",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "bt470",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "smpte170m",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "bt709",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT709 },        0, 0, FLAGS, "color" },
+        { "fcc",         NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_FCC },          0, 0, FLAGS, "color" },
+        { "smpte240m",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_SMPTE240M },    0, 0, FLAGS, "color" },
+        { "bt2020",      NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT2020_NCL },   0, 0, FLAGS, "color" },
     {  "in_range", "set input color range",  OFFSET( in_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "out_range", "set output color range", OFFSET(out_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "auto",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 0, FLAGS, "range" },