| Message ID | PR3PR03MB6665012A545032403B4932748FF99@PR3PR03MB6665.eurprd03.prod.outlook.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,01/23] avfilter/vf_(guided|program_opencl): Add missing dynamic inputs flag | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Andreas Rheinhardt (12021-08-12): > This can be enabled/disabled on a per-filter basis by setting > the new internal flags FF_FILTER_FLAG_FREE_(IN|OUT)PADS. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > It would be possible to only free the names of non-static pads; > it could then be used with the headphone and afir filters. > But I don't think the additional complexity is worth it. I agree on principle. But I think it would be cleaner to implement it with a per-pad flag. We could replace the needs_writable field by a flags field, with NEEDS_WRITABLE as the first flag and FREE_NAME as a new flag. That way, you do not need to distinguish input and output pads in the surrounding code. But since you already wrote the code, I will not insist if you prefer to keep it as it is. Regards,
Nicolas George: > Andreas Rheinhardt (12021-08-12): >> This can be enabled/disabled on a per-filter basis by setting >> the new internal flags FF_FILTER_FLAG_FREE_(IN|OUT)PADS. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> It would be possible to only free the names of non-static pads; >> it could then be used with the headphone and afir filters. >> But I don't think the additional complexity is worth it. > > I agree on principle. But I think it would be cleaner to implement it > with a per-pad flag. > > We could replace the needs_writable field by a flags field, with > NEEDS_WRITABLE as the first flag and FREE_NAME as a new flag. > > That way, you do not need to distinguish input and output pads in the > surrounding code. > > But since you already wrote the code, I will not insist if you prefer to > keep it as it is. > As it so happens, I am working on a different patchset [1] that does add a flags field to AVFilterPads [2], [3] right now. So I am certainly not against this part of your proposal. But it felt more natural to do it this way given that these are freed in avfilter_free(), but making it a per-pad property would allow to easily cover the two remaining cases, namely the headphone and afir filters, so I'll try it and see whether it simplifies things. - Andreas [1]: https://github.com/mkver/FFmpeg/tree/pad_count [2]: https://github.com/mkver/FFmpeg/commit/a492a9db5ab22a8ed03e90de4381d4d1050841e7 [3]: https://github.com/mkver/FFmpeg/commit/8fec2d737e55038754a47b3f5d25ca7ce1105175
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index de7501c37b..9ac247c251 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -117,7 +117,7 @@ void ff_command_queue_pop(AVFilterContext *filter) */ static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off, AVFilterPad **pads, AVFilterLink ***links, - AVFilterPad *newpad) + AVFilterPad *newpad, int free_on_failure) { AVFilterLink **newlinks; AVFilterPad *newpads; @@ -131,8 +131,11 @@ static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off, *pads = newpads; if (newlinks) *links = newlinks; - if (!newpads || !newlinks) + if (!newpads || !newlinks) { + if (free_on_failure) + av_freep(&newpad->name); return AVERROR(ENOMEM); + } memmove(*pads + idx + 1, *pads + idx, sizeof(AVFilterPad) * (*count - idx)); memmove(*links + idx + 1, *links + idx, sizeof(AVFilterLink*) * (*count - idx)); @@ -150,13 +153,15 @@ static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off, int ff_insert_inpad(AVFilterContext *f, unsigned index, AVFilterPad *p) { return insert_pad(index, &f->nb_inputs, offsetof(AVFilterLink, dstpad), - &f->input_pads, &f->inputs, p); + &f->input_pads, &f->inputs, p, + f->filter->flags_internal & FF_FILTER_FLAG_FREE_INPADS); } int ff_insert_outpad(AVFilterContext *f, unsigned index, AVFilterPad *p) { return insert_pad(index, &f->nb_outputs, offsetof(AVFilterLink, srcpad), - &f->output_pads, &f->outputs, p); + &f->output_pads, &f->outputs, p, + f->filter->flags_internal & FF_FILTER_FLAG_FREE_OUTPADS); } int avfilter_link(AVFilterContext *src, unsigned srcpad, @@ -767,7 +772,13 @@ void avfilter_free(AVFilterContext *filter) av_buffer_unref(&filter->hw_device_ctx); av_freep(&filter->name); + if (filter->filter->flags_internal & FF_FILTER_FLAG_FREE_INPADS) + for (unsigned i = 0; i < filter->nb_inputs; i++) + av_freep(&filter->input_pads[i].name); av_freep(&filter->input_pads); + if (filter->filter->flags_internal & FF_FILTER_FLAG_FREE_OUTPADS) + for (unsigned i = 0; i < filter->nb_outputs; i++) + av_freep(&filter->output_pads[i].name); av_freep(&filter->output_pads); av_freep(&filter->inputs); av_freep(&filter->outputs); diff --git a/libavfilter/internal.h b/libavfilter/internal.h index 615b725cab..d9ff997e72 100644 --- a/libavfilter/internal.h +++ b/libavfilter/internal.h @@ -306,6 +306,16 @@ void ff_filter_graph_remove_filter(AVFilterGraph *graph, AVFilterContext *filter */ #define FF_FILTER_FLAG_HWFRAME_AWARE (1 << 0) +/** + * The names of all input pads are allocated and should be freed generically. + */ + #define FF_FILTER_FLAG_FREE_INPADS (1 << 1) + +/** + * The names of all output pads are allocated and should be freed generically. + */ + #define FF_FILTER_FLAG_FREE_OUTPADS (1 << 2) + /** * Run one round of processing on a filter graph. */
This can be enabled/disabled on a per-filter basis by setting the new internal flags FF_FILTER_FLAG_FREE_(IN|OUT)PADS. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- It would be possible to only free the names of non-static pads; it could then be used with the headphone and afir filters. But I don't think the additional complexity is worth it. libavfilter/avfilter.c | 19 +++++++++++++++---- libavfilter/internal.h | 10 ++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-)