From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id A328E4E395 for ; Mon, 10 Mar 2025 23:03:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 011BF68E0A3; Tue, 11 Mar 2025 01:03:20 +0200 (EET) Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C96E868DECD for ; Tue, 11 Mar 2025 01:03:13 +0200 (EET) Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-4394036c0efso28057805e9.2 for ; Mon, 10 Mar 2025 16:03:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741647793; x=1742252593; darn=ffmpeg.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=oXdy0Y+pHQ88SQfu6yDXVbNVg7OX5P1iEvH8TeKjz5U=; b=l9PzkOhqZ4TL2pjnS03CJNrGa9H/aG5RAm/s2dEaEhR3jgWaVMHTE8n5Yz52v+eeBT Fv5dtFbjRjFtFz4zDDF0skDUHW19QNUEODsZM16WRsKhmhTmHlgp8qUXaC7Gy556RgSs M/J/YNb7PwvP06S1TJS93e5srwPKWltwT/w/XumHo5d7OLqDBSdkX3NJ9d8cUu1qvVhD HLUFQGZojd6gi87ZUisljSVuIlIJd+3Z3ti2aD1P0dUM06uGgMeFOsc62piGOmcMslC6 YWm2kcVDYYCr1m5Tx1KQze7ZSybnJubmnNMCujHOwh2d8AmDkCdGVv3bQo2eK7nM3YGv 8ohA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741647793; x=1742252593; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=oXdy0Y+pHQ88SQfu6yDXVbNVg7OX5P1iEvH8TeKjz5U=; b=L9PZUEek59w/RWXj9hTosNYeiLzRNsh0HLbJbw4Bk7w0h4Na1z0nGb82KRhe36InRa oAxmrzyi1SreKq0NigTZtZNnuvD/x9WXHTAyJYaw/J305Ac0oyD6cytOslCP+quNh6aQ ZeIiSh/eNO8OeUYuZWh5GZ3zbqJjwNf24mGu2+Ut8Rpn4cvbDQ1BPjQkOe6aXnbHV5NT akfFLBWOpMBfsg5ZbNwjJWwk6Vy/rePJ1ktWe39+RbOgJI7x4hVOs2cUHK2hsg6XvuMC KVX7fG9RiC+9CqqoMwgxwjLygfxUq2btYh8Co3+2kwPgwsoMz7mL3nYTlWOMA32A2F3o fNmQ== X-Gm-Message-State: AOJu0YygI0du0AQNWn2ui5YCYJ2MGjldcwqAkGXDW4pOs6px/A+zyzmY PTCwawBPP71KeiAk63Pzs+QJ30I+0V9jnVpDBcDVbxEeCbWqtR31pcR99Q== X-Gm-Gg: ASbGncsNzAfb4R981NselHSTXyhHxuPbKwKQ2Jy2ts2eBoSBkwfVCuoQLrs+pKE/1os dAOxQgwwIvf4dM4oKoW6eJPkGLJ1vNaJj3niL0StGxMApNeV6ysC5rt/zxQ5DJIZoSz2Lruc4x4 GBGGikIF3UhFF8XBDlA0RJvhSozUB6g5VEHc76s/l/zmTv/B3vXTZx9OCFsay7e9ngiEzlRI/bP rXGN0NPBonQ5pLnybD/ENGPTtRAsjIu3uvvsEcqeGAa3dqiXaOyYtGzlJmTN+ALUl/cCed9ylka mzssebA9Qmz2tosV9DEm5HCNv7gBJX2XZUMfEceG+3T0WD/thRq7DE9fvje2rILMS5xLAgVCtpU O/KePXGIQ X-Google-Smtp-Source: AGHT+IHyPpMKxmWX/DiS6v2GL+dFldCFUHYErwvcG9Zez68vYleB9VI5k/6yrN4N0vqiodgxUaynDg== X-Received: by 2002:a05:600c:4316:b0:43d:35b:9a74 with SMTP id 5b1f17b1804b1-43d035b9d3fmr6619205e9.6.1741647792333; Mon, 10 Mar 2025 16:03:12 -0700 (PDT) Received: from mariano (dynamic-adsl-84-220-189-10.clienti.tiscali.it. [84.220.189.10]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43cfa8aa700sm44319425e9.17.2025.03.10.16.03.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Mar 2025 16:03:11 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id B4F8EBFCE8; Tue, 11 Mar 2025 00:03:09 +0100 (CET) Date: Tue, 11 Mar 2025 00:03:09 +0100 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: Mail-Followup-To: FFmpeg development discussions and patches , softworkz References: <8ae08a700697d4c6c350939896c6a75405057883.1741464994.git.ffmpegagent@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8ae08a700697d4c6c350939896c6a75405057883.1741464994.git.ffmpegagent@gmail.com> User-Agent: Mutt/2.1.4 (2021-12-11) Subject: Re: [FFmpeg-devel] [PATCH v6 6/8] fftools/ffmpeg_graphprint: Add options for filtergraph printing X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: softworkz Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On date Saturday 2025-03-08 20:16:32 +0000, softworkz wrote: > From: softworkz > > 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 > --- > 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 > + > +#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 > + > +#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".