Re: [FFmpeg-devel] [PATCH 1/5] lavfi/graphdump: add plain listing output

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Nicolas George:
> Signed-off-by: Nicolas George <george@nsup.org>
> ---
>  libavfilter/avfilter.h  |  5 ++-
>  libavfilter/graphdump.c | 79 +++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 80 insertions(+), 4 deletions(-)
> 
> 
> Unchanged since last summer.
> The last patch is what this is needed for.
> 
> 
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index b105dc3159..b338766609 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -1153,7 +1153,10 @@ int avfilter_graph_queue_command(AVFilterGraph *graph, const char *target, const
>   * Dump a graph into a human-readable string representation.
>   *
>   * @param graph    the graph to dump
> - * @param options  formatting options; currently ignored
> + * @param options  formatting options; can be NULL, empty
> + *                 or "f=aa" for clumsy ascii-art drawing,
> + *                 or "f=tech" for plain listing;
> + *                 other values silently ignored
>   * @return  a string, or NULL in case of memory allocation failure;
>   *          the string must be freed using av_free
>   */
> diff --git a/libavfilter/graphdump.c b/libavfilter/graphdump.c
> index cf8914b558..80dbccc66a 100644
> --- a/libavfilter/graphdump.c
> +++ b/libavfilter/graphdump.c
> @@ -27,6 +27,9 @@
>  #include "avfilter.h"
>  #include "internal.h"
>  
> +#define FORMAT_AA 0
> +#define FORMAT_TECH 1
> +
>  static int print_link_prop(AVBPrint *buf, AVFilterLink *link)
>  {
>      const char *format;
> @@ -60,7 +63,51 @@ static int print_link_prop(AVBPrint *buf, AVFilterLink *link)
>      return buf->len;
>  }
>  
> -static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)

The name of avfilter_graph_dump_to_buf makes it seem as if this were a
public function due to its prefix. Given that you already change the
signature, you already change every occurence of
avfilter_graph_dump_to_buf. So could you also rename it to a name more
befitting of a static function?

> +static inline const char *fcname(const AVFilterContext *filter)
> +{
> +    return filter->name ? filter->name : "<unnamed>";
> +}
> +
> +static void dump_tech(AVBPrint *buf, AVFilterGraph *graph)
> +{
> +    unsigned i, j;
> +
> +    for (i = 0; i < graph->nb_filters; i++) {
> +        AVFilterContext *filter = graph->filters[i];
> +
> +        if (i)
> +            av_bprintf(buf, "\n");
> +        av_bprintf(buf, "Filter: %s (%s)\n", fcname(filter), filter->filter->name);
> +
> +        for (j = 0; j < filter->nb_inputs; j++) {
> +            AVFilterPad *pad = &filter->input_pads[j];
> +            AVFilterLink *link = filter->inputs[j];
> +            AVFilterPad *ppad = link->srcpad;
> +            AVFilterContext *peer = link->src;
> +
> +            av_bprintf(buf, "  in %d: %s ← %s.%d:%s ",
> +                       j, pad->name,
> +                       fcname(peer), FF_OUTLINK_IDX(link), ppad->name);
> +            print_link_prop(buf, link);
> +            av_bprintf(buf, "\n");
> +        }
> +
> +        for (j = 0; j < filter->nb_outputs; j++) {
> +            AVFilterPad *pad = &filter->output_pads[j];
> +            AVFilterLink *link = filter->outputs[j];
> +            AVFilterPad *ppad = link->dstpad;
> +            AVFilterContext *peer = link->dst;
> +
> +            av_bprintf(buf, "  out %d: %s → %s.%d:%s ",
> +                       j, pad->name,
> +                       fcname(peer), FF_INLINK_IDX(link), ppad->name);
> +            print_link_prop(buf, link);
> +            av_bprintf(buf, "\n");
> +        }
> +    }
> +}
> +
> +static void dump_ascii_art(AVBPrint *buf, AVFilterGraph *graph)
>  {
>      unsigned i, j, x, e;
>  
> @@ -152,17 +199,43 @@ static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)
>      }
>  }
>  
> +static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph, const char *options)
> +{
> +    unsigned format = FORMAT_AA;
> +
> +    /* For a very long time, options was ignored.
> +       Having a string for that task was a mistake, but it is not important.
> +       It is not worth a proper parsing.
> +     */
> +    if (options && *options) {
> +        if (!strcmp("f=aa", options)) {
> +            format = FORMAT_AA;
> +        } else if (!strcmp("f=tech", options)) {
> +            format = FORMAT_TECH;
> +        }
> +        /* ignore other values */
> +    }
> +    switch (format) {
> +    case FORMAT_AA:
> +        dump_ascii_art(buf, graph);
> +        break;
> +    case FORMAT_TECH:
> +        dump_tech(buf, graph);
> +        break;
> +    }
> +}
> +
>  char *avfilter_graph_dump(AVFilterGraph *graph, const char *options)
>  {
>      AVBPrint buf;
>      char *dump = NULL;
>  
>      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
> -    avfilter_graph_dump_to_buf(&buf, graph);
> +    avfilter_graph_dump_to_buf(&buf, graph, options);
>      dump = av_malloc(buf.len + 1);
>      if (!dump)
>          return NULL;
>      av_bprint_init_for_buffer(&buf, dump, buf.len + 1);
> -    avfilter_graph_dump_to_buf(&buf, graph);
> +    avfilter_graph_dump_to_buf(&buf, graph, options);
>      return dump;
>  }

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