[FFmpeg-devel] [PATCH v2 18/21] avfilter/vf_hwdownload: Fix leak of formats list upon error

[Nicolas George]

Andreas Rheinhardt (12020-08-21):
> If adding the list of input formats to its AVFilterLink fails, the list
> of output formats (which has not been attached to permanent storage yet)
> leaks. This has been fixed by not creating the lists of in- and output
> formats simultaneously. Instead creating said lists is relegated to
> ff_formats_pixdesc_filter() (this also avoids the reallocations implicit
> in using ff_add_format()) and the second list is only created after (and
> if) the first list has been permanently attached to ist AVFilterLink.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> ff_formats_pixdesc_filter() uses av_pix_fmt_desc_get() under the hood;
> but both av_pix_fmt_desc_get() and av_pix_fmt_desc_next() lead
> (currently) to the same order of pixel formats (it coincides with the
> natural order) and given that this filter doesn't seem to have any
> preferred order at all, it wouldn't even matter if these two methods
> yielded different results one day.

The order of formats should not matter, the API does not mean them to
matter.

> 
>  libavfilter/vf_hwdownload.c | 23 +++++------------------
>  1 file changed, 5 insertions(+), 18 deletions(-)
> 
> diff --git a/libavfilter/vf_hwdownload.c b/libavfilter/vf_hwdownload.c
> index 33af30cf40..faf2ea8c0e 100644
> --- a/libavfilter/vf_hwdownload.c
> +++ b/libavfilter/vf_hwdownload.c
> @@ -37,26 +37,13 @@ typedef struct HWDownloadContext {
>  
>  static int hwdownload_query_formats(AVFilterContext *avctx)
>  {
> -    AVFilterFormats  *infmts = NULL;
> -    AVFilterFormats *outfmts = NULL;
> -    const AVPixFmtDescriptor *desc;
> +    AVFilterFormats *fmts;
>      int err;
>  
> -    for (desc = av_pix_fmt_desc_next(NULL); desc;
> -         desc = av_pix_fmt_desc_next(desc)) {
> -        if (desc->flags & AV_PIX_FMT_FLAG_HWACCEL)
> -            err = ff_add_format(&infmts,  av_pix_fmt_desc_get_id(desc));
> -        else
> -            err = ff_add_format(&outfmts, av_pix_fmt_desc_get_id(desc));
> -        if (err) {
> -            ff_formats_unref(&infmts);
> -            ff_formats_unref(&outfmts);
> -            return err;
> -        }
> -    }
> -
> -    if ((err = ff_formats_ref(infmts,  &avctx->inputs[0]->out_formats)) < 0 ||
> -        (err = ff_formats_ref(outfmts, &avctx->outputs[0]->in_formats)) < 0)
> +    if ((err = ff_formats_pixdesc_filter(&fmts, AV_PIX_FMT_FLAG_HWACCEL, 0)) ||
> +        (err = ff_formats_ref(fmts, &avctx->inputs[0]->out_formats))         ||
> +        (err = ff_formats_pixdesc_filter(&fmts, 0, AV_PIX_FMT_FLAG_HWACCEL)) ||
> +        (err = ff_formats_ref(fmts, &avctx->outputs[0]->in_formats)))
>          return err;
>  
>      return 0;

It looks ok, but I do not maintain this code.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200822/e3305470/attachment.sig>