[FFmpeg-devel] [PATCH] avfilter: properly reduce YUV colorspace format lists

[FFmpeg-devel] [PATCH] avfilter: properly reduce YUV colorspace format lists

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Doing this with REDUCE_FORMATS() instead of swap_color_*() is not only
shorter, but more importantly comes with the benefit of being done
inside a loop, allowing us to correctly propagate complex graphs
involving multiple conversion filters (e.g. -vf scale,zscale).

The latter family of swapping functions is only used to settle the
best *remaining* entry if no exact match was found, and as such was
never the correct solution to YUV colorspaces, which only care about
exact matches.
---
 libavfilter/avfiltergraph.c | 84 ++-----------------------------------
 1 file changed, 4 insertions(+), 80 deletions(-)

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index bb5399c55e8..12ff7d6ffb9 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -794,6 +794,10 @@ static int reduce_formats_on_filter(AVFilterContext *filter)
                    nb_formats, ff_add_format);
     REDUCE_FORMATS(int,      AVFilterFormats,        samplerates,     formats,
                    nb_formats, ff_add_format);
+    REDUCE_FORMATS(int,      AVFilterFormats,        color_spaces,    formats,
+                   nb_formats, ff_add_format);
+    REDUCE_FORMATS(int,      AVFilterFormats,        color_ranges,    formats,
+                   nb_formats, ff_add_format);
 
     /* reduce channel layouts */
     for (i = 0; i < filter->nb_inputs; i++) {
@@ -906,82 +910,6 @@ static void swap_samplerates(AVFilterGraph *graph)
         swap_samplerates_on_filter(graph->filters[i]);
 }
 
-static void swap_color_spaces_on_filter(AVFilterContext *filter)
-{
-    AVFilterLink *link = NULL;
-    enum AVColorSpace csp;
-    int i;
-
-    for (i = 0; i < filter->nb_inputs; i++) {
-        link = filter->inputs[i];
-        if (link->type == AVMEDIA_TYPE_VIDEO &&
-            link->outcfg.color_spaces->nb_formats == 1)
-            break;
-    }
-    if (i == filter->nb_inputs)
-        return;
-
-    csp = link->outcfg.color_spaces->formats[0];
-
-    for (i = 0; i < filter->nb_outputs; i++) {
-        AVFilterLink *outlink = filter->outputs[i];
-        if (outlink->type != AVMEDIA_TYPE_VIDEO)
-            continue;
-        /* there is no meaningful 'score' between different yuv matrices,
-         * so just prioritize an exact match if it exists */
-        for (int j = 0; j < outlink->incfg.color_spaces->nb_formats; j++) {
-            if (csp == outlink->incfg.color_spaces->formats[j]) {
-                FFSWAP(int, outlink->incfg.color_spaces->formats[0],
-                       outlink->incfg.color_spaces->formats[j]);
-                break;
-            }
-        }
-    }
-}
-
-static void swap_color_spaces(AVFilterGraph *graph)
-{
-    for (int i = 0; i < graph->nb_filters; i++)
-        swap_color_spaces_on_filter(graph->filters[i]);
-}
-
-static void swap_color_ranges_on_filter(AVFilterContext *filter)
-{
-    AVFilterLink *link = NULL;
-    enum AVColorRange range;
-    int i;
-
-    for (i = 0; i < filter->nb_inputs; i++) {
-        link = filter->inputs[i];
-        if (link->type == AVMEDIA_TYPE_VIDEO &&
-            link->outcfg.color_ranges->nb_formats == 1)
-            break;
-    }
-    if (i == filter->nb_inputs)
-        return;
-
-    range = link->outcfg.color_ranges->formats[0];
-
-    for (i = 0; i < filter->nb_outputs; i++) {
-        AVFilterLink *outlink = filter->outputs[i];
-        if (outlink->type != AVMEDIA_TYPE_VIDEO)
-            continue;
-        for (int j = 0; j < outlink->incfg.color_ranges->nb_formats; j++) {
-            if (range == outlink->incfg.color_ranges->formats[j]) {
-                FFSWAP(int, outlink->incfg.color_ranges->formats[0],
-                       outlink->incfg.color_ranges->formats[j]);
-                break;
-            }
-        }
-    }
-}
-
-static void swap_color_ranges(AVFilterGraph *graph)
-{
-    for (int i = 0; i < graph->nb_filters; i++)
-        swap_color_ranges_on_filter(graph->filters[i]);
-}
-
 #define CH_CENTER_PAIR (AV_CH_FRONT_LEFT_OF_CENTER | AV_CH_FRONT_RIGHT_OF_CENTER)
 #define CH_FRONT_PAIR  (AV_CH_FRONT_LEFT           | AV_CH_FRONT_RIGHT)
 #define CH_STEREO_PAIR (AV_CH_STEREO_LEFT          | AV_CH_STEREO_RIGHT)
@@ -1258,10 +1186,6 @@ static int graph_config_formats(AVFilterGraph *graph, void *log_ctx)
     if ((ret = reduce_formats(graph)) < 0)
         return ret;
 
-    /* for video filters, ensure that the best colorspace metadata is selected */
-    swap_color_spaces(graph);
-    swap_color_ranges(graph);
-
     /* for audio filters, ensure the best format, sample rate and channel layout
      * is selected */
     swap_sample_fmts(graph);
-- 
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] avfilter: properly reduce YUV colorspace format lists

[Damiano Galassi]

On Mon, Mar 25, 2024 at 4:10 PM Niklas Haas <ffmpeg@haasn.xyz> wrote:

> From: Niklas Haas <git@haasn.dev>
>
> Doing this with REDUCE_FORMATS() instead of swap_color_*() is not only
> shorter, but more importantly comes with the benefit of being done
> inside a loop, allowing us to correctly propagate complex graphs
> involving multiple conversion filters (e.g. -vf scale,zscale).
>
> The latter family of swapping functions is only used to settle the
> best *remaining* entry if no exact match was found, and as such was
> never the correct solution to YUV colorspaces, which only care about
> exact matches.
>

I can confirm this fixes the issue I was seeing.
_______________________________________________
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".