| Message ID | 1585668840-30347-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | Accepted |
| Commit | 2b3206891649f317c20993411efef4bee39ae784 |
| Headers | show |
| Series | [FFmpeg-devel] lavc/vaapi_encode: add FF_CODEC_CAP_INIT_CLEANUP caps for encoders | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
> From: Fu, Linjie <linjie.fu@intel.com> > Sent: Tuesday, March 31, 2020 23:34 > To: ffmpeg-devel@ffmpeg.org > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: [PATCH] lavc/vaapi_encode: add FF_CODEC_CAP_INIT_CLEANUP > caps for encoders > > ff_vaapi_encode_close() is not enough to free the resources like cbs > if initialization failure happens after codec->configure (except for > vp8/vp9). > > We need to call avctx->codec->close() to deallocate, otherwise memory > leak happens. > > Add FF_CODEC_CAP_INIT_CLEANUP for vaapi encoders and deallocate the > resources at free_and_end inside avcodec_open2(). > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/vaapi_encode.c | 1 - > libavcodec/vaapi_encode_h264.c | 1 + > libavcodec/vaapi_encode_h265.c | 1 + > libavcodec/vaapi_encode_mjpeg.c | 1 + > libavcodec/vaapi_encode_mpeg2.c | 1 + > libavcodec/vaapi_encode_vp8.c | 1 + > libavcodec/vaapi_encode_vp9.c | 1 + > 7 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 8ff720e..d371033 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -2361,7 +2361,6 @@ av_cold int ff_vaapi_encode_init(AVCodecContext > *avctx) > return 0; > > fail: > - ff_vaapi_encode_close(avctx); > return err; > } > > diff --git a/libavcodec/vaapi_encode_h264.c > b/libavcodec/vaapi_encode_h264.c > index f4965d8..6a86905 100644 > --- a/libavcodec/vaapi_encode_h264.c > +++ b/libavcodec/vaapi_encode_h264.c > @@ -1356,6 +1356,7 @@ AVCodec ff_h264_vaapi_encoder = { > .close = &vaapi_encode_h264_close, > .priv_class = &vaapi_encode_h264_class, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_h264_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > diff --git a/libavcodec/vaapi_encode_h265.c > b/libavcodec/vaapi_encode_h265.c > index 97dc5a7..4c24f7c 100644 > --- a/libavcodec/vaapi_encode_h265.c > +++ b/libavcodec/vaapi_encode_h265.c > @@ -1292,6 +1292,7 @@ AVCodec ff_hevc_vaapi_encoder = { > .close = &vaapi_encode_h265_close, > .priv_class = &vaapi_encode_h265_class, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_h265_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > diff --git a/libavcodec/vaapi_encode_mjpeg.c > b/libavcodec/vaapi_encode_mjpeg.c > index bd029cc..c469e70 100644 > --- a/libavcodec/vaapi_encode_mjpeg.c > +++ b/libavcodec/vaapi_encode_mjpeg.c > @@ -565,6 +565,7 @@ AVCodec ff_mjpeg_vaapi_encoder = { > .priv_class = &vaapi_encode_mjpeg_class, > .capabilities = AV_CODEC_CAP_HARDWARE | > AV_CODEC_CAP_INTRA_ONLY, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_mjpeg_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > diff --git a/libavcodec/vaapi_encode_mpeg2.c > b/libavcodec/vaapi_encode_mpeg2.c > index bac9ea1..55f9289 100644 > --- a/libavcodec/vaapi_encode_mpeg2.c > +++ b/libavcodec/vaapi_encode_mpeg2.c > @@ -702,6 +702,7 @@ AVCodec ff_mpeg2_vaapi_encoder = { > .close = &vaapi_encode_mpeg2_close, > .priv_class = &vaapi_encode_mpeg2_class, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_mpeg2_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > diff --git a/libavcodec/vaapi_encode_vp8.c > b/libavcodec/vaapi_encode_vp8.c > index 6e7bf9d..d8fb031 100644 > --- a/libavcodec/vaapi_encode_vp8.c > +++ b/libavcodec/vaapi_encode_vp8.c > @@ -257,6 +257,7 @@ AVCodec ff_vp8_vaapi_encoder = { > .close = &ff_vaapi_encode_close, > .priv_class = &vaapi_encode_vp8_class, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_vp8_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > diff --git a/libavcodec/vaapi_encode_vp9.c > b/libavcodec/vaapi_encode_vp9.c > index d7f415d..ea19470 100644 > --- a/libavcodec/vaapi_encode_vp9.c > +++ b/libavcodec/vaapi_encode_vp9.c > @@ -291,6 +291,7 @@ AVCodec ff_vp9_vaapi_encoder = { > .close = &ff_vaapi_encode_close, > .priv_class = &vaapi_encode_vp9_class, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > .defaults = vaapi_encode_vp9_defaults, > .pix_fmts = (const enum AVPixelFormat[]) { > AV_PIX_FMT_VAAPI, > -- Ping for review, this patch helps to fix the potential memory leaks.
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Thursday, April 23, 2020 22:59 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode: add > FF_CODEC_CAP_INIT_CLEANUP caps for encoders > > > From: Fu, Linjie <linjie.fu@intel.com> > > Sent: Tuesday, March 31, 2020 23:34 > > To: ffmpeg-devel@ffmpeg.org > > Cc: Fu, Linjie <linjie.fu@intel.com> > > Subject: [PATCH] lavc/vaapi_encode: add FF_CODEC_CAP_INIT_CLEANUP > > caps for encoders > > > > ff_vaapi_encode_close() is not enough to free the resources like cbs > > if initialization failure happens after codec->configure (except for > > vp8/vp9). > > > > We need to call avctx->codec->close() to deallocate, otherwise memory > > leak happens. > > > > Add FF_CODEC_CAP_INIT_CLEANUP for vaapi encoders and deallocate the > > resources at free_and_end inside avcodec_open2(). > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavcodec/vaapi_encode.c | 1 - > > libavcodec/vaapi_encode_h264.c | 1 + > > libavcodec/vaapi_encode_h265.c | 1 + > > libavcodec/vaapi_encode_mjpeg.c | 1 + > > libavcodec/vaapi_encode_mpeg2.c | 1 + > > libavcodec/vaapi_encode_vp8.c | 1 + > > libavcodec/vaapi_encode_vp9.c | 1 + > > 7 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > > index 8ff720e..d371033 100644 > > --- a/libavcodec/vaapi_encode.c > > +++ b/libavcodec/vaapi_encode.c > > @@ -2361,7 +2361,6 @@ av_cold int > ff_vaapi_encode_init(AVCodecContext > > *avctx) > > return 0; > > > > fail: > > - ff_vaapi_encode_close(avctx); > > return err; > > } > > > > diff --git a/libavcodec/vaapi_encode_h264.c > > b/libavcodec/vaapi_encode_h264.c > > index f4965d8..6a86905 100644 > > --- a/libavcodec/vaapi_encode_h264.c > > +++ b/libavcodec/vaapi_encode_h264.c > > @@ -1356,6 +1356,7 @@ AVCodec ff_h264_vaapi_encoder = { > > .close = &vaapi_encode_h264_close, > > .priv_class = &vaapi_encode_h264_class, > > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_h264_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > diff --git a/libavcodec/vaapi_encode_h265.c > > b/libavcodec/vaapi_encode_h265.c > > index 97dc5a7..4c24f7c 100644 > > --- a/libavcodec/vaapi_encode_h265.c > > +++ b/libavcodec/vaapi_encode_h265.c > > @@ -1292,6 +1292,7 @@ AVCodec ff_hevc_vaapi_encoder = { > > .close = &vaapi_encode_h265_close, > > .priv_class = &vaapi_encode_h265_class, > > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_h265_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > diff --git a/libavcodec/vaapi_encode_mjpeg.c > > b/libavcodec/vaapi_encode_mjpeg.c > > index bd029cc..c469e70 100644 > > --- a/libavcodec/vaapi_encode_mjpeg.c > > +++ b/libavcodec/vaapi_encode_mjpeg.c > > @@ -565,6 +565,7 @@ AVCodec ff_mjpeg_vaapi_encoder = { > > .priv_class = &vaapi_encode_mjpeg_class, > > .capabilities = AV_CODEC_CAP_HARDWARE | > > AV_CODEC_CAP_INTRA_ONLY, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_mjpeg_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > diff --git a/libavcodec/vaapi_encode_mpeg2.c > > b/libavcodec/vaapi_encode_mpeg2.c > > index bac9ea1..55f9289 100644 > > --- a/libavcodec/vaapi_encode_mpeg2.c > > +++ b/libavcodec/vaapi_encode_mpeg2.c > > @@ -702,6 +702,7 @@ AVCodec ff_mpeg2_vaapi_encoder = { > > .close = &vaapi_encode_mpeg2_close, > > .priv_class = &vaapi_encode_mpeg2_class, > > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_mpeg2_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > diff --git a/libavcodec/vaapi_encode_vp8.c > > b/libavcodec/vaapi_encode_vp8.c > > index 6e7bf9d..d8fb031 100644 > > --- a/libavcodec/vaapi_encode_vp8.c > > +++ b/libavcodec/vaapi_encode_vp8.c > > @@ -257,6 +257,7 @@ AVCodec ff_vp8_vaapi_encoder = { > > .close = &ff_vaapi_encode_close, > > .priv_class = &vaapi_encode_vp8_class, > > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_vp8_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > diff --git a/libavcodec/vaapi_encode_vp9.c > > b/libavcodec/vaapi_encode_vp9.c > > index d7f415d..ea19470 100644 > > --- a/libavcodec/vaapi_encode_vp9.c > > +++ b/libavcodec/vaapi_encode_vp9.c > > @@ -291,6 +291,7 @@ AVCodec ff_vp9_vaapi_encoder = { > > .close = &ff_vaapi_encode_close, > > .priv_class = &vaapi_encode_vp9_class, > > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, > > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > > .defaults = vaapi_encode_vp9_defaults, > > .pix_fmts = (const enum AVPixelFormat[]) { > > AV_PIX_FMT_VAAPI, > > -- > > Ping for review, this patch helps to fix the potential memory leaks. > Ping. - Linjie
On 31.03.2020 17:34, Linjie Fu wrote: > ff_vaapi_encode_close() is not enough to free the resources like cbs > if initialization failure happens after codec->configure (except for > vp8/vp9). > > We need to call avctx->codec->close() to deallocate, otherwise memory > leak happens. > > Add FF_CODEC_CAP_INIT_CLEANUP for vaapi encoders and deallocate the > resources at free_and_end inside avcodec_open2(). > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> Not my area of code, but the patch looks straight forward enough to my eyes.
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Timo Rothenpieler > Sent: Monday, May 4, 2020 13:20 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] lavc/vaapi_encode: add > FF_CODEC_CAP_INIT_CLEANUP caps for encoders > > On 31.03.2020 17:34, Linjie Fu wrote: > > ff_vaapi_encode_close() is not enough to free the resources like cbs > > if initialization failure happens after codec->configure (except for > > vp8/vp9). > > > > We need to call avctx->codec->close() to deallocate, otherwise memory > > leak happens. > > > > Add FF_CODEC_CAP_INIT_CLEANUP for vaapi encoders and deallocate the > > resources at free_and_end inside avcodec_open2(). > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > Not my area of code, but the patch looks straight forward enough to my eyes. Thanks for review, and hope this could be merged soon. - Linjie
diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 8ff720e..d371033 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -2361,7 +2361,6 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx) return 0; fail: - ff_vaapi_encode_close(avctx); return err; } diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c index f4965d8..6a86905 100644 --- a/libavcodec/vaapi_encode_h264.c +++ b/libavcodec/vaapi_encode_h264.c @@ -1356,6 +1356,7 @@ AVCodec ff_h264_vaapi_encoder = { .close = &vaapi_encode_h264_close, .priv_class = &vaapi_encode_h264_class, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_h264_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI, diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c index 97dc5a7..4c24f7c 100644 --- a/libavcodec/vaapi_encode_h265.c +++ b/libavcodec/vaapi_encode_h265.c @@ -1292,6 +1292,7 @@ AVCodec ff_hevc_vaapi_encoder = { .close = &vaapi_encode_h265_close, .priv_class = &vaapi_encode_h265_class, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_h265_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI, diff --git a/libavcodec/vaapi_encode_mjpeg.c b/libavcodec/vaapi_encode_mjpeg.c index bd029cc..c469e70 100644 --- a/libavcodec/vaapi_encode_mjpeg.c +++ b/libavcodec/vaapi_encode_mjpeg.c @@ -565,6 +565,7 @@ AVCodec ff_mjpeg_vaapi_encoder = { .priv_class = &vaapi_encode_mjpeg_class, .capabilities = AV_CODEC_CAP_HARDWARE | AV_CODEC_CAP_INTRA_ONLY, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_mjpeg_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI, diff --git a/libavcodec/vaapi_encode_mpeg2.c b/libavcodec/vaapi_encode_mpeg2.c index bac9ea1..55f9289 100644 --- a/libavcodec/vaapi_encode_mpeg2.c +++ b/libavcodec/vaapi_encode_mpeg2.c @@ -702,6 +702,7 @@ AVCodec ff_mpeg2_vaapi_encoder = { .close = &vaapi_encode_mpeg2_close, .priv_class = &vaapi_encode_mpeg2_class, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_mpeg2_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI, diff --git a/libavcodec/vaapi_encode_vp8.c b/libavcodec/vaapi_encode_vp8.c index 6e7bf9d..d8fb031 100644 --- a/libavcodec/vaapi_encode_vp8.c +++ b/libavcodec/vaapi_encode_vp8.c @@ -257,6 +257,7 @@ AVCodec ff_vp8_vaapi_encoder = { .close = &ff_vaapi_encode_close, .priv_class = &vaapi_encode_vp8_class, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_vp8_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI, diff --git a/libavcodec/vaapi_encode_vp9.c b/libavcodec/vaapi_encode_vp9.c index d7f415d..ea19470 100644 --- a/libavcodec/vaapi_encode_vp9.c +++ b/libavcodec/vaapi_encode_vp9.c @@ -291,6 +291,7 @@ AVCodec ff_vp9_vaapi_encoder = { .close = &ff_vaapi_encode_close, .priv_class = &vaapi_encode_vp9_class, .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HARDWARE, + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, .defaults = vaapi_encode_vp9_defaults, .pix_fmts = (const enum AVPixelFormat[]) { AV_PIX_FMT_VAAPI,
ff_vaapi_encode_close() is not enough to free the resources like cbs if initialization failure happens after codec->configure (except for vp8/vp9). We need to call avctx->codec->close() to deallocate, otherwise memory leak happens. Add FF_CODEC_CAP_INIT_CLEANUP for vaapi encoders and deallocate the resources at free_and_end inside avcodec_open2(). Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/vaapi_encode.c | 1 - libavcodec/vaapi_encode_h264.c | 1 + libavcodec/vaapi_encode_h265.c | 1 + libavcodec/vaapi_encode_mjpeg.c | 1 + libavcodec/vaapi_encode_mpeg2.c | 1 + libavcodec/vaapi_encode_vp8.c | 1 + libavcodec/vaapi_encode_vp9.c | 1 + 7 files changed, 6 insertions(+), 1 deletion(-)