Re: [FFmpeg-devel] [PATCH v6 6/8] fftools/ffmpeg_graphprint: Add options for filtergraph printing

[Stefano Sabatini <stefasab@gmail.com>]

On date Saturday 2025-03-08 20:16:32 +0000, softworkz wrote:
> From: softworkz <softworkz@hotmail.com>
> 
> The key benefits are:
> 
> - Different to other graph printing methods, this is outputting:
>   - all graphs with runtime state
>     (including auto-inserted filters)
>   - each graph with its inputs and outputs
>   - all filters with their in- and output pads
>   - all connections between all input- and output pads
>   - for each connection:
>     - the runtime-negotiated format and media type
>     - the hw context
>     - if video hw context, both: hw pixfmt + sw pixfmt
> - Output can either be printed to stdout or written to specified file
> - Output is machine-readable
> - Use the same output implementation as ffprobe, supporting multiple
>   formats
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  fftools/Makefile            |  11 +
>  fftools/ffmpeg.h            |   3 +
>  fftools/ffmpeg_graphprint.c | 518 ++++++++++++++++++++++++++++++++++++
>  fftools/ffmpeg_graphprint.h |  35 +++
>  fftools/ffmpeg_opt.c        |  12 +
>  5 files changed, 579 insertions(+)
>  create mode 100644 fftools/ffmpeg_graphprint.c
>  create mode 100644 fftools/ffmpeg_graphprint.h
> 
> diff --git a/fftools/Makefile b/fftools/Makefile
> index 664b73b161..03cdbd4b6e 100644
> --- a/fftools/Makefile
> +++ b/fftools/Makefile
> @@ -19,8 +19,19 @@ OBJS-ffmpeg +=                  \
>      fftools/ffmpeg_mux_init.o   \
>      fftools/ffmpeg_opt.o        \
>      fftools/ffmpeg_sched.o      \
> +    fftools/ffmpeg_graphprint.o \
>      fftools/sync_queue.o        \
>      fftools/thread_queue.o      \
> +    fftools/textformat/avtextformat.o \
> +    fftools/textformat/tf_compact.o   \
> +    fftools/textformat/tf_default.o   \
> +    fftools/textformat/tf_flat.o      \
> +    fftools/textformat/tf_ini.o       \
> +    fftools/textformat/tf_json.o      \
> +    fftools/textformat/tf_xml.o       \
> +    fftools/textformat/tw_avio.o      \
> +    fftools/textformat/tw_buffer.o    \
> +    fftools/textformat/tw_stdout.o    \
>  
>  OBJS-ffprobe +=                       \
>      fftools/textformat/avtextformat.o \
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 86a3e10c6b..9880236162 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -715,6 +715,9 @@ extern float max_error_rate;
>  extern char *filter_nbthreads;
>  extern int filter_complex_nbthreads;
>  extern int vstats_version;
> +extern int print_graphs;

> +extern char* print_graphs_file;
> +extern char* print_graphs_format;

style: char *VAR here and below (also make sure to run patcheck in
case it spots other style issues)

>  extern int auto_conversion_filters;
>  
>  extern const AVIOInterruptCB int_cb;
> diff --git a/fftools/ffmpeg_graphprint.c b/fftools/ffmpeg_graphprint.c
> new file mode 100644
> index 0000000000..dd6862679e
> --- /dev/null
> +++ b/fftools/ffmpeg_graphprint.c
> @@ -0,0 +1,518 @@
> +/*

> + * Copyright (c) 2018 - softworkz

2018?

> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * output writers for filtergraph details
> + */
> +
> +#include "config.h"
> +
> +#include <string.h>
> +
> +#include "ffmpeg_graphprint.h"
> +#include "ffmpeg_filter.h"
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/dict.h"
> +#include "libavutil/common.h"
> +#include "libavfilter/avfilter.h"
> +#include "libavutil/buffer.h"
> +#include "libavutil/hwcontext.h"
> +#include "textformat/avtextformat.h"
> +
> +typedef enum {
> +    SECTION_ID_ROOT,
> +    SECTION_ID_PROGRAM_VERSION,
> +    SECTION_ID_FILTERGRAPHS,
> +    SECTION_ID_FILTERGRAPH,
> +    SECTION_ID_INPUTS,
> +    SECTION_ID_INPUT,
> +    SECTION_ID_OUTPUTS,
> +    SECTION_ID_OUTPUT,
> +    SECTION_ID_FILTERS,
> +    SECTION_ID_FILTER,
> +    SECTION_ID_HWDEViCECONTEXT,
> +    SECTION_ID_HWFRAMESCONTEXT,
> +    SECTION_ID_ERROR,
> +    SECTION_ID_LOG,
> +    SECTION_ID_LOGS,
> +} SectionID;
> +
> +static struct AVTextFormatSection sections[] = {

> +    [SECTION_ID_ROOT] =               { SECTION_ID_ROOT, "GraphDescription", AV_TEXTFORMAT_SECTION_FLAG_IS_WRAPPER,

> +                                      { SECTION_ID_ERROR, SECTION_ID_PROGRAM_VERSION, SECTION_ID_FILTERGRAPHS, SECTION_ID_LOGS, -1} },

more readable if you move indent this to the right to explain it is
not on the same level

> +    [SECTION_ID_PROGRAM_VERSION] =    { SECTION_ID_PROGRAM_VERSION, "ProgramVersion", 0, { -1 } },
> +
> +    [SECTION_ID_FILTERGRAPHS] =       { SECTION_ID_FILTERGRAPHS, "Graphs", AV_TEXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_FILTERGRAPH, -1 } },
> +    [SECTION_ID_FILTERGRAPH] =        { SECTION_ID_FILTERGRAPH, "Graph", 0, { SECTION_ID_INPUTS, SECTION_ID_OUTPUTS, SECTION_ID_FILTERS, -1 },  },
> +
> +    [SECTION_ID_INPUTS] =             { SECTION_ID_INPUTS, "Inputs", AV_TEXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_INPUT, SECTION_ID_ERROR, -1 } },
> +    [SECTION_ID_INPUT] =              { SECTION_ID_INPUT, "Input", 0, { SECTION_ID_HWFRAMESCONTEXT, SECTION_ID_ERROR, -1 },  },
> +
> +    [SECTION_ID_OUTPUTS] =            { SECTION_ID_OUTPUTS, "Outputs", AV_TEXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_OUTPUT, SECTION_ID_ERROR, -1 } },
> +    [SECTION_ID_OUTPUT] =             { SECTION_ID_OUTPUT, "Output", 0, { SECTION_ID_HWFRAMESCONTEXT, SECTION_ID_ERROR, -1 },  },
> +
> +    [SECTION_ID_FILTERS] =            { SECTION_ID_FILTERS, "Filters", AV_TEXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_FILTER, SECTION_ID_ERROR, -1 } },
> +    [SECTION_ID_FILTER] =             { SECTION_ID_FILTER, "Filter", 0, { SECTION_ID_HWDEViCECONTEXT, SECTION_ID_ERROR, -1 },  },
> +

> +    [SECTION_ID_HWDEViCECONTEXT] =    { SECTION_ID_HWDEViCECONTEXT, "HwDeviceContext", 0, { SECTION_ID_ERROR, -1 },  },

MIXEDSTyLE is expected?

> +    [SECTION_ID_HWFRAMESCONTEXT] =    { SECTION_ID_HWFRAMESCONTEXT, "HwFramesContext", 0, { SECTION_ID_ERROR, -1 },  },
> +
> +    [SECTION_ID_ERROR] =              { SECTION_ID_ERROR, "Error", 0, { -1 } },

> +    [SECTION_ID_LOGS] =               { SECTION_ID_LOGS, "Log", AV_TEXTFORMAT_SECTION_FLAG_IS_ARRAY, { SECTION_ID_LOG, -1 } },
> +    [SECTION_ID_LOG] =                { SECTION_ID_LOG, "LogEntry", 0, { -1 },  },

Why not Logs/Log? It's OK but I'd keep ID and names in synch to
simplify reading/editing.

> +};
> +
> +/* Text Format API Shortcuts */
> +#define print_int(k, v)         avtext_print_integer(w, k, v)
> +#define print_q(k, v, s)        avtext_print_rational(w, k, v, s)
> +#define print_str(k, v)         avtext_print_string(w, k, v, 0)
> +
> +static void print_hwdevicecontext(AVTextFormatContext *w, const AVHWDeviceContext *hw_device_context)
> +{
> +    avtext_print_section_header(w, NULL, SECTION_ID_HWDEViCECONTEXT);
> +

> +    print_int("HasHwDeviceContext", 1);

Is this even needed? If missing I'd expect type to be none. Or not?

> +    print_str("DeviceType", av_hwdevice_get_type_name(hw_device_context->type));
> +
> +    avtext_print_section_footer(w); // SECTION_ID_HWDEViCECONTEXT
> +}
> +

> +static void print_hwframescontext(AVTextFormatContext *w, const AVHWFramesContext *hw_frames_context)
> +{

> +    const AVPixFmtDescriptor* pixdescHw;
> +    const AVPixFmtDescriptor* pixdescSw;

let's not mix snakes and camels

> +
> +    avtext_print_section_header(w, NULL, SECTION_ID_HWFRAMESCONTEXT);
> +

> +    print_int("HasHwFramesContext", 1);

again, probably not needed since in this case you can deduct the info
from a missing hw/sw pixdesc?

> +    pixdescHw = av_pix_fmt_desc_get(hw_frames_context->format);

> +    if (pixdescHw) {
> +        print_str("HwPixelFormat", pixdescHw->name);
> +        print_str("HwPixelFormatAlias", pixdescHw->alias);
> +    }
> +
> +    pixdescSw = av_pix_fmt_desc_get(hw_frames_context->sw_format);
> +    if (pixdescSw) {
> +        print_str("SwPixelFormat", pixdescSw->name);
> +        print_str("SwPixelFormatAlias", pixdescSw->alias);
> +    }
> +

> +    print_int("Width", hw_frames_context->width);
> +    print_int("Height", hw_frames_context->height);

is this meaningful in case of no context? Or should we rather skip them?

> +
> +    print_hwdevicecontext(w, hw_frames_context->device_ctx);
> +
> +    avtext_print_section_footer(w); // SECTION_ID_HWFRAMESCONTEXT
> +}
> +
> +static void print_link(AVTextFormatContext *w, AVFilterLink *link)
> +{
> +    AVBufferRef *hw_frames_ctx;
> +    char layoutString[64];
> +
> +    switch (link->type) {
> +        case AVMEDIA_TYPE_VIDEO:
> +            print_str("Format",  av_x_if_null(av_get_pix_fmt_name(link->format), "?"));
> +            print_int("Width", link->w);
> +            print_int("Height", link->h);
> +            print_q("SAR", link->sample_aspect_ratio, ':');
> +            print_q("TimeBase", link->time_base, '/');
> +            break;
> +

> +        ////case AVMEDIA_TYPE_SUBTITLE:
> +        ////    print_str("Format",  av_x_if_null(av_get_subtitle_fmt_name(link->format), "?"));
> +        ////    print_int("Width", link->w);
> +        ////    print_int("Height", link->h);
> +        ////    print_q("TimeBase", link->time_base, '/');
> +        ////    break;

I guess this does not exist yet right?

> +
> +        case AVMEDIA_TYPE_AUDIO:
> +            av_channel_layout_describe(&link->ch_layout, layoutString, sizeof(layoutString));
> +            print_str("ChannelString", layoutString);
> +            print_int("Channels", link->ch_layout.nb_channels);

> +            ////print_int("ChannelLayout", link->ch_layout);

development leftover?

> +            print_int("SampleRate", link->sample_rate);
> +            break;
> +    }
> +
> +    hw_frames_ctx = avfilter_link_get_hw_frames_ctx(link);
> +

> +    if (hw_frames_ctx && hw_frames_ctx->buffer) {
> +      print_hwframescontext(w, (AVHWFramesContext *)hw_frames_ctx->data);

weird reindent

> +    }
> +}
> +
> +static void print_filter(AVTextFormatContext *w, const AVFilterContext* filter)
> +{
> +    avtext_print_section_header(w, NULL, SECTION_ID_FILTER);
> +
> +    print_str("Name", filter->name);
> +
> +    if (filter->filter) {

> +        print_str("Name2", filter->filter->name);

something more descriptive such as "class name"? 

> +        print_str("Description", filter->filter->description);
> +    }
> +
> +    if (filter->hw_device_ctx) {

> +        AVHWDeviceContext* decCtx = (AVHWDeviceContext*)filter->hw_device_ctx->data;

again, use snake_case for overall consistency

> +        print_hwdevicecontext(w, decCtx);
> +    }
> +
> +    avtext_print_section_header(w, NULL, SECTION_ID_INPUTS);
> +
> +    for (unsigned i = 0; i < filter->nb_inputs; i++) {
> +        AVFilterLink *link = filter->inputs[i];
> +        avtext_print_section_header(w, NULL, SECTION_ID_INPUT);
> +
> +        print_str("SourceName", link->src->name);
> +        print_str("SourcePadName", avfilter_pad_get_name(link->srcpad, 0));
> +        print_str("DestPadName", avfilter_pad_get_name(link->dstpad, 0));
> +
> +        print_link(w, link);
> +
> +        avtext_print_section_footer(w); // SECTION_ID_INPUT
> +    }
> +
> +    avtext_print_section_footer(w); // SECTION_ID_INPUTS
> +
> +    avtext_print_section_header(w, NULL, SECTION_ID_OUTPUTS);
> +
> +    for (unsigned i = 0; i < filter->nb_outputs; i++) {
> +        AVFilterLink *link = filter->outputs[i];
> +        avtext_print_section_header(w, NULL, SECTION_ID_OUTPUT);
> +

> +        print_str("DestName", link->dst->name);
> +        print_str("DestPadName", avfilter_pad_get_name(link->dstpad, 0));
> +        print_str("SourceName", link->src->name);

possibly unrelated, but I'm a bit surprised by the asymmetry

> +
> +        print_link(w, link);
> +
> +        avtext_print_section_footer(w); // SECTION_ID_OUTPUT
> +    }
> +
> +    avtext_print_section_footer(w); // SECTION_ID_OUTPUTS
> +
> +    avtext_print_section_footer(w); // SECTION_ID_FILTER
> +}
> +
> +static void init_sections(void)
> +{
> +    for (unsigned i = 0; i < FF_ARRAY_ELEMS(sections); i++)
> +        sections[i].show_all_entries = 1;
> +}
> +

> +static void print_filtergraph_single(AVTextFormatContext *w, FilterGraph* fg, AVFilterGraph *graph)

mixed type*var styles

> +{
> +    char layoutString[64];

snake_case, here and below

> +    FilterGraphPriv *fgp = fgp_from_fg(fg);
> +
> +    print_int("GraphIndex", fg->index);
> +    print_str("Description", fgp->graph_desc);
> +
> +    avtext_print_section_header(w, NULL, SECTION_ID_INPUTS);
> +
> +    for (int i = 0; i < fg->nb_inputs; i++) {
> +        InputFilterPriv* ifilter = ifp_from_ifilter(fg->inputs[i]);
> +        enum AVMediaType mediaType = ifilter->type;
> +
> +        avtext_print_section_header(w, NULL, SECTION_ID_INPUT);
> +
> +        print_str("Name1", (char*)ifilter->ifilter.name);
> +
> +        if (ifilter->filter) {

> +            print_str("Name2", ifilter->filter->name);
> +            print_str("Name3", ifilter->filter->filter->name);

ditto, although I'm a bit lost about what the thricely nested bird is

> +            print_str("Description", ifilter->filter->filter->description);
> +        }
> +
> +        print_str("MediaType", av_get_media_type_string(mediaType));
> +        print_int("MediaTypeId", mediaType);
> +
> +        switch (ifilter->type) {
> +        case AVMEDIA_TYPE_VIDEO:
> +        case AVMEDIA_TYPE_SUBTITLE:

> +            print_str("Format",  av_x_if_null(av_get_pix_fmt_name(ifilter->format), "?"));

nit++: drop double space

> +            print_int("Width", ifilter->width);
> +            print_int("Height", ifilter->height);
> +            print_q("SAR", ifilter->sample_aspect_ratio, ':');
> +            break;

> +        case AVMEDIA_TYPE_AUDIO:
> +

weird empty line

> +            av_channel_layout_describe(&ifilter->ch_layout, layoutString, sizeof(layoutString));

> +            print_str("ChannelString", layoutString);
> +            print_int("SampleRate", ifilter->sample_rate);
> +            break;
> +        case AVMEDIA_TYPE_ATTACHMENT:
> +        case AVMEDIA_TYPE_DATA:
> +            break;
> +        }
> +

> +        if (ifilter->hw_frames_ctx)
> +            print_hwframescontext(w, (AVHWFramesContext*)ifilter->hw_frames_ctx->data);
> +        else if (ifilter->filter && ifilter->filter->hw_device_ctx) {
> +            AVHWDeviceContext* devCtx = (AVHWDeviceContext*)ifilter->filter->hw_device_ctx->data;
> +            print_hwdevicecontext(w, devCtx);
> +        }

nit+: style if () { ... } else if () {...} should be slightly more
readable

> +
> +        avtext_print_section_footer(w); // SECTION_ID_INPUT
> +    }
> +
> +    avtext_print_section_footer(w); // SECTION_ID_INPUTS

> +
> +

drop double emtpy line

> +    avtext_print_section_header(w, NULL, SECTION_ID_OUTPUTS);
> +
> +    for (int i = 0; i < fg->nb_outputs; i++) {
> +        OutputFilterPriv *ofilter = ofp_from_ofilter(fg->outputs[i]);
> +        enum AVMediaType mediaType = AVMEDIA_TYPE_UNKNOWN;
> +
> +        avtext_print_section_header(w, NULL, SECTION_ID_OUTPUT);

> +        print_str("Name1", ofilter->name);
> +
> +        if (ofilter->filter) {
> +            print_str("Name2", ofilter->filter->name);
> +            print_str("Name3", ofilter->filter->filter->name);

ditto, having more descriptive names would help user big times

> +            print_str("Description", ofilter->filter->filter->description);
> +
> +            if (ofilter->filter->nb_inputs > 0)
> +                mediaType = ofilter->filter->inputs[0]->type;
> +        }
> +
> +        print_str("MediaType", av_get_media_type_string(mediaType));

> +        print_int("MediaTypeId", mediaType);

yes but... this might be broken if you make assumption based on the
value (might change when bumping major) so should be probably drop

> +
> +        switch (ofilter->ofilter.type) {
> +        case AVMEDIA_TYPE_VIDEO:
> +        case AVMEDIA_TYPE_SUBTITLE:

> +            print_str("Format",  av_x_if_null(av_get_pix_fmt_name(ofilter->format), "?"));

double space

also you can probably use a function/macro to factorized duplicated pattern

> +            print_int("Width", ofilter->width);
> +            print_int("Height", ofilter->height);
> +            break;
> +        case AVMEDIA_TYPE_AUDIO:
> +
> +            av_channel_layout_describe(&ofilter->ch_layout, layoutString, sizeof(layoutString));

> +            print_str("ChannelString", layoutString);

ChannelLayout? We don't print other representations anyway so there is
no ambiguity here

> +            print_int("SampleRate", ofilter->sample_rate);
> +            break;
> +        case AVMEDIA_TYPE_ATTACHMENT:
> +        case AVMEDIA_TYPE_DATA:
> +            break;
> +        }
> +
> +        if (ofilter->filter && ofilter->filter->hw_device_ctx) {

> +            AVHWDeviceContext* devCtx = (AVHWDeviceContext*)ofilter->filter->hw_device_ctx->data;

style

> +            print_hwdevicecontext(w, devCtx);
> +        }
> +
> +        avtext_print_section_footer(w); // SECTION_ID_OUTPUT
> +    }
> +
> +    avtext_print_section_footer(w); // SECTION_ID_OUTPUTS
> +
> +
> +    avtext_print_section_header(w, NULL, SECTION_ID_FILTERS);
> +
> +    if (graph) {
> +        for (unsigned i = 0; i < graph->nb_filters; i++) {
> +            AVFilterContext *filter = graph->filters[i];
> +            avtext_print_section_header(w, NULL, SECTION_ID_FILTER);
> +
> +            print_filter(w, filter);
> +
> +            avtext_print_section_footer(w); // SECTION_ID_FILTER
> +        }
> +    }
> +
> +    avtext_print_section_footer(w); // SECTION_ID_FILTERS
> +}
> +
> +int print_filtergraph(FilterGraph *fg, AVFilterGraph *graph)
> +{
> +    const AVTextFormatter *text_formatter;
> +    AVTextFormatContext *tctx;
> +    AVTextWriterContext *wctx;
> +    char *w_name, *w_args;
> +    int ret;
> +    FilterGraphPriv *fgp = fgp_from_fg(fg);
> +    AVBPrint *target_buf = &fgp->graph_print_buf;
> +
> +    init_sections();
> +
> +    if (target_buf->len)
> +        av_bprint_finalize(target_buf, NULL);
> +
> +    av_bprint_init(target_buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> +
> +    if (!print_graphs_format)
> +        print_graphs_format = av_strdup("default");
> +    if (!print_graphs_format)
> +        return AVERROR(ENOMEM);
> +
> +    w_name = av_strtok(print_graphs_format, "=", &w_args);
> +    if (!w_name) {
> +        av_log(NULL, AV_LOG_ERROR, "No name specified for the filter graph output format\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    text_formatter = avtext_get_formatter_by_name(w_name);

> +    if (text_formatter == NULL) {

if (!text_formatter)
?

for consistencty with the line above

> +        av_log(NULL, AV_LOG_ERROR, "Unknown filter graph  output format with name '%s'\n", w_name);

double space

> +        return AVERROR(EINVAL);
> +    }
> +
> +    ret = avtextwriter_create_buffer(&wctx, target_buf);
> +    if (ret < 0) {

> +        av_log(NULL, AV_LOG_ERROR, "avtextwriter_create_buffer failed. Error code %d\n", ret);

Is this function already logging? If not it probably should, if yes
this is log is probably redundant.

> +        return AVERROR(EINVAL);

why not propagating the error?

> +    }
> +
> +    if ((ret = avtext_context_open(&tctx, text_formatter, wctx, w_args, sections, FF_ARRAY_ELEMS(sections), 0, 0, 0, 0, -1, NULL)) >= 0) {

> +        avtext_print_section_header(tctx, NULL, SECTION_ID_ROOT);
> +        avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPHS);
> +        avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPH);
> +
> +        av_bprint_clear(target_buf);

If I understand this is printing the sections and then discarding the
generated buffer, right? Maybe add a not to explain why this is done
(I don't know...).

> +
> +        print_filtergraph_single(tctx, fg, graph);
> +
> +        avtext_context_close(&tctx);
> +        avtextwriter_context_close(&wctx);
> +    } else
> +        return ret;
> +
> +    return 0;
> +}
> +
> +int print_filtergraphs(FilterGraph **graphs, int nb_graphs, OutputFile **ofiles, int nb_ofiles)
> +{
> +    const AVTextFormatter *text_formatter;
> +    AVTextFormatContext *tctx;
> +    AVTextWriterContext *wctx;
> +    AVBPrint target_buf;
> +    char *buf, *w_name, *w_args;
> +    int ret;
> +
> +    init_sections();
> +
> +    if (!print_graphs_format)
> +        print_graphs_format = av_strdup("default");
> +    if (!print_graphs_format) {
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    w_name = av_strtok(print_graphs_format, "=", &buf);
> +    if (!w_name) {
> +        av_log(NULL, AV_LOG_ERROR, "No name specified for the filter graph output format\n");
> +        return AVERROR(EINVAL);
> +    }
> +    w_args = buf;
> +
> +    text_formatter = avtext_get_formatter_by_name(w_name);
> +    if (text_formatter == NULL) {

> +        av_log(NULL, AV_LOG_ERROR, "Unknown filter graph  output format with name '%s'\n", w_name);

double space

> +        return AVERROR(EINVAL);
> +    }
> +
> +    av_bprint_init(&target_buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> +

> +    ret = avtextwriter_create_buffer(&wctx, &target_buf);
> +    if (ret < 0) {
> +        av_log(NULL, AV_LOG_ERROR, "avtextwriter_create_buffer failed. Error code %d\n", ret);

ditto

> +        return AVERROR(EINVAL);
> +    }
> +
> +    if ((ret = avtext_context_open(&tctx, text_formatter, wctx, w_args, sections, FF_ARRAY_ELEMS(sections), 0, 0, 0, 0, -1, NULL)) >= 0) {
> +        avtext_print_section_header(tctx, NULL, SECTION_ID_ROOT);
> +
> +        avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPHS);
> +
> +        for (int i = 0; i < nb_graphs; i++) {
> +
> +            FilterGraphPriv *fgp = fgp_from_fg(graphs[i]);
> +            AVBPrint *graph_buf = &fgp->graph_print_buf;
> +
> +            if (graph_buf->len > 0) {
> +                avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPH);
> +
> +                av_bprint_append_data(&target_buf, graph_buf->str, graph_buf->len);
> +                av_bprint_finalize(graph_buf, NULL);
> +
> +                avtext_print_section_footer(tctx); // SECTION_ID_FILTERGRAPH
> +            }
> +       }
> +
> +        for (int n = 0; n < nb_ofiles; n++) {
> +            OutputFile *of = ofiles[n];
> +
> +            for (int i = 0; i < of->nb_streams; i++) {
> +                OutputStream *ost = of->streams[i];
> +
> +                if (ost->fg_simple) {
> +                    FilterGraphPriv *fgp = fgp_from_fg(ost->fg_simple);
> +                    AVBPrint *graph_buf = &fgp->graph_print_buf;
> +
> +                    if (graph_buf->len > 0) {
> +                        avtext_print_section_header(tctx, NULL, SECTION_ID_FILTERGRAPH);
> +
> +                        av_bprint_append_data(&target_buf, graph_buf->str, graph_buf->len);
> +                        av_bprint_finalize(graph_buf, NULL);
> +
> +                        avtext_print_section_footer(tctx); // SECTION_ID_FILTERGRAPH
> +                    }
> +                }
> +            }
> +        }
> +
> +        avtext_print_section_footer(tctx); // SECTION_ID_FILTERGRAPHS
> +        avtext_print_section_footer(tctx); // SECTION_ID_ROOT
> +
> +        if (print_graphs_file) {
> +            AVIOContext *avio = NULL;
> +
> +            ret = avio_open2(&avio, print_graphs_file, AVIO_FLAG_WRITE, NULL, NULL);
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "Failed to open graph output file, \"%s\": %s\n",
> +                       print_graphs_file, av_err2str(ret));
> +                return ret;
> +            }
> +
> +            avio_write(avio, (const unsigned char*)target_buf.str, FFMIN(target_buf.len, target_buf.size - 1));
> +            avio_flush(avio);
> +
> +            if ((ret = avio_closep(&avio)) < 0)
> +                av_log(NULL, AV_LOG_ERROR, "Error closing graph output file, loss of information possible: %s\n", av_err2str(ret));
> +        }
> +
> +        if (print_graphs) {
> +            printf("%s", target_buf.str);

> +            av_log(NULL, AV_LOG_INFO, "%s    %c", target_buf.str, '\n');

why it not hardcoding the newline in the message?

> +        }
> +
> +        avtext_context_close(&tctx);
> +        avtextwriter_context_close(&wctx);
> +    }
> +
> +    return 0;
> +}
> diff --git a/fftools/ffmpeg_graphprint.h b/fftools/ffmpeg_graphprint.h
> new file mode 100644
> index 0000000000..e95a0773ba
> --- /dev/null
> +++ b/fftools/ffmpeg_graphprint.h
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (c) 2018 - softworkz
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef FFTOOLS_FFMPEG_GRAPHPRINT_H
> +#define FFTOOLS_FFMPEG_GRAPHPRINT_H
> +
> +#include <stdint.h>
> +
> +#include "config.h"
> +#include "ffmpeg.h"
> +#include "libavutil/avutil.h"
> +#include "libavutil/bprint.h"
> +#include "textformat/avtextformat.h"
> +
> +int print_filtergraphs(FilterGraph **graphs, int nb_graphs, OutputFile **output_files, int nb_output_files);
> +int print_filtergraph(FilterGraph *fg, AVFilterGraph *graph);
> +
> +#endif /* FFTOOLS_FFMPEG_GRAPHPRINT_H */
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 27a9fc9e42..f63d253f53 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -75,6 +75,9 @@ float max_error_rate  = 2.0/3;
>  char *filter_nbthreads;
>  int filter_complex_nbthreads = 0;
>  int vstats_version = 2;
> +int print_graphs = 0;

> +char* print_graphs_file = NULL;
> +char* print_graphs_format = NULL;

char *print_graphs_xxx = NULL;

>  int auto_conversion_filters = 1;
>  int64_t stats_period = 500000;
>  
> @@ -1733,6 +1736,15 @@ const OptionDef options[] = {
>          { .func_arg = opt_filter_complex_script },
>          "deprecated, use -/filter_complex instead", "filename" },
>  #endif

> +    { "print_graphs",   OPT_TYPE_BOOL, 0,
> +        { &print_graphs },
> +        "prints filtergraph details to stderr" },

prints -> print

why to stderr not stdout? that's what's generally exepected on unix*

> +    { "print_graphs_file", OPT_TYPE_STRING, 0,
> +        { &print_graphs_file },
> +        "writes graph details to a file", "filename" },

write graph ...

> +    { "print_graphs_format", OPT_TYPE_STRING, 0,
> +        { &print_graphs_format },

> +      "set the output printing format (available formats are: default, compact, csv, flat, ini, json, xml)", "format" },

non blocking but it would be good to avoid the hardcode in a case a
new format is added (or we'll skip update)

[...]

About the output style, this is using FooBar style against foo_bar
employed by ffprobe itself. I'm not against this, but I wonder if this
was considered and if using a more consistent style across tools is
helpful.

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