Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
@ 2025-02-17 12:17 Niklas Haas
  2025-02-17 12:42 ` Nicolas George
  0 siblings, 1 reply; 6+ messages in thread
From: Niklas Haas @ 2025-02-17 12:17 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Niklas Haas

From: Niklas Haas <git@haasn.dev>

This developer tool is especially handy when debugging filter graph
auto-negotiation, although it can be useful in whatever scenario to
get a canonical dump of the fully settled filter graph.

To make the result slightly more useful, we omit buffersrc/buffersink
filters and instead print the corresponding input/output name. Sadly, this
is lossy w.r.t. the link names used in the original filter graph, although
the result has the advantage of being in a normalized format.

As an example, the following filter graph (taken from FATE):

  sws_flags=+accurate_rnd+bitexact;
  split [main][over];
  [over] scale=88:72, pad=96:80:4:4 [overf];
  [main][overf] overlay=240:16:format=yuv422

Results in this output:

Filter graph:
  [0:0] split=thread_type=0x00000000 [L0] [L1];
  [L1] scale=w=88:width=88:h=72:height=72:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L2];
  [L2] pad=width=96:w=96:height=80:h=80:x=4:y=4:thread_type=0x00000000 [L3];
  [L4] [L3] overlay=x=240:y=16:format=2 [#0:0];
  [L0] scale=w=iw:width=iw:h=ih:height=ih:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L4];
Filter links:
  L0: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
  L1: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
  L2: yuva422p 88x72 [SAR 0:1] csp:unknown range:tv
  L3: yuva422p 96x80 [SAR 0:1] csp:unknown range:tv
  L4: yuva422p 352x288 [SAR 0:1] csp:unknown range:tv

I do acknowledge the overlap between this and avfilter/graphdump.c, but there
are a couple of important deviations:

1. graphdump.c prints a "pretty printed" ASCII art graph for human consumption,
   but the goal here is to print it in a format that can be passed back to
   -filter_complex.

2. graphdump.c does not know anything about buffersrc/buffersink filters or
   input/output names, which is why this implementation has to live inside
   ffmpeg_filter.c.

It's possible that we could instead move this implementation to graphdump.c as
well, though that would require some sort of explicit callback to get the names
for buffer sources / sinks, to avoid breaking the first design goal.
---
 doc/ffmpeg.texi         |   6 ++
 fftools/ffmpeg.h        |   1 +
 fftools/ffmpeg_filter.c | 132 ++++++++++++++++++++++++++++++++++++++++
 fftools/ffmpeg_opt.c    |   4 ++
 4 files changed, 143 insertions(+)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index da6549f043..4c5e9dc146 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -2768,6 +2768,12 @@ filter (scale, aresample) in the graph.
 On by default, to explicitly disable it you need to specify
 @code{-noauto_conversion_filters}.
 
+@item -print_filter_graph
+Print out the fully settled filter graph, after all automatic conversion
+and format restriction filters have been inserted, in a format suitable for
+passing back into @code{-filter_complex}. The exact formatting is subject to
+change.
+
 @item -bits_per_raw_sample[:@var{stream_specifier}] @var{value} (@emph{output,per-stream})
 Declare the number of bits per raw sample in the given output stream to be
 @var{value}. Note that this option sets the information provided to the
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 9439be0f41..83cedfd1d9 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -715,6 +715,7 @@ extern char *filter_nbthreads;
 extern int filter_complex_nbthreads;
 extern int vstats_version;
 extern int auto_conversion_filters;
+extern int print_filter_graph;
 
 extern const AVIOInterruptCB int_cb;
 
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index a0d04fd76f..63efdb5d30 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1896,6 +1896,13 @@ static int filter_is_buffersrc(const AVFilterContext *f)
             !strcmp(f->filter->name, "abuffer"));
 }
 
+static int filter_is_buffersink(const AVFilterContext *f)
+{
+    return f->nb_outputs == 0 &&
+           (!strcmp(f->filter->name, "buffersink") ||
+            !strcmp(f->filter->name, "abuffersink"));
+}
+
 static int graph_is_meta(AVFilterGraph *graph)
 {
     for (unsigned i = 0; i < graph->nb_filters; i++) {
@@ -1913,6 +1920,125 @@ static int graph_is_meta(AVFilterGraph *graph)
     return 1;
 }
 
+static int get_link_id(AVFilterLink ***links, int *nb_links, AVFilterLink *link)
+{
+    int ret;
+    for (int i = 0; i < *nb_links; i++) {
+        if ((*links)[i] == link)
+            return i;
+    }
+
+    ret = av_dynarray_add_nofree(links, nb_links, link);
+    return ret ? ret : *nb_links - 1;
+}
+
+static int print_complex_filter_graph(FilterGraph *fg, FilterGraphThread *fgt)
+{
+    const AVFilterGraph *graph = fgt->graph;
+
+    /* Keep track of seen filter links to assign a unique ID to each */
+    AVFilterLink **links = NULL;
+    int nb_links = 0;
+    int ret = AVERROR(ENOMEM);
+    char *opts = NULL;
+
+    AVBPrint buf;
+    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
+
+    for (int i = 0; i < graph->nb_filters; i++) {
+        AVFilterContext *filter = graph->filters[i];
+        if (i == 0)
+            av_bprintf(&buf, "Filter graph:\n");
+        if (filter_is_buffersrc(filter) || filter_is_buffersink(filter))
+            continue;
+
+        ret = av_opt_serialize(filter, AV_OPT_FLAG_FILTERING_PARAM,
+                               AV_OPT_SERIALIZE_SKIP_DEFAULTS |
+                               AV_OPT_SERIALIZE_SEARCH_CHILDREN, &opts, '=', ':');
+        if (ret < 0)
+            goto fail;
+
+        av_bprintf(&buf, "  ");
+        for (int j = 0; j < filter->nb_inputs; j++) {
+            AVFilterLink *link = filter->inputs[j];
+            if (filter_is_buffersrc(link->src)) {
+                for (int k = 0; k < fg->nb_inputs; k++) {
+                    InputFilterPriv *ifp = ifp_from_ifilter(fg->inputs[k]);
+                    if (ifp->filter != link->src)
+                        continue;
+                    av_bprintf(&buf, "[%s] ", ifp->opts.name);
+                    break;
+                }
+            } else {
+                ret = get_link_id(&links, &nb_links, link);
+                if (ret < 0)
+                    goto fail;
+                av_bprintf(&buf, "[L%d] ", ret);
+            }
+        }
+
+        av_bprintf(&buf, "%s=%s", filter->filter->name, opts);
+        av_freep(&opts);
+
+        for (int j = 0; j < filter->nb_outputs; j++) {
+            AVFilterLink *link = filter->outputs[j];
+            if (filter_is_buffersink(link->dst)) {
+                for (int k = 0; k < fg->nb_outputs; k++) {
+                    OutputFilterPriv *ofp = ofp_from_ofilter(fg->outputs[k]);
+                    if (ofp->filter == link->dst) {
+                        av_bprintf(&buf, " [%s]", ofp->name);
+                        break;
+                    }
+                }
+            } else {
+                ret = get_link_id(&links, &nb_links, link);
+                if (ret < 0)
+                    goto fail;
+                av_bprintf(&buf, " [L%d]", ret);
+            }
+        }
+        av_bprintf(&buf, ";\n");
+    }
+
+    /* Dump a summary of all seen links */
+    for (int i = 0; i < nb_links; i++) {
+        AVFilterLink *link = links[i];
+        if (i == 0)
+            av_bprintf(&buf, "Filter links:\n");
+        switch (link->type) {
+        case AVMEDIA_TYPE_VIDEO:
+            av_bprintf(&buf, "  L%d: %s %dx%d [SAR %d:%d] csp:%s range:%s\n",
+                       i, av_get_pix_fmt_name(link->format), link->w, link->h,
+                       link->sample_aspect_ratio.num, link->sample_aspect_ratio.den,
+                       av_color_space_name(link->colorspace),
+                       av_color_range_name(link->color_range));
+            break;
+        case AVMEDIA_TYPE_AUDIO:
+            av_bprintf(&buf, "  L%d: %s %dHz ",
+                       i, av_get_sample_fmt_name(link->format), link->sample_rate);
+            av_channel_layout_describe_bprint(&link->ch_layout, &buf);
+            av_bprintf(&buf, "\n");
+            break;
+        default: continue;
+        }
+    }
+
+    if (!av_bprint_is_complete(&buf)) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    if (buf.len)
+        av_log(NULL, AV_LOG_INFO, "%s", buf.str);
+
+    ret = 0;
+fail:
+    av_bprint_finalize(&buf, NULL);
+    av_free(links);
+    av_free(opts);
+    return ret;
+}
+
 static int sub2video_frame(InputFilter *ifilter, AVFrame *frame, int buffer);
 
 static int configure_filtergraph(FilterGraph *fg, FilterGraphThread *fgt)
@@ -1990,6 +2116,12 @@ static int configure_filtergraph(FilterGraph *fg, FilterGraphThread *fgt)
     if ((ret = avfilter_graph_config(fgt->graph, NULL)) < 0)
         goto fail;
 
+    /* Print the generated filter graph after insertion of auto filters */
+    if (print_filter_graph) {
+        if ((ret = print_complex_filter_graph(fg, fgt)) < 0)
+            goto fail;
+    }
+
     fgp->is_meta = graph_is_meta(fgt->graph);
 
     /* limit the lists of allowed formats to the ones selected, to
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 3c0c682594..8795cb1107 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -76,6 +76,7 @@ char *filter_nbthreads;
 int filter_complex_nbthreads = 0;
 int vstats_version = 2;
 int auto_conversion_filters = 1;
+int print_filter_graph = 0;
 int64_t stats_period = 500000;
 
 
@@ -1733,6 +1734,9 @@ const OptionDef options[] = {
     { "auto_conversion_filters", OPT_TYPE_BOOL, OPT_EXPERT,
         { &auto_conversion_filters },
         "enable automatic conversion filters globally" },
+    { "print_filter_graph", OPT_TYPE_BOOL, 0,
+        { &print_filter_graph },
+        "dump complex filter graph after insertion of auto-filters" },
     { "stats",               OPT_TYPE_BOOL, 0,
         { &print_stats },
         "print progress report during encoding", },
-- 
2.47.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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
  2025-02-17 12:17 [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option Niklas Haas
@ 2025-02-17 12:42 ` Nicolas George
  2025-02-17 13:40   ` Soft Works
  2025-02-17 13:53   ` Niklas Haas
  0 siblings, 2 replies; 6+ messages in thread
From: Nicolas George @ 2025-02-17 12:42 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Niklas Haas (HE12025-02-17):
> From: Niklas Haas <git@haasn.dev>
> 
> This developer tool is especially handy when debugging filter graph
> auto-negotiation, although it can be useful in whatever scenario to
> get a canonical dump of the fully settled filter graph.
> 
> To make the result slightly more useful, we omit buffersrc/buffersink
> filters and instead print the corresponding input/output name. Sadly, this
> is lossy w.r.t. the link names used in the original filter graph, although
> the result has the advantage of being in a normalized format.
> 
> As an example, the following filter graph (taken from FATE):
> 
>   sws_flags=+accurate_rnd+bitexact;
>   split [main][over];
>   [over] scale=88:72, pad=96:80:4:4 [overf];
>   [main][overf] overlay=240:16:format=yuv422
> 
> Results in this output:
> 
> Filter graph:
>   [0:0] split=thread_type=0x00000000 [L0] [L1];
>   [L1] scale=w=88:width=88:h=72:height=72:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L2];
>   [L2] pad=width=96:w=96:height=80:h=80:x=4:y=4:thread_type=0x00000000 [L3];
>   [L4] [L3] overlay=x=240:y=16:format=2 [#0:0];
>   [L0] scale=w=iw:width=iw:h=ih:height=ih:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L4];
> Filter links:
>   L0: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
>   L1: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
>   L2: yuva422p 88x72 [SAR 0:1] csp:unknown range:tv
>   L3: yuva422p 96x80 [SAR 0:1] csp:unknown range:tv
>   L4: yuva422p 352x288 [SAR 0:1] csp:unknown range:tv
> 
> I do acknowledge the overlap between this and avfilter/graphdump.c, but there
> are a couple of important deviations:
> 

> 1. graphdump.c prints a "pretty printed" ASCII art graph for human consumption,
>    but the goal here is to print it in a format that can be passed back to
>    -filter_complex.

The ASCII art output is shitty indeed. But notice that
avfilter_graph_dump() takes an options argument that is set from the
argument of the dumpgraph option. This is precisely meant to allow
future different output formats.

> 2. graphdump.c does not know anything about buffersrc/buffersink filters or
>    input/output names, which is why this implementation has to live inside
>    ffmpeg_filter.c.

I find this a very feeble reason to duplicate the feature.

> It's possible that we could instead move this implementation to graphdump.c as
> well, though that would require some sort of explicit callback to get the names
> for buffer sources / sinks, to avoid breaking the first design goal.

It would be enough to add “const char *name_in, *name_out;” to
AVFilterLink. Or a single parsed_name field to AVFilterPad.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
  2025-02-17 12:42 ` Nicolas George
@ 2025-02-17 13:40   ` Soft Works
  2025-02-17 13:50     ` Niklas Haas
  2025-02-17 13:53   ` Niklas Haas
  1 sibling, 1 reply; 6+ messages in thread
From: Soft Works @ 2025-02-17 13:40 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Montag, 17. Februar 2025 13:43
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> print_filter_graph option
> 
> Niklas Haas (HE12025-02-17):
> > From: Niklas Haas <git@haasn.dev>
> >
> > This developer tool is especially handy when debugging filter graph
> > auto-negotiation, although it can be useful in whatever scenario to
> > get a canonical dump of the fully settled filter graph.
> >
> > To make the result slightly more useful, we omit buffersrc/buffersink
> > filters and instead print the corresponding input/output name. Sadly, this
> > is lossy w.r.t. the link names used in the original filter graph, although
> > the result has the advantage of being in a normalized format.
> >
> > As an example, the following filter graph (taken from FATE):
> >
> >   sws_flags=+accurate_rnd+bitexact;
> >   split [main][over];
> >   [over] scale=88:72, pad=96:80:4:4 [overf];
> >   [main][overf] overlay=240:16:format=yuv422
> >
> > Results in this output:
> >
> > Filter graph:
> >   [0:0] split=thread_type=0x00000000 [L0] [L1];
> >   [L1]
> scale=w=88:width=88:h=72:height=72:flags=+accurate_rnd+bitexact:thread_t
> ype=0x00000000 [L2];
> >   [L2]
> pad=width=96:w=96:height=80:h=80:x=4:y=4:thread_type=0x00000000
> [L3];
> >   [L4] [L3] overlay=x=240:y=16:format=2 [#0:0];
> >   [L0]
> scale=w=iw:width=iw:h=ih:height=ih:flags=+accurate_rnd+bitexact:thread_typ
> e=0x00000000 [L4];
> > Filter links:
> >   L0: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
> >   L1: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
> >   L2: yuva422p 88x72 [SAR 0:1] csp:unknown range:tv
> >   L3: yuva422p 96x80 [SAR 0:1] csp:unknown range:tv
> >   L4: yuva422p 352x288 [SAR 0:1] csp:unknown range:tv
> >
> > I do acknowledge the overlap between this and avfilter/graphdump.c, but
> there
> > are a couple of important deviations:
> >
> 
> > 1. graphdump.c prints a "pretty printed" ASCII art graph for human
> consumption,
> >    but the goal here is to print it in a format that can be passed back to
> >    -filter_complex.
> 
> The ASCII art output is shitty indeed. But notice that
> avfilter_graph_dump() takes an options argument that is set from the
> argument of the dumpgraph option. This is precisely meant to allow
> future different output formats.

Well, then the question would be when or whether this future will become a reality?

Or maybe the solution rather lies in the past?

IIRC, I had already submitted a patchset with a complete solution:

- It includes absolutely all relevant information
  - filter string from the command line
  - filter name and description
  - parsed options
  - formats, hw context
  - all input and output pads/connections
    including all format information

- It prints using the same method like FFprobe, so it can print as
  - default
  - xml
  - json
  - csv

- Here's an example JSON output:

https://gist.github.com/softworkz/2a9e8699b288f5d40fa381c2a496e165


I can resubmit it, it's the most comprehensive implementation in this regard.

sw









_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
  2025-02-17 13:40   ` Soft Works
@ 2025-02-17 13:50     ` Niklas Haas
  0 siblings, 0 replies; 6+ messages in thread
From: Niklas Haas @ 2025-02-17 13:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Mon, 17 Feb 2025 13:40:49 +0000 Soft Works <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> Well, then the question would be when or whether this future will become a reality?
>
> Or maybe the solution rather lies in the past?
>
> IIRC, I had already submitted a patchset with a complete solution:
>
> - It includes absolutely all relevant information
>   - filter string from the command line
>   - filter name and description
>   - parsed options
>   - formats, hw context
>   - all input and output pads/connections
>     including all format information
>
> - It prints using the same method like FFprobe, so it can print as
>   - default
>   - xml
>   - json
>   - csv
>
> - Here's an example JSON output:
>
> https://gist.github.com/softworkz/2a9e8699b288f5d40fa381c2a496e165

That seems amazing and exactly what I need.

>
>
> I can resubmit it, it's the most comprehensive implementation in this regard.

Yes, please resubmit it. I will review it.

>
> sw
>
>
>
>
>
>
>
>
>
> _______________________________________________
> 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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
  2025-02-17 12:42 ` Nicolas George
  2025-02-17 13:40   ` Soft Works
@ 2025-02-17 13:53   ` Niklas Haas
  2025-02-17 15:35     ` Nicolas George
  1 sibling, 1 reply; 6+ messages in thread
From: Niklas Haas @ 2025-02-17 13:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Mon, 17 Feb 2025 13:42:47 +0100 Nicolas George <george@nsup.org> wrote:
> Niklas Haas (HE12025-02-17):
> > From: Niklas Haas <git@haasn.dev>
> >
> > This developer tool is especially handy when debugging filter graph
> > auto-negotiation, although it can be useful in whatever scenario to
> > get a canonical dump of the fully settled filter graph.
> >
> > To make the result slightly more useful, we omit buffersrc/buffersink
> > filters and instead print the corresponding input/output name. Sadly, this
> > is lossy w.r.t. the link names used in the original filter graph, although
> > the result has the advantage of being in a normalized format.
> >
> > As an example, the following filter graph (taken from FATE):
> >
> >   sws_flags=+accurate_rnd+bitexact;
> >   split [main][over];
> >   [over] scale=88:72, pad=96:80:4:4 [overf];
> >   [main][overf] overlay=240:16:format=yuv422
> >
> > Results in this output:
> >
> > Filter graph:
> >   [0:0] split=thread_type=0x00000000 [L0] [L1];
> >   [L1] scale=w=88:width=88:h=72:height=72:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L2];
> >   [L2] pad=width=96:w=96:height=80:h=80:x=4:y=4:thread_type=0x00000000 [L3];
> >   [L4] [L3] overlay=x=240:y=16:format=2 [#0:0];
> >   [L0] scale=w=iw:width=iw:h=ih:height=ih:flags=+accurate_rnd+bitexact:thread_type=0x00000000 [L4];
> > Filter links:
> >   L0: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
> >   L1: yuv420p 352x288 [SAR 0:1] csp:unknown range:tv
> >   L2: yuva422p 88x72 [SAR 0:1] csp:unknown range:tv
> >   L3: yuva422p 96x80 [SAR 0:1] csp:unknown range:tv
> >   L4: yuva422p 352x288 [SAR 0:1] csp:unknown range:tv
> >
> > I do acknowledge the overlap between this and avfilter/graphdump.c, but there
> > are a couple of important deviations:
> >
>
> > 1. graphdump.c prints a "pretty printed" ASCII art graph for human consumption,
> >    but the goal here is to print it in a format that can be passed back to
> >    -filter_complex.
>
> The ASCII art output is shitty indeed. But notice that
> avfilter_graph_dump() takes an options argument that is set from the
> argument of the dumpgraph option. This is precisely meant to allow
> future different output formats.
>
> > 2. graphdump.c does not know anything about buffersrc/buffersink filters or
> >    input/output names, which is why this implementation has to live inside
> >    ffmpeg_filter.c.
>
> I find this a very feeble reason to duplicate the feature.
>
> > It's possible that we could instead move this implementation to graphdump.c as
> > well, though that would require some sort of explicit callback to get the names
> > for buffer sources / sinks, to avoid breaking the first design goal.
>
> It would be enough to add “const char *name_in, *name_out;” to
> AVFilterLink. Or a single parsed_name field to AVFilterPad.

For the record, this approach seems to work well - adding a const char *label
to the AVFilterPad struct and then setting it during
avfilter_graph_segment_create_filters() recovers the full label names without
any need for shenanigans.

That aside, how do you feel about scrapping the current ASCII art output entirely
in favor of the new format? I think that it is, at the very least, providing
just as much information, and IMO more readable. The ASCII art graph is not
really enhancing clarity for me.

(Though given what Soft Works noted below, it may be a good idea to focus that
version instead)

>
> Regards,
>
> --
>   Nicolas George
> _______________________________________________
> 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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option
  2025-02-17 13:53   ` Niklas Haas
@ 2025-02-17 15:35     ` Nicolas George
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas George @ 2025-02-17 15:35 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Niklas Haas (HE12025-02-17):
> For the record, this approach seems to work well - adding a const char *label
> to the AVFilterPad struct and then setting it during
> avfilter_graph_segment_create_filters() recovers the full label names without
> any need for shenanigans.

Thanks for looking into it.

> That aside, how do you feel about scrapping the current ASCII art
> output entirely in favor of the new format? I think that it is, at the
> very least, providing just as much information, and IMO more readable.
> The ASCII art graph is not really enhancing clarity for me.
> 
> (Though given what Soft Works noted below, it may be a good idea to focus that
> version instead)

We need at least two formats: one easy to read for humans, one reliable
to parse for scripts. Therefore, we need a mechanism to select the
format. Under these circumstances, I see no merit in removing the
current format; somebody might have built something around it.

(Ideally, the computer-readable outputs should offer various formats:
JSON, XML, etc. For that, we would need to move into the library with a
proper API the code in ffprobe. And for that we need a good strings API.
Yet another thing blocked by the people blocking AVWriter.)

On the other hand, I do not object to a new format being made the
default.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-02-17 15:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-17 12:17 [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option Niklas Haas
2025-02-17 12:42 ` Nicolas George
2025-02-17 13:40   ` Soft Works
2025-02-17 13:50     ` Niklas Haas
2025-02-17 13:53   ` Niklas Haas
2025-02-17 15:35     ` Nicolas George

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git