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

[Michael Niedermayer]

On Tue, Oct 31, 2023 at 03:54:48PM +0100, Niklas Haas wrote:
> From: Niklas Haas 
> 
> 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

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

[Niklas Haas]

From: Niklas Haas 

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(-)

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 **), _full,
  , , );
 
-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 =