diff mbox series

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

Message ID 20200821204440.28769-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 257cd5fa389465032b2b222fff5ada9dfebeb4d0
Headers show
Series None | expand

Commit Message

Andreas Rheinhardt Aug. 21, 2020, 8:44 p.m. UTC
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@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.

 libavfilter/vf_hwdownload.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

Comments

Nicolas George Aug. 22, 2020, 12:22 p.m. UTC | #1
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@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,
diff mbox series

Patch

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;