diff mbox series

[FFmpeg-devel,v5,2/3] avdevice/lavfi: support the dumpgraph with options

Message ID 1590280970-16256-2-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v5,1/3] avfilter/graphdump: support for the graph2dot function | expand

Checks

Context Check Description
andriy/default pending
andriy/make fail Make failed

Commit Message

Lance Wang May 24, 2020, 12:42 a.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 doc/indevs.texi     | 15 +++++++++++++--
 libavdevice/lavfi.c |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

Comments

Marton Balint May 24, 2020, 7:47 a.m. UTC | #1
On Sun, 24 May 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> doc/indevs.texi     | 15 +++++++++++++--
> libavdevice/lavfi.c |  2 +-
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 6f5afaf..7ec7062 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -973,8 +973,13 @@ Set the filename of the filtergraph to be read and sent to the other
> filters. Syntax of the filtergraph is the same as the one specified by
> the option @var{graph}.
> 
> -@item dumpgraph
> -Dump graph to stderr.
> +@item dumpgraph @var{options}
> +Dump graph to stderr with more options
> +
> +options is a ':'-separated list of @var{key=value} pairs.
> +
> +Set the graph with graphviz DOT format by @var{fmt=dot|DOT},
> +set the filename of filtergraph to output by @var{filename=path}.
> 
> @end table
> 
> @@ -988,6 +993,12 @@ ffplay -f lavfi -graph "color=c=pink [out0]" dummy
> @end example
> 
> @item
> +dump the filter graph with graphviz DOT output format to ./test.tmp
> +@example
> +ffplay -dumpgraph fmt=dot:filename=./test.tmp -f lavfi color=c=pink
> +@end example
> +
> +@item
> As the previous example, but use filename for specifying the graph
> description, and omit the "out0" label:
> @example
> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> index c949ff7..fa75fde 100644
> --- a/libavdevice/lavfi.c
> +++ b/libavdevice/lavfi.c
> @@ -493,7 +493,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
> static const AVOption options[] = {
>     { "graph",     "set libavfilter graph", OFFSET(graph_str),  AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
>     { "graph_file","set libavfilter graph filename", OFFSET(graph_filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC},
> -    { "dumpgraph", "dump graph to stderr",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> +    { "dumpgraph", "dump graph to stderr with more options",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },

There are examples in the docs which use -dumpgraph 1, but if -dumpgraph 
is parsed as a dictionary, then that will not work anymore.

Maybe it is cleaner to specify the options in a separate option, 
e.g. -dumpgraph_opts, and change the type of -dumpgraph to BOOL? After all 
you should be able to dumpgraph with no options specified, right?

Regards,
Marton
Lance Wang May 24, 2020, 10:56 a.m. UTC | #2
On Sun, May 24, 2020 at 09:47:12AM +0200, Marton Balint wrote:
> 
> 
> On Sun, 24 May 2020, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > doc/indevs.texi     | 15 +++++++++++++--
> > libavdevice/lavfi.c |  2 +-
> > 2 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/doc/indevs.texi b/doc/indevs.texi
> > index 6f5afaf..7ec7062 100644
> > --- a/doc/indevs.texi
> > +++ b/doc/indevs.texi
> > @@ -973,8 +973,13 @@ Set the filename of the filtergraph to be read and sent to the other
> > filters. Syntax of the filtergraph is the same as the one specified by
> > the option @var{graph}.
> > 
> > -@item dumpgraph
> > -Dump graph to stderr.
> > +@item dumpgraph @var{options}
> > +Dump graph to stderr with more options
> > +
> > +options is a ':'-separated list of @var{key=value} pairs.
> > +
> > +Set the graph with graphviz DOT format by @var{fmt=dot|DOT},
> > +set the filename of filtergraph to output by @var{filename=path}.
> > 
> > @end table
> > 
> > @@ -988,6 +993,12 @@ ffplay -f lavfi -graph "color=c=pink [out0]" dummy
> > @end example
> > 
> > @item
> > +dump the filter graph with graphviz DOT output format to ./test.tmp
> > +@example
> > +ffplay -dumpgraph fmt=dot:filename=./test.tmp -f lavfi color=c=pink
> > +@end example
> > +
> > +@item
> > As the previous example, but use filename for specifying the graph
> > description, and omit the "out0" label:
> > @example
> > diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> > index c949ff7..fa75fde 100644
> > --- a/libavdevice/lavfi.c
> > +++ b/libavdevice/lavfi.c
> > @@ -493,7 +493,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
> > static const AVOption options[] = {
> >     { "graph",     "set libavfilter graph", OFFSET(graph_str),  AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> >     { "graph_file","set libavfilter graph filename", OFFSET(graph_filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC},
> > -    { "dumpgraph", "dump graph to stderr",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> > +    { "dumpgraph", "dump graph to stderr with more options",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
> 
> There are examples in the docs which use -dumpgraph 1, but if -dumpgraph is
> parsed as a dictionary, then that will not work anymore.

Yes, I'll return error if not dictionary format.

> 
> Maybe it is cleaner to specify the options in a separate option, e.g.
> -dumpgraph_opts, and change the type of -dumpgraph to BOOL? After all you
> should be able to dumpgraph with no options specified, right?

Good idea, I'm fine with it, it'll not break the old CLI. I'll update it if no
other comments.


> 
> Regards,
> Marton
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 6f5afaf..7ec7062 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -973,8 +973,13 @@  Set the filename of the filtergraph to be read and sent to the other
 filters. Syntax of the filtergraph is the same as the one specified by
 the option @var{graph}.
 
-@item dumpgraph
-Dump graph to stderr.
+@item dumpgraph @var{options}
+Dump graph to stderr with more options
+
+options is a ':'-separated list of @var{key=value} pairs.
+
+Set the graph with graphviz DOT format by @var{fmt=dot|DOT},
+set the filename of filtergraph to output by @var{filename=path}.
 
 @end table
 
@@ -988,6 +993,12 @@  ffplay -f lavfi -graph "color=c=pink [out0]" dummy
 @end example
 
 @item
+dump the filter graph with graphviz DOT output format to ./test.tmp
+@example
+ffplay -dumpgraph fmt=dot:filename=./test.tmp -f lavfi color=c=pink
+@end example
+
+@item
 As the previous example, but use filename for specifying the graph
 description, and omit the "out0" label:
 @example
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index c949ff7..fa75fde 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -493,7 +493,7 @@  static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
 static const AVOption options[] = {
     { "graph",     "set libavfilter graph", OFFSET(graph_str),  AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
     { "graph_file","set libavfilter graph filename", OFFSET(graph_filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC},
-    { "dumpgraph", "dump graph to stderr",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
+    { "dumpgraph", "dump graph to stderr with more options",  OFFSET(dump_graph), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
     { NULL },
 };