[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_filter: make FilterGraphPriv private again
softworkz .
softworkz at hotmail.com
Thu May 29 06:16:28 EEST 2025
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> James Almer
> Sent: Donnerstag, 29. Mai 2025 05:07
> To: ffmpeg-devel at ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_filter: make
> FilterGraphPriv private again
>
> As the name implies, it's a struct meant to be internal and private to
> the
> filter handling code. If a field is required in other modules, then it
> can
> be moved to the public facing struct, which is done in this commit.
>
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> fftools/ffmpeg.h | 4 +++
> fftools/ffmpeg_filter.c | 51 ++++++++++++++++++++++++++++++++-----
> -
> fftools/ffmpeg_filter.h | 42 -------------------------------
> fftools/graph/graphprint.c | 16 +++++-------
> 4 files changed, 53 insertions(+), 60 deletions(-)
>
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 7fbf0ad532..641582ae63 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -39,6 +39,7 @@
> #include "libavfilter/avfilter.h"
>
> #include "libavutil/avutil.h"
> +#include "libavutil/bprint.h"
> #include "libavutil/dict.h"
> #include "libavutil/eval.h"
> #include "libavutil/fifo.h"
> @@ -381,6 +382,9 @@ typedef struct FilterGraph {
> int nb_inputs;
> OutputFilter **outputs;
> int nb_outputs;
> +
> + const char *graph_desc;
> + struct AVBPrint graph_print_buf;
> } FilterGraph;
>
> enum DecoderFlags {
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index b774606562..464e17ca7c 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -44,6 +44,42 @@
> // FIXME private header, used for mid_pred()
> #include "libavcodec/mathops.h"
>
> +typedef struct FilterGraphPriv {
> + FilterGraph fg;
> +
> + // name used for logging
> + char log_name[32];
> +
> + int is_simple;
> + // true when the filtergraph contains only meta filters
> + // that do not modify the frame data
> + int is_meta;
> + // source filters are present in the graph
> + int have_sources;
> + int disable_conversions;
> +
> + unsigned nb_outputs_done;
> +
> + int nb_threads;
> +
> + // frame for temporarily holding output from the filtergraph
> + AVFrame *frame;
> + // frame for sending output to the encoder
> + AVFrame *frame_enc;
> +
> + Scheduler *sch;
> + unsigned sch_idx;
> +} FilterGraphPriv;
> +
> +static FilterGraphPriv *fgp_from_fg(FilterGraph *fg)
> +{
> + return (FilterGraphPriv*)fg;
> +}
> +
> +static const FilterGraphPriv *cfgp_from_cfg(const FilterGraph *fg)
> +{
> + return (const FilterGraphPriv*)fg;
> +}
>
> // data that is local to the filter thread and not visible outside of
> it
> typedef struct FilterGraphThread {
> @@ -856,7 +892,7 @@ void fg_free(FilterGraph **pfg)
> av_freep(&fg->outputs[j]);
> }
> av_freep(&fg->outputs);
> - av_freep(&fgp->graph_desc);
> + av_freep(&fg->graph_desc);
>
> av_frame_free(&fgp->frame);
> av_frame_free(&fgp->frame_enc);
> @@ -909,7 +945,7 @@ int fg_create(FilterGraph **pfg, char *graph_desc,
> Scheduler *sch)
> }
>
> fg->class = &fg_class;
> - fgp->graph_desc = graph_desc;
> + fg->graph_desc = graph_desc;
> fgp->disable_conversions = !auto_conversion_filters;
> fgp->nb_threads = -1;
> fgp->sch = sch;
> @@ -928,7 +964,7 @@ int fg_create(FilterGraph **pfg, char *graph_desc,
> Scheduler *sch)
> return AVERROR(ENOMEM);;
> graph->nb_threads = 1;
>
> - ret = graph_parse(fg, graph, fgp->graph_desc, &inputs, &outputs,
> + ret = graph_parse(fg, graph, fg->graph_desc, &inputs, &outputs,
> hw_device_for_filter());
> if (ret < 0)
> goto fail;
> @@ -1070,7 +1106,6 @@ int fg_create_simple(FilterGraph **pfg,
>
> static int fg_complex_bind_input(FilterGraph *fg, InputFilter
> *ifilter)
> {
> - FilterGraphPriv *fgp = fgp_from_fg(fg);
> InputFilterPriv *ifp = ifp_from_ifilter(ifilter);
> InputStream *ist = NULL;
> enum AVMediaType type = ifp->type;
> @@ -1086,7 +1121,7 @@ static int fg_complex_bind_input(FilterGraph
> *fg, InputFilter *ifilter)
> dec_idx = strtol(ifp->linklabel + 4, &p, 0);
> if (dec_idx < 0 || dec_idx >= nb_decoders) {
> av_log(fg, AV_LOG_ERROR, "Invalid decoder index %d in
> filtergraph description %s\n",
> - dec_idx, fgp->graph_desc);
> + dec_idx, fg->graph_desc);
> return AVERROR(EINVAL);
> }
>
> @@ -1137,7 +1172,7 @@ static int fg_complex_bind_input(FilterGraph
> *fg, InputFilter *ifilter)
> file_idx = strtol(ifp->linklabel, &p, 0);
> if (file_idx < 0 || file_idx >= nb_input_files) {
> av_log(fg, AV_LOG_FATAL, "Invalid file index %d in
> filtergraph description %s.\n",
> - file_idx, fgp->graph_desc);
> + file_idx, fg->graph_desc);
> return AVERROR(EINVAL);
> }
> s = input_files[file_idx]->ctx;
> @@ -1171,7 +1206,7 @@ static int fg_complex_bind_input(FilterGraph
> *fg, InputFilter *ifilter)
> stream_specifier_uninit(&ss);
> if (!st) {
> av_log(fg, AV_LOG_FATAL, "Stream specifier '%s' in
> filtergraph description %s "
> - "matches no streams.\n", p, fgp->graph_desc);
> + "matches no streams.\n", p, fg->graph_desc);
> return AVERROR(EINVAL);
> }
> ist = input_files[file_idx]->streams[st->index];
> @@ -1733,7 +1768,7 @@ static int configure_filtergraph(FilterGraph
> *fg, FilterGraphThread *fgt)
> AVFilterInOut *inputs, *outputs, *cur;
> int ret = AVERROR_BUG, i, simple = filtergraph_is_simple(fg);
> int have_input_eof = 0;
> - const char *graph_desc = fgp->graph_desc;
> + const char *graph_desc = fg->graph_desc;
>
> cleanup_filtergraph(fg, fgt);
> fgt->graph = avfilter_graph_alloc();
> diff --git a/fftools/ffmpeg_filter.h b/fftools/ffmpeg_filter.h
> index 94b94beece..bf690bdc91 100644
> --- a/fftools/ffmpeg_filter.h
> +++ b/fftools/ffmpeg_filter.h
> @@ -37,48 +37,6 @@
> #include "libavutil/channel_layout.h"
> #include "libavutil/downmix_info.h"
>
> -typedef struct FilterGraphPriv {
> - FilterGraph fg;
> -
> - // name used for logging
> - char log_name[32];
> -
> - int is_simple;
> - // true when the filtergraph contains only meta filters
> - // that do not modify the frame data
> - int is_meta;
> - // source filters are present in the graph
> - int have_sources;
> - int disable_conversions;
> -
> - unsigned nb_outputs_done;
> -
> - const char *graph_desc;
> -
> - int nb_threads;
> -
> - // frame for temporarily holding output from the filtergraph
> - AVFrame *frame;
> - // frame for sending output to the encoder
> - AVFrame *frame_enc;
> -
> - Scheduler *sch;
> - unsigned sch_idx;
> -
> - AVBPrint graph_print_buf;
> -
> -} FilterGraphPriv;
> -
> -static inline FilterGraphPriv *fgp_from_fg(FilterGraph *fg)
> -{
> - return (FilterGraphPriv*)fg;
> -}
> -
> -static inline const FilterGraphPriv *cfgp_from_cfg(const FilterGraph
> *fg)
> -{
> - return (const FilterGraphPriv*)fg;
> -}
> -
> typedef struct InputFilterPriv {
> InputFilter ifilter;
>
> diff --git a/fftools/graph/graphprint.c b/fftools/graph/graphprint.c
> index 852a8f6c0c..e55c8d7507 100644
> --- a/fftools/graph/graphprint.c
> +++ b/fftools/graph/graphprint.c
> @@ -479,14 +479,13 @@ static void init_sections(void)
> static void print_filtergraph_single(GraphPrintContext *gpc,
> FilterGraph *fg, AVFilterGraph *graph)
> {
> AVTextFormatContext *tfc = gpc->tfc;
> - FilterGraphPriv *fgp = fgp_from_fg(fg);
> AVDictionary *input_map = NULL;
> AVDictionary *output_map = NULL;
>
> print_int("graph_index", fg->index);
> print_fmt("name", "Graph %d.%d", gpc->id_prefix_num, fg->index);
> print_fmt("id", "Graph_%d_%d", gpc->id_prefix_num, fg->index);
> - print_str("description", fgp->graph_desc);
> + print_str("description", fg->graph_desc);
>
> print_section_header_id(gpc, SECTION_ID_GRAPH_INPUTS,
> "Input_File", 0);
>
> @@ -557,7 +556,7 @@ static void
> print_filtergraph_single(GraphPrintContext *gpc, FilterGraph *fg, AV
>
> if (gpc->is_diagram) {
> print_fmt("name", "Graph %d.%d", gpc->id_prefix_num, fg-
> >index);
> - print_str("description", fgp->graph_desc);
> + print_str("description", fg->graph_desc);
> print_str("id", sec_ctx.context_id);
> }
>
> @@ -967,11 +966,10 @@ int print_filtergraph(FilterGraph *fg,
> AVFilterGraph *graph)
> {
> GraphPrintContext *gpc = NULL;
> AVTextFormatContext *tfc;
> - FilterGraphPriv *fgp = fgp_from_fg(fg);
> - AVBPrint *target_buf = &fgp->graph_print_buf;
> + AVBPrint *target_buf = &fg->graph_print_buf;
> int ret;
>
> - if (!fg || !fgp) {
> + if (!fg) {
> av_log(NULL, AV_LOG_ERROR, "Invalid filter graph
> provided\n");
> return AVERROR(EINVAL);
> }
> @@ -1035,8 +1033,7 @@ static int print_filtergraphs_priv(FilterGraph
> **graphs, int nb_graphs, InputFil
> avtext_print_section_header(tfc, 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;
> + AVBPrint *graph_buf = &graphs[i]->graph_print_buf;
>
> if (graph_buf->len > 0) {
> avtext_print_section_header(tfc, NULL,
> SECTION_ID_FILTERGRAPH);
> @@ -1053,8 +1050,7 @@ static int print_filtergraphs_priv(FilterGraph
> **graphs, int nb_graphs, InputFil
> OutputStream *ost = of->streams[i];
>
> if (ost->fg_simple) {
> - FilterGraphPriv *fgp = fgp_from_fg(ost->fg_simple);
> - AVBPrint *graph_buf = &fgp->graph_print_buf;
> + AVBPrint *graph_buf = &ost->fg_simple-
> >graph_print_buf;
>
> if (graph_buf->len > 0) {
> avtext_print_section_header(tfc, NULL,
> SECTION_ID_FILTERGRAPH);
> --
> 2.49.0
>
> _______________________________________________
Hi James,
thanks for the patches. Generally, I'm totally fine with this way.
It's just that I had expected objections when moving the member
fields around.
I'll take a closer look at the weekend.
Best regards
sw
More information about the ffmpeg-devel
mailing list