Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -print_filter_graph option

[Niklas Haas <ffmpeg@haasn.xyz>]

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