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

Limin Wang lance.lmwang at gmail.com
Sat May 23 17:26:12 EEST 2020


On Sat, May 23, 2020 at 02:49:39PM +0200, Nicolas George wrote:
> lance.lmwang at gmail.com (12020-05-23):
> > From: Limin Wang <lance.lmwang at gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang at 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.
> >  
> > + at 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

> 
> > + at example
> > +dot -Tpng dump_fg_filename -o dump_graph.png
> > + at end example
> > +
> > + at 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



-- 
Thanks,
Limin Wang


More information about the ffmpeg-devel mailing list