[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