[FFmpeg-devel] [PATCH 4/6] avfilter/avfilter: Allow to free non-static pads generically

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Sat Aug 21 10:33:53 EEST 2021


Nicolas George:
> Andreas Rheinhardt (12021-08-17):
>> Well, this has the problem that it adds code in many places, whereas
>> static flags for the filter just need code in two places (with zero
>> marginal cost for making another filter use this feature, whereas a flag
>> to ff_append_... has a nonzero cost even to users not making use of this
>> feature). I also like that it uses a static flag for what is essentially
>> a static property.
> 
> I do not understand your reasoning. If you have one case that is the
> majority by a wide margin, then it makes sense to make it the default
> and require the other case to be explicit. But that is not what is
> happening here.
> 
> A quick count indicate that among filters adding pads dynamically, there
> are two using allocated names for one using static names. Two-to-one is
> not a huge majority, it is one of the cases where it makes more sense to
> write it explicitly on both cases.
> 
> Furthermore, it is two-to-one for dynamically allocated, which means the
> default should be to free(), and the flags should be
> FF_FILTER_FLAG_DO_NOT_FREE_(IN|OUT)PADS, which would make the logic
> quite ugly.
> 
> And honestly, we are talking adding ", 1" or ", 0" to each
> ff_insert_(in|out)pad() call. Just the name of the flag requires eight
> times as much characters in the source code.
> 

You are distinguishing between filters with allocated input/output names
and those with static input/output names among those filters with
dynamic inputs/outputs and don't see a wide margin between allocated and
static; I am distinguishing between those filters that have both
allocated and static input (or output) names and those that don't have
this mix (among inputs or among outputs) and I see an overwhelming
majority of the latter, as the former group has only two elements
(actually, one could make the former group empty, by making the inpads
using static names static). So the easiest is to use a static flag for
the common default case and to treat the others by other means (either
completely separately as in the first version of this patch or by
setting the flags manually for them).

- Andreas


More information about the ffmpeg-devel mailing list