[FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: make InputFilterPriv and OutputFilterPriv private again

[softworkz .]

Hello James,

> -----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 2/2] fftools/ffmpeg_filter: make
> InputFilterPriv and OutputFilterPriv private again
> 
> As the names imply, they are structs 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 structs, which is done in this commit.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---

[..]

>  }
> @@ -535,10 +669,10 @@ static int ifilter_bind_ist(InputFilter *ifilter,
> InputStream *ist,
>      av_assert0(!ifp->bound);
>      ifp->bound = 1;
> 
> -    if (ifp->type != ist->par->codec_type &&
> -        !(ifp->type == AVMEDIA_TYPE_VIDEO && ist->par->codec_type ==
> AVMEDIA_TYPE_SUBTITLE)) {
> +    if (ifilter->type != ist->par->codec_type &&
> +        !(ifilter->type == AVMEDIA_TYPE_VIDEO && ist->par->codec_type ==
> AVMEDIA_TYPE_SUBTITLE)) {
>          av_log(fgp, AV_LOG_ERROR, "Tried to connect %s stream to %s
> filtergraph input\n",
> -               av_get_media_type_string(ist->par->codec_type),
> av_get_media_type_string(ifp->type));
> +               av_get_media_type_string(ist->par->codec_type),
> av_get_media_type_string(ifilter->type));
>          return AVERROR(EINVAL);
>      }
> 
> @@ -553,8 +687,12 @@ static int ifilter_bind_ist(InputFilter *ifilter,
> InputStream *ist,
>      if (ret < 0)
>          return ret;
> 
> +    ifilter->input_name = av_strdup(ifp->opts.name);
> +    if (!ifilter->input_name)
> +        return AVERROR(EINVAL);

Shouldn't it be ENOMEM?
opts.name hasn't been checked for NULL before, not sure if it's meant to
check for that as well - the return would be null in either case (which 
you know of course) - so I'm just asking.

(not marking the other 3 cases below)

> +
>      ret = sch_connect(fgp->sch,
> -                      src, SCH_FILTER_IN(fgp->sch_idx, ifp->index));
> +                      src, SCH_FILTER_IN(fgp->sch_idx, ifilter->index));
>      if (ret < 0)
>          return ret;
> 

[..]

> --- a/fftools/graph/graphprint.c
> +++ b/fftools/graph/graphprint.c
> @@ -28,7 +28,7 @@
> 
>  #include "graphprint.h"
> 
> -#include "fftools/ffmpeg_filter.h"
> +#include "fftools/ffmpeg.h"
>  #include "fftools/ffmpeg_mux.h"
> 
>  #include "libavutil/avassert.h"
> @@ -490,7 +490,7 @@ static void print_filtergraph_single(GraphPrintContext
> *gpc, FilterGraph *fg, AV
>      print_section_header_id(gpc, SECTION_ID_GRAPH_INPUTS, "Input_File", 0);
> 
>      for (int i = 0; i < fg->nb_inputs; i++) {
> -        InputFilterPriv *ifilter = ifp_from_ifilter(fg->inputs[i]);
> +        InputFilter *ifilter = fg->inputs[i];
>          enum AVMediaType media_type = ifilter->type;
> 
>          avtext_print_section_header(tfc, NULL, SECTION_ID_GRAPH_INPUT);
> @@ -507,8 +507,8 @@ static void print_filtergraph_single(GraphPrintContext
> *gpc, FilterGraph *fg, AV
> 
>          if (ifilter->linklabel && ifilter->filter)
>              av_dict_set(&input_map, ifilter->filter->name, (const char
> *)ifilter->linklabel, 0);
> -        else if (ifilter->opts.name && ifilter->filter)
> -            av_dict_set(&input_map, ifilter->filter->name, (const char
> *)ifilter->opts.name, 0);
> +        else if (ifilter->input_name && ifilter->filter)
> +            av_dict_set(&input_map, ifilter->filter->name, (const char
> *)ifilter->input_name, 0);
> 
>          print_str("media_type", av_get_media_type_string(media_type));
> 
> @@ -520,13 +520,13 @@ static void print_filtergraph_single(GraphPrintContext
> *gpc, FilterGraph *fg, AV
>      print_section_header_id(gpc, SECTION_ID_GRAPH_OUTPUTS, "Output_File", 0);
> 
>      for (int i = 0; i < fg->nb_outputs; i++) {
> -        OutputFilterPriv *ofilter = ofp_from_ofilter(fg->outputs[i]);
> +        OutputFilter *ofilter = fg->outputs[i];
> 
>          avtext_print_section_header(tfc, NULL, SECTION_ID_GRAPH_OUTPUT);
> 
>          print_int("output_index", ofilter->index);
> 
> -        print_str("name", ofilter->name);
> +        print_str("name", ofilter->output_name);
> 
>          if (fg->outputs[i]->linklabel)
>              print_str("link_label", (const char*)fg->outputs[i]->linklabel);

7 lines further down, there's another change needed:


        if (ofilter->output_name && ofilter->filter)
            av_dict_set(&output_map, ofilter->filter->name, ofilter->output_name, 0);


This is what broke the graph connections on the output side.

Thanks
sw