[FFmpeg-devel,v4,1/2] fftools: add options to dump filter graph

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 doc/ffmpeg.texi         | 15 ++++++++++++
 fftools/ffmpeg.h        |  2 ++
 fftools/ffmpeg_filter.c | 20 ++++++++++++++++
 fftools/ffmpeg_opt.c    | 20 ++++++++++++++++
 libavfilter/graphdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)

On Sat, May 23, 2020 at 2:23 AM <lance.lmwang@gmail.com> wrote:
>
> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  doc/ffmpeg.texi         | 15 ++++++++++++
>  fftools/ffmpeg.h        |  2 ++
>  fftools/ffmpeg_filter.c | 20 ++++++++++++++++
>  fftools/ffmpeg_opt.c    | 20 ++++++++++++++++
>  libavfilter/graphdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 120 insertions(+)
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index ed437bb..699991f 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -735,6 +735,21 @@ Technical note -- attachments are implemented as codec extradata, so this
>  option can actually be used to extract extradata from any stream, not just
>  attachments.
>
> +@item -dump_filtergraph @var{filename} (@emph{global})
> +Set the output file name of filter graph for dump.
> +
> +It is "ASCII" format by default. for Graphviz DOT output format,
> +you can convert it to png by GraphViz tool:
> +@example
> +dot -Tpng dump_fg_filename -o dump_graph.png
> +@end example
> +
> +@item -dump_filtergraph_format @var{format} (@emph{global})
> +Set the output format of filter graph for dump. Support format: "DOT", "ASCII".
> +
> +DOT is the text file format of the suite GraphViz, ASCII is the text file format
> +of ASCII style.
> +
>  @item -noautorotate
>  Disable automatically rotating video based on file metadata.
>
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 38205a1..55f115b 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -606,6 +606,8 @@ extern AVIOContext *progress_avio;
>  extern float max_error_rate;
>  extern char *videotoolbox_pixfmt;
>
> +extern char* dump_fg_filename;
> +extern char* dump_fg_format;
>  extern int filter_nbthreads;
>  extern int filter_complex_nbthreads;
>  extern int vstats_version;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 8b5b157..485fc73 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -1106,6 +1106,26 @@ int configure_filtergraph(FilterGraph *fg)
>      if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
>          goto fail;
>
> +    if (dump_fg_filename) {
> +        char *dump = avfilter_graph_dump(fg->graph, dump_fg_format);
> +        FILE *fg_file = fopen(dump_fg_filename, "w");
> +
> +        if (!dump) {
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +        if (fg_file) {
> +            fputs(dump, fg_file);
> +            fflush(fg_file);
> +            fclose(fg_file);
> +        } else {
> +            ret = AVERROR(EINVAL);
> +            av_free(dump);
> +            goto fail;
> +        }
> +        av_free(dump);
> +    }
> +
>      /* limit the lists of allowed formats to the ones selected, to
>       * make sure they stay the same if the filtergraph is reconfigured later */
>      for (i = 0; i < fg->nb_outputs; i++) {
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 60bb437..bdd8957 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -143,6 +143,8 @@ HWDevice *filter_hw_device;
>
>  char *vstats_filename;
>  char *sdp_filename;
> +char *dump_fg_filename;
> +char *dump_fg_format;
>
>  float audio_drift_threshold = 0.1;
>  float dts_delta_threshold   = 10;
> @@ -2930,6 +2932,20 @@ static int opt_vstats(void *optctx, const char *opt, const char *arg)
>      return opt_vstats_file(NULL, opt, filename);
>  }
>
> +static int opt_fg_filename(void *optctx, const char *opt, const char *arg)
> +{
> +    av_free (dump_fg_filename);
> +    dump_fg_filename = av_strdup (arg);
> +    return 0;
> +}
> +
> +static int opt_fg_format(void *optctx, const char *opt, const char *arg)
> +{
> +    av_free (dump_fg_format);
> +    dump_fg_format = av_strdup (arg);
> +    return 0;
> +}
> +
>  static int opt_video_frames(void *optctx, const char *opt, const char *arg)
>  {
>      OptionsContext *o = optctx;
> @@ -3548,6 +3564,10 @@ const OptionDef options[] = {
>      { "dump_attachment", HAS_ARG | OPT_STRING | OPT_SPEC |
>                           OPT_EXPERT | OPT_INPUT,                     { .off = OFFSET(dump_attachment) },
>          "extract an attachment into a file", "filename" },
> +    { "dump_filtergraph",HAS_ARG | OPT_EXPERT,                       { .func_arg = opt_fg_filename },
> +        "the output filename of filter graph for dump", "filename" },
> +    { "dump_filtergraph_format", HAS_ARG | OPT_EXPERT,               { .func_arg = opt_fg_format },
> +        "the format of filter graph for dump, support DOT or ASCII"},

Possible to use AV_OPT_TYPE_CONST for these? maybe lowercase names?

Output for dot graph will very useful for me, thanks!

>      { "stream_loop", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_INPUT |
>                          OPT_OFFSET,                                  { .off = OFFSET(loop) }, "set number of times input stream shall be looped", "loop count" },
>      { "debug_ts",       OPT_BOOL | OPT_EXPERT,                       { &debug_ts },
> diff --git a/libavfilter/graphdump.c b/libavfilter/graphdump.c
> index 79ef1a7..b15f498 100644
> --- a/libavfilter/graphdump.c
> +++ b/libavfilter/graphdump.c
> @@ -151,15 +151,78 @@ static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)
>      }
>  }
>
> +static void avfilter_graph2dot_to_buf(AVBPrint *buf, AVFilterGraph *graph)
> +{
> +    int i, j;
> +
> +    av_bprintf(buf, "digraph G {\n");
> +    av_bprintf(buf, "node [shape=box]\n");
> +    av_bprintf(buf, "rankdir=LR\n");
> +
> +    for (i = 0; i < graph->nb_filters; i++) {
> +        char filter_ctx_label[128];
> +        const AVFilterContext *filter_ctx = graph->filters[i];
> +
> +        snprintf(filter_ctx_label, sizeof(filter_ctx_label), "%s\\n(%s)",
> +                 filter_ctx->name,
> +                 filter_ctx->filter->name);
> +
> +        for (j = 0; j < filter_ctx->nb_outputs; j++) {
> +            AVFilterLink *link = filter_ctx->outputs[j];
> +            if (link) {
> +                char dst_filter_ctx_label[128];
> +                const AVFilterContext *dst_filter_ctx = link->dst;
> +
> +                snprintf(dst_filter_ctx_label, sizeof(dst_filter_ctx_label),
> +                         "%s\\n(%s)",
> +                         dst_filter_ctx->name,
> +                         dst_filter_ctx->filter->name);
> +
> +                av_bprintf(buf, "\"%s\" -> \"%s\" [ label= \"inpad:%s -> outpad:%s\\n",
> +                        filter_ctx_label, dst_filter_ctx_label,
> +                        avfilter_pad_get_name(link->srcpad, 0),
> +                        avfilter_pad_get_name(link->dstpad, 0));
> +
> +                if (link->type == AVMEDIA_TYPE_VIDEO) {
> +                    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> +                    av_bprintf(buf,
> +                            "fmt:%s w:%d h:%d tb:%d/%d",
> +                            desc->name,
> +                            link->w, link->h,
> +                            link->time_base.num, link->time_base.den);
> +                } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> +                    char audio_buf[255];
> +                    av_get_channel_layout_string(audio_buf, sizeof(audio_buf), -1,
> +                                                 link->channel_layout);
> +                    av_bprintf(buf,
> +                            "fmt:%s sr:%d cl:%s tb:%d/%d",
> +                            av_get_sample_fmt_name(link->format),
> +                            link->sample_rate, audio_buf,
> +                            link->time_base.num, link->time_base.den);
> +                }
> +                av_bprintf(buf, "\" ];\n");
> +            }
> +        }
> +    }
> +    av_bprintf(buf, "}\n");
> +}
> +
>  char *avfilter_graph_dump(AVFilterGraph *graph, const char *options)
>  {
>      AVBPrint buf;
>      char *dump = NULL;
>
> +    if (options && !strcmp(options, "DOT")) {
> +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> +    avfilter_graph2dot_to_buf(&buf, graph);
> +    av_bprint_finalize(&buf, &dump);
> +    } else {
>      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
>      avfilter_graph_dump_to_buf(&buf, graph);
>      av_bprint_init(&buf, buf.len + 1, buf.len + 1);
>      avfilter_graph_dump_to_buf(&buf, graph);
>      av_bprint_finalize(&buf, &dump);
> +    }
> +
>      return dump;
>  }
> --
> 2.6.4
>
> _______________________________________________
> 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".

lance.lmwang@gmail.com (12020-05-23):
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  doc/ffmpeg.texi         | 15 ++++++++++++

>  fftools/ffmpeg.h        |  2 ++
>  fftools/ffmpeg_filter.c | 20 ++++++++++++++++
>  fftools/ffmpeg_opt.c    | 20 ++++++++++++++++
>  libavfilter/graphdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++

Changes in the library belong and changes in the command-line tools
belong in different patches with corresponding commit messages.

>  5 files changed, 120 insertions(+)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index ed437bb..699991f 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -735,6 +735,21 @@ Technical note -- attachments are implemented as codec extradata, so this
>  option can actually be used to extract extradata from any stream, not just
>  attachments.
>  
> +@item -dump_filtergraph @var{filename} (@emph{global})
> +Set the output file name of filter graph for dump.
> +
> +It is "ASCII" format by default. for Graphviz DOT output format,

> +you can convert it to png by GraphViz tool:

The is no CammelCase in the official name of this project. Same below.

> +@example
> +dot -Tpng dump_fg_filename -o dump_graph.png
> +@end example
> +
> +@item -dump_filtergraph_format @var{format} (@emph{global})
> +Set the output format of filter graph for dump. Support format: "DOT", "ASCII".
> +
> +DOT is the text file format of the suite GraphViz, ASCII is the text file format
> +of ASCII style.
> +
>  @item -noautorotate
>  Disable automatically rotating video based on file metadata.
>  
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 38205a1..55f115b 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -606,6 +606,8 @@ extern AVIOContext *progress_avio;
>  extern float max_error_rate;
>  extern char *videotoolbox_pixfmt;
>  
> +extern char* dump_fg_filename;
> +extern char* dump_fg_format;
>  extern int filter_nbthreads;
>  extern int filter_complex_nbthreads;
>  extern int vstats_version;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 8b5b157..485fc73 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -1106,6 +1106,26 @@ int configure_filtergraph(FilterGraph *fg)
>      if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
>          goto fail;
>  
> +    if (dump_fg_filename) {
> +        char *dump = avfilter_graph_dump(fg->graph, dump_fg_format);
> +        FILE *fg_file = fopen(dump_fg_filename, "w");
> +
> +        if (!dump) {
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }

> +        if (fg_file) {
> +            fputs(dump, fg_file);
> +            fflush(fg_file);
> +            fclose(fg_file);
> +        } else {
> +            ret = AVERROR(EINVAL);
> +            av_free(dump);
> +            goto fail;
> +        }

Spaghetti code. Check errors where they happen, not in a weird else
clause.

> +        av_free(dump);
> +    }
> +
>      /* limit the lists of allowed formats to the ones selected, to
>       * make sure they stay the same if the filtergraph is reconfigured later */
>      for (i = 0; i < fg->nb_outputs; i++) {
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 60bb437..bdd8957 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -143,6 +143,8 @@ HWDevice *filter_hw_device;
>  
>  char *vstats_filename;
>  char *sdp_filename;
> +char *dump_fg_filename;
> +char *dump_fg_format;
>  
>  float audio_drift_threshold = 0.1;
>  float dts_delta_threshold   = 10;
> @@ -2930,6 +2932,20 @@ static int opt_vstats(void *optctx, const char *opt, const char *arg)
>      return opt_vstats_file(NULL, opt, filename);
>  }
>  
> +static int opt_fg_filename(void *optctx, const char *opt, const char *arg)
> +{
> +    av_free (dump_fg_filename);

> +    dump_fg_filename = av_strdup (arg);

Stray space. Same below.

> +    return 0;
> +}
> +
> +static int opt_fg_format(void *optctx, const char *opt, const char *arg)
> +{
> +    av_free (dump_fg_format);
> +    dump_fg_format = av_strdup (arg);
> +    return 0;
> +}
> +
>  static int opt_video_frames(void *optctx, const char *opt, const char *arg)
>  {
>      OptionsContext *o = optctx;
> @@ -3548,6 +3564,10 @@ const OptionDef options[] = {
>      { "dump_attachment", HAS_ARG | OPT_STRING | OPT_SPEC |
>                           OPT_EXPERT | OPT_INPUT,                     { .off = OFFSET(dump_attachment) },
>          "extract an attachment into a file", "filename" },
> +    { "dump_filtergraph",HAS_ARG | OPT_EXPERT,                       { .func_arg = opt_fg_filename },
> +        "the output filename of filter graph for dump", "filename" },
> +    { "dump_filtergraph_format", HAS_ARG | OPT_EXPERT,               { .func_arg = opt_fg_format },
> +        "the format of filter graph for dump, support DOT or ASCII"},
>      { "stream_loop", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_INPUT |
>                          OPT_OFFSET,                                  { .off = OFFSET(loop) }, "set number of times input stream shall be looped", "loop count" },
>      { "debug_ts",       OPT_BOOL | OPT_EXPERT,                       { &debug_ts },
> diff --git a/libavfilter/graphdump.c b/libavfilter/graphdump.c
> index 79ef1a7..b15f498 100644
> --- a/libavfilter/graphdump.c
> +++ b/libavfilter/graphdump.c
> @@ -151,15 +151,78 @@ static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)
>      }
>  }
>  

> +static void avfilter_graph2dot_to_buf(AVBPrint *buf, AVFilterGraph *graph)

This code looks like it comes straight from tools/graph2dot.c. The
commit message should say it.

And code should not be duplicated. If the feature is now in the library,
tools/graph2dot.c should go away.

> +{
> +    int i, j;
> +
> +    av_bprintf(buf, "digraph G {\n");
> +    av_bprintf(buf, "node [shape=box]\n");
> +    av_bprintf(buf, "rankdir=LR\n");
> +
> +    for (i = 0; i < graph->nb_filters; i++) {
> +        char filter_ctx_label[128];
> +        const AVFilterContext *filter_ctx = graph->filters[i];
> +
> +        snprintf(filter_ctx_label, sizeof(filter_ctx_label), "%s\\n(%s)",
> +                 filter_ctx->name,
> +                 filter_ctx->filter->name);
> +
> +        for (j = 0; j < filter_ctx->nb_outputs; j++) {
> +            AVFilterLink *link = filter_ctx->outputs[j];
> +            if (link) {
> +                char dst_filter_ctx_label[128];
> +                const AVFilterContext *dst_filter_ctx = link->dst;
> +
> +                snprintf(dst_filter_ctx_label, sizeof(dst_filter_ctx_label),
> +                         "%s\\n(%s)",
> +                         dst_filter_ctx->name,
> +                         dst_filter_ctx->filter->name);
> +
> +                av_bprintf(buf, "\"%s\" -> \"%s\" [ label= \"inpad:%s -> outpad:%s\\n",
> +                        filter_ctx_label, dst_filter_ctx_label,
> +                        avfilter_pad_get_name(link->srcpad, 0),
> +                        avfilter_pad_get_name(link->dstpad, 0));
> +
> +                if (link->type == AVMEDIA_TYPE_VIDEO) {
> +                    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> +                    av_bprintf(buf,
> +                            "fmt:%s w:%d h:%d tb:%d/%d",
> +                            desc->name,
> +                            link->w, link->h,
> +                            link->time_base.num, link->time_base.den);
> +                } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> +                    char audio_buf[255];
> +                    av_get_channel_layout_string(audio_buf, sizeof(audio_buf), -1,
> +                                                 link->channel_layout);
> +                    av_bprintf(buf,
> +                            "fmt:%s sr:%d cl:%s tb:%d/%d",
> +                            av_get_sample_fmt_name(link->format),
> +                            link->sample_rate, audio_buf,
> +                            link->time_base.num, link->time_base.den);
> +                }
> +                av_bprintf(buf, "\" ];\n");
> +            }
> +        }
> +    }
> +    av_bprintf(buf, "}\n");
> +}
> +
>  char *avfilter_graph_dump(AVFilterGraph *graph, const char *options)
>  {
>      AVBPrint buf;
>      char *dump = NULL;
>  

> +    if (options && !strcmp(options, "DOT")) {

It is called "options", not "format", which, for this project, means
key-value pairs.

> +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> +    avfilter_graph2dot_to_buf(&buf, graph);
> +    av_bprint_finalize(&buf, &dump);
> +    } else {
>      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
>      avfilter_graph_dump_to_buf(&buf, graph);
>      av_bprint_init(&buf, buf.len + 1, buf.len + 1);
>      avfilter_graph_dump_to_buf(&buf, graph);
>      av_bprint_finalize(&buf, &dump);
> +    }
> +
>      return dump;
>  }

Regards,

On Sat, May 23, 2020 at 02:41:01PM +0200, Mattias Wadman wrote:
> On Sat, May 23, 2020 at 2:23 AM <lance.lmwang@gmail.com> wrote:
> >
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  doc/ffmpeg.texi         | 15 ++++++++++++
> >  fftools/ffmpeg.h        |  2 ++
> >  fftools/ffmpeg_filter.c | 20 ++++++++++++++++
> >  fftools/ffmpeg_opt.c    | 20 ++++++++++++++++
> >  libavfilter/graphdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 120 insertions(+)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index ed437bb..699991f 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -735,6 +735,21 @@ Technical note -- attachments are implemented as codec extradata, so this
> >  option can actually be used to extract extradata from any stream, not just
> >  attachments.
> >
> > +@item -dump_filtergraph @var{filename} (@emph{global})
> > +Set the output file name of filter graph for dump.
> > +
> > +It is "ASCII" format by default. for Graphviz DOT output format,
> > +you can convert it to png by GraphViz tool:
> > +@example
> > +dot -Tpng dump_fg_filename -o dump_graph.png
> > +@end example
> > +
> > +@item -dump_filtergraph_format @var{format} (@emph{global})
> > +Set the output format of filter graph for dump. Support format: "DOT", "ASCII".
> > +
> > +DOT is the text file format of the suite GraphViz, ASCII is the text file format
> > +of ASCII style.
> > +
> >  @item -noautorotate
> >  Disable automatically rotating video based on file metadata.
> >
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 38205a1..55f115b 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -606,6 +606,8 @@ extern AVIOContext *progress_avio;
> >  extern float max_error_rate;
> >  extern char *videotoolbox_pixfmt;
> >
> > +extern char* dump_fg_filename;
> > +extern char* dump_fg_format;
> >  extern int filter_nbthreads;
> >  extern int filter_complex_nbthreads;
> >  extern int vstats_version;
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 8b5b157..485fc73 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -1106,6 +1106,26 @@ int configure_filtergraph(FilterGraph *fg)
> >      if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
> >          goto fail;
> >
> > +    if (dump_fg_filename) {
> > +        char *dump = avfilter_graph_dump(fg->graph, dump_fg_format);
> > +        FILE *fg_file = fopen(dump_fg_filename, "w");
> > +
> > +        if (!dump) {
> > +            ret = AVERROR(ENOMEM);
> > +            goto fail;
> > +        }
> > +        if (fg_file) {
> > +            fputs(dump, fg_file);
> > +            fflush(fg_file);
> > +            fclose(fg_file);
> > +        } else {
> > +            ret = AVERROR(EINVAL);
> > +            av_free(dump);
> > +            goto fail;
> > +        }
> > +        av_free(dump);
> > +    }
> > +
> >      /* limit the lists of allowed formats to the ones selected, to
> >       * make sure they stay the same if the filtergraph is reconfigured later */
> >      for (i = 0; i < fg->nb_outputs; i++) {
> > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> > index 60bb437..bdd8957 100644
> > --- a/fftools/ffmpeg_opt.c
> > +++ b/fftools/ffmpeg_opt.c
> > @@ -143,6 +143,8 @@ HWDevice *filter_hw_device;
> >
> >  char *vstats_filename;
> >  char *sdp_filename;
> > +char *dump_fg_filename;
> > +char *dump_fg_format;
> >
> >  float audio_drift_threshold = 0.1;
> >  float dts_delta_threshold   = 10;
> > @@ -2930,6 +2932,20 @@ static int opt_vstats(void *optctx, const char *opt, const char *arg)
> >      return opt_vstats_file(NULL, opt, filename);
> >  }
> >
> > +static int opt_fg_filename(void *optctx, const char *opt, const char *arg)
> > +{
> > +    av_free (dump_fg_filename);
> > +    dump_fg_filename = av_strdup (arg);
> > +    return 0;
> > +}
> > +
> > +static int opt_fg_format(void *optctx, const char *opt, const char *arg)
> > +{
> > +    av_free (dump_fg_format);
> > +    dump_fg_format = av_strdup (arg);
> > +    return 0;
> > +}
> > +
> >  static int opt_video_frames(void *optctx, const char *opt, const char *arg)
> >  {
> >      OptionsContext *o = optctx;
> > @@ -3548,6 +3564,10 @@ const OptionDef options[] = {
> >      { "dump_attachment", HAS_ARG | OPT_STRING | OPT_SPEC |
> >                           OPT_EXPERT | OPT_INPUT,                     { .off = OFFSET(dump_attachment) },
> >          "extract an attachment into a file", "filename" },
> > +    { "dump_filtergraph",HAS_ARG | OPT_EXPERT,                       { .func_arg = opt_fg_filename },
> > +        "the output filename of filter graph for dump", "filename" },
> > +    { "dump_filtergraph_format", HAS_ARG | OPT_EXPERT,               { .func_arg = opt_fg_format },
> > +        "the format of filter graph for dump, support DOT or ASCII"},
> 
> Possible to use AV_OPT_TYPE_CONST for these? maybe lowercase names?

It's used as string, so I haven't use AV_OPT_TYPE_CONST. But it's more simple to use const anyway. 
lowercase is supported also I think. I'll consider how to use const anyway.

> 
> Output for dot graph will very useful for me, thanks!
> 
> >      { "stream_loop", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_INPUT |
> >                          OPT_OFFSET,                                  { .off = OFFSET(loop) }, "set number of times input stream shall be looped", "loop count" },
> >      { "debug_ts",       OPT_BOOL | OPT_EXPERT,                       { &debug_ts },
> > diff --git a/libavfilter/graphdump.c b/libavfilter/graphdump.c
> > index 79ef1a7..b15f498 100644
> > --- a/libavfilter/graphdump.c
> > +++ b/libavfilter/graphdump.c
> > @@ -151,15 +151,78 @@ static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)
> >      }
> >  }
> >
> > +static void avfilter_graph2dot_to_buf(AVBPrint *buf, AVFilterGraph *graph)
> > +{
> > +    int i, j;
> > +
> > +    av_bprintf(buf, "digraph G {\n");
> > +    av_bprintf(buf, "node [shape=box]\n");
> > +    av_bprintf(buf, "rankdir=LR\n");
> > +
> > +    for (i = 0; i < graph->nb_filters; i++) {
> > +        char filter_ctx_label[128];
> > +        const AVFilterContext *filter_ctx = graph->filters[i];
> > +
> > +        snprintf(filter_ctx_label, sizeof(filter_ctx_label), "%s\\n(%s)",
> > +                 filter_ctx->name,
> > +                 filter_ctx->filter->name);
> > +
> > +        for (j = 0; j < filter_ctx->nb_outputs; j++) {
> > +            AVFilterLink *link = filter_ctx->outputs[j];
> > +            if (link) {
> > +                char dst_filter_ctx_label[128];
> > +                const AVFilterContext *dst_filter_ctx = link->dst;
> > +
> > +                snprintf(dst_filter_ctx_label, sizeof(dst_filter_ctx_label),
> > +                         "%s\\n(%s)",
> > +                         dst_filter_ctx->name,
> > +                         dst_filter_ctx->filter->name);
> > +
> > +                av_bprintf(buf, "\"%s\" -> \"%s\" [ label= \"inpad:%s -> outpad:%s\\n",
> > +                        filter_ctx_label, dst_filter_ctx_label,
> > +                        avfilter_pad_get_name(link->srcpad, 0),
> > +                        avfilter_pad_get_name(link->dstpad, 0));
> > +
> > +                if (link->type == AVMEDIA_TYPE_VIDEO) {
> > +                    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> > +                    av_bprintf(buf,
> > +                            "fmt:%s w:%d h:%d tb:%d/%d",
> > +                            desc->name,
> > +                            link->w, link->h,
> > +                            link->time_base.num, link->time_base.den);
> > +                } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> > +                    char audio_buf[255];
> > +                    av_get_channel_layout_string(audio_buf, sizeof(audio_buf), -1,
> > +                                                 link->channel_layout);
> > +                    av_bprintf(buf,
> > +                            "fmt:%s sr:%d cl:%s tb:%d/%d",
> > +                            av_get_sample_fmt_name(link->format),
> > +                            link->sample_rate, audio_buf,
> > +                            link->time_base.num, link->time_base.den);
> > +                }
> > +                av_bprintf(buf, "\" ];\n");
> > +            }
> > +        }
> > +    }
> > +    av_bprintf(buf, "}\n");
> > +}
> > +
> >  char *avfilter_graph_dump(AVFilterGraph *graph, const char *options)
> >  {
> >      AVBPrint buf;
> >      char *dump = NULL;
> >
> > +    if (options && !strcmp(options, "DOT")) {
> > +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> > +    avfilter_graph2dot_to_buf(&buf, graph);
> > +    av_bprint_finalize(&buf, &dump);
> > +    } else {
> >      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
> >      avfilter_graph_dump_to_buf(&buf, graph);
> >      av_bprint_init(&buf, buf.len + 1, buf.len + 1);
> >      avfilter_graph_dump_to_buf(&buf, graph);
> >      av_bprint_finalize(&buf, &dump);
> > +    }
> > +
> >      return dump;
> >  }
> > --
> > 2.6.4
> >
> > _______________________________________________
> > 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".

On Sat, May 23, 2020 at 02:49:39PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-05-23):
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  doc/ffmpeg.texi         | 15 ++++++++++++
> 
> >  fftools/ffmpeg.h        |  2 ++
> >  fftools/ffmpeg_filter.c | 20 ++++++++++++++++
> >  fftools/ffmpeg_opt.c    | 20 ++++++++++++++++
> >  libavfilter/graphdump.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Changes in the library belong and changes in the command-line tools
> belong in different patches with corresponding commit messages.

OK, will split the patch

> 
> >  5 files changed, 120 insertions(+)
> > 
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index ed437bb..699991f 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -735,6 +735,21 @@ Technical note -- attachments are implemented as codec extradata, so this
> >  option can actually be used to extract extradata from any stream, not just
> >  attachments.
> >  
> > +@item -dump_filtergraph @var{filename} (@emph{global})
> > +Set the output file name of filter graph for dump.
> > +
> > +It is "ASCII" format by default. for Graphviz DOT output format,
> 
> > +you can convert it to png by GraphViz tool:
> 
> The is no CammelCase in the official name of this project. Same below.

Yes, will correct them

> 
> > +@example
> > +dot -Tpng dump_fg_filename -o dump_graph.png
> > +@end example
> > +
> > +@item -dump_filtergraph_format @var{format} (@emph{global})
> > +Set the output format of filter graph for dump. Support format: "DOT", "ASCII".
> > +
> > +DOT is the text file format of the suite GraphViz, ASCII is the text file format
> > +of ASCII style.
> > +
> >  @item -noautorotate
> >  Disable automatically rotating video based on file metadata.
> >  
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 38205a1..55f115b 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -606,6 +606,8 @@ extern AVIOContext *progress_avio;
> >  extern float max_error_rate;
> >  extern char *videotoolbox_pixfmt;
> >  
> > +extern char* dump_fg_filename;
> > +extern char* dump_fg_format;
> >  extern int filter_nbthreads;
> >  extern int filter_complex_nbthreads;
> >  extern int vstats_version;
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 8b5b157..485fc73 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -1106,6 +1106,26 @@ int configure_filtergraph(FilterGraph *fg)
> >      if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
> >          goto fail;
> >  
> > +    if (dump_fg_filename) {
> > +        char *dump = avfilter_graph_dump(fg->graph, dump_fg_format);
> > +        FILE *fg_file = fopen(dump_fg_filename, "w");
> > +
> > +        if (!dump) {
> > +            ret = AVERROR(ENOMEM);
> > +            goto fail;
> > +        }
> 
> > +        if (fg_file) {
> > +            fputs(dump, fg_file);
> > +            fflush(fg_file);
> > +            fclose(fg_file);
> > +        } else {
> > +            ret = AVERROR(EINVAL);
> > +            av_free(dump);
> > +            goto fail;
> > +        }
> 
> Spaghetti code. Check errors where they happen, not in a weird else
> clause.

OK, will fix.

> 
> > +        av_free(dump);
> > +    }
> > +
> >      /* limit the lists of allowed formats to the ones selected, to
> >       * make sure they stay the same if the filtergraph is reconfigured later */
> >      for (i = 0; i < fg->nb_outputs; i++) {
> > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> > index 60bb437..bdd8957 100644
> > --- a/fftools/ffmpeg_opt.c
> > +++ b/fftools/ffmpeg_opt.c
> > @@ -143,6 +143,8 @@ HWDevice *filter_hw_device;
> >  
> >  char *vstats_filename;
> >  char *sdp_filename;
> > +char *dump_fg_filename;
> > +char *dump_fg_format;
> >  
> >  float audio_drift_threshold = 0.1;
> >  float dts_delta_threshold   = 10;
> > @@ -2930,6 +2932,20 @@ static int opt_vstats(void *optctx, const char *opt, const char *arg)
> >      return opt_vstats_file(NULL, opt, filename);
> >  }
> >  
> > +static int opt_fg_filename(void *optctx, const char *opt, const char *arg)
> > +{
> > +    av_free (dump_fg_filename);
> 
> > +    dump_fg_filename = av_strdup (arg);
> 
> Stray space. Same below.

OK, I copy the code from opt_vstats_file(), it has the stray space, I'll fix it.

> 
> > +    return 0;
> > +}
> > +
> > +static int opt_fg_format(void *optctx, const char *opt, const char *arg)
> > +{
> > +    av_free (dump_fg_format);
> > +    dump_fg_format = av_strdup (arg);
> > +    return 0;
> > +}
> > +
> >  static int opt_video_frames(void *optctx, const char *opt, const char *arg)
> >  {
> >      OptionsContext *o = optctx;
> > @@ -3548,6 +3564,10 @@ const OptionDef options[] = {
> >      { "dump_attachment", HAS_ARG | OPT_STRING | OPT_SPEC |
> >                           OPT_EXPERT | OPT_INPUT,                     { .off = OFFSET(dump_attachment) },
> >          "extract an attachment into a file", "filename" },
> > +    { "dump_filtergraph",HAS_ARG | OPT_EXPERT,                       { .func_arg = opt_fg_filename },
> > +        "the output filename of filter graph for dump", "filename" },
> > +    { "dump_filtergraph_format", HAS_ARG | OPT_EXPERT,               { .func_arg = opt_fg_format },
> > +        "the format of filter graph for dump, support DOT or ASCII"},
> >      { "stream_loop", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_INPUT |
> >                          OPT_OFFSET,                                  { .off = OFFSET(loop) }, "set number of times input stream shall be looped", "loop count" },
> >      { "debug_ts",       OPT_BOOL | OPT_EXPERT,                       { &debug_ts },
> > diff --git a/libavfilter/graphdump.c b/libavfilter/graphdump.c
> > index 79ef1a7..b15f498 100644
> > --- a/libavfilter/graphdump.c
> > +++ b/libavfilter/graphdump.c
> > @@ -151,15 +151,78 @@ static void avfilter_graph_dump_to_buf(AVBPrint *buf, AVFilterGraph *graph)
> >      }
> >  }
> >  
> 
> > +static void avfilter_graph2dot_to_buf(AVBPrint *buf, AVFilterGraph *graph)
> 
> This code looks like it comes straight from tools/graph2dot.c. The
> commit message should say it.

Yes, by your comments, I reuse the code, I'll add it into the commit message.

> 
> And code should not be duplicated. If the feature is now in the library,
> tools/graph2dot.c should go away.

OK, if nobody object it, I'll remove it after the patchset are OK.

> 
> > +{
> > +    int i, j;
> > +
> > +    av_bprintf(buf, "digraph G {\n");
> > +    av_bprintf(buf, "node [shape=box]\n");
> > +    av_bprintf(buf, "rankdir=LR\n");
> > +
> > +    for (i = 0; i < graph->nb_filters; i++) {
> > +        char filter_ctx_label[128];
> > +        const AVFilterContext *filter_ctx = graph->filters[i];
> > +
> > +        snprintf(filter_ctx_label, sizeof(filter_ctx_label), "%s\\n(%s)",
> > +                 filter_ctx->name,
> > +                 filter_ctx->filter->name);
> > +
> > +        for (j = 0; j < filter_ctx->nb_outputs; j++) {
> > +            AVFilterLink *link = filter_ctx->outputs[j];
> > +            if (link) {
> > +                char dst_filter_ctx_label[128];
> > +                const AVFilterContext *dst_filter_ctx = link->dst;
> > +
> > +                snprintf(dst_filter_ctx_label, sizeof(dst_filter_ctx_label),
> > +                         "%s\\n(%s)",
> > +                         dst_filter_ctx->name,
> > +                         dst_filter_ctx->filter->name);
> > +
> > +                av_bprintf(buf, "\"%s\" -> \"%s\" [ label= \"inpad:%s -> outpad:%s\\n",
> > +                        filter_ctx_label, dst_filter_ctx_label,
> > +                        avfilter_pad_get_name(link->srcpad, 0),
> > +                        avfilter_pad_get_name(link->dstpad, 0));
> > +
> > +                if (link->type == AVMEDIA_TYPE_VIDEO) {
> > +                    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> > +                    av_bprintf(buf,
> > +                            "fmt:%s w:%d h:%d tb:%d/%d",
> > +                            desc->name,
> > +                            link->w, link->h,
> > +                            link->time_base.num, link->time_base.den);
> > +                } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> > +                    char audio_buf[255];
> > +                    av_get_channel_layout_string(audio_buf, sizeof(audio_buf), -1,
> > +                                                 link->channel_layout);
> > +                    av_bprintf(buf,
> > +                            "fmt:%s sr:%d cl:%s tb:%d/%d",
> > +                            av_get_sample_fmt_name(link->format),
> > +                            link->sample_rate, audio_buf,
> > +                            link->time_base.num, link->time_base.den);
> > +                }
> > +                av_bprintf(buf, "\" ];\n");
> > +            }
> > +        }
> > +    }
> > +    av_bprintf(buf, "}\n");
> > +}
> > +
> >  char *avfilter_graph_dump(AVFilterGraph *graph, const char *options)
> >  {
> >      AVBPrint buf;
> >      char *dump = NULL;
> >  
> 
> > +    if (options && !strcmp(options, "DOT")) {
> 
> It is called "options", not "format", which, for this project, means
> key-value pairs.

I have no idea how to use parse_key_value_pair() and how to support it in command
line with option? so I use it as format directly.

> 
> > +    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> > +    avfilter_graph2dot_to_buf(&buf, graph);
> > +    av_bprint_finalize(&buf, &dump);
> > +    } else {
> >      av_bprint_init(&buf, 0, AV_BPRINT_SIZE_COUNT_ONLY);
> >      avfilter_graph_dump_to_buf(&buf, graph);
> >      av_bprint_init(&buf, buf.len + 1, buf.len + 1);
> >      avfilter_graph_dump_to_buf(&buf, graph);
> >      av_bprint_finalize(&buf, &dump);
> > +    }
> > +
> >      return dump;
> >  }
> 
> Regards,
> 
> -- 
>   Nicolas George

Limin Wang (12020-05-23):
> OK, if nobody object it, I'll remove it after the patchset are OK.

That should be in the same patch.

> I have no idea how to use parse_key_value_pair() and how to support it in command
> line with option? so I use it as format directly.

The answer to something we do not know should be to learn, not to do
without and result in an inferior implementation.

I am very much opposed to turning this option into just the name of the
format, especially now that it starts getting extended.

In this case, transforming the string into an AVDictionary, passing it
around and warning if there are remaining options should be the simplest
choice.

Regards,

On Sat, May 23, 2020 at 04:36:24PM +0200, Nicolas George wrote:
> Limin Wang (12020-05-23):
> > OK, if nobody object it, I'll remove it after the patchset are OK.
> 
> That should be in the same patch.
> 
> > I have no idea how to use parse_key_value_pair() and how to support it in command
> > line with option? so I use it as format directly.
> 
> The answer to something we do not know should be to learn, not to do
> without and result in an inferior implementation.
OK, I'm glad to learn how to use it, I haven't find how to use it in the existing
code. 

> 
> I am very much opposed to turning this option into just the name of the
> format, especially now that it starts getting extended.
> 
> In this case, transforming the string into an AVDictionary, passing it
> around and warning if there are remaining options should be the simplest
> choice.

So I should to use av_find_info_tag to parse the options string?  any sample code
in the existing code for refering?



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

Limin Wang (12020-05-23):
> > In this case, transforming the string into an AVDictionary, passing it
> > around and warning if there are remaining options should be the simplest
> > choice.
> 
> So I should to use av_find_info_tag to parse the options string?  any sample code
> in the existing code for refering?

What?!? Definitely not. Have you read its doc? It is made for URLs, we
do not have any URL here.

Regards,

On Sat, May 23, 2020 at 04:52:47PM +0200, Nicolas George wrote:
> Limin Wang (12020-05-23):
> > > In this case, transforming the string into an AVDictionary, passing it
> > > around and warning if there are remaining options should be the simplest
> > > choice.
> > 
> > So I should to use av_find_info_tag to parse the options string?  any sample code
> > in the existing code for refering?
> 
> What?!? Definitely not. Have you read its doc? It is made for URLs, we
> do not have any URL here.

I'm not clear about the char * option usage yet, below is my draft code, am I
understand for you correctly?


+    int ret;
+    AVDictionary *dict = NULL;
+    AVDictionaryEntry *format = NULL;

+    ret = av_dict_parse_string(&dict, options, "=", ":", 0);
+    if (ret < 0) {
+        av_dict_free(&dict);
+        return NULL;
+    }
+    format = av_dict_get(dict, "fmt", NULL, 0);
+
+    if (format && !strcmp(format->value, "DOT")) {
+    av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED);
+    avfilter_graph2dot_to_buf(&buf, graph);
+    av_bprint_finalize(&buf, &dump);
+    } else {



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