> -----Original Message-----
> From: Li, Zhong
> Sent: Thursday, April 11, 2019 11:14
> To: FFmpeg development discussions and patches <ffmpeg-
> de...@ffmpeg.org>
> Cc: Fu, Linjie <linjie...@intel.com>
> Subject: RE: [FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: Fix the memory leak
> for enc_ctrl.Payload
> 
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of Linjie Fu
> > Sent: Wednesday, April 10, 2019 7:27 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie <linjie...@intel.com>
> > Subject: [FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: Fix the memory leak for
> > enc_ctrl.Payload
> >
> > frame->enc_ctrl.Payload is malloced in get_free_frame, directly memset
> > the whole structure of enc_ctrl to zero will cause the memory leak for
> > enc_ctrl.Payload.
> >
> > Fix the memory leak issue and reset other members in mfxEncodeCtrl.
> >
> > Signed-off-by: Linjie Fu <linjie...@intel.com>
> > ---
> >  libavcodec/qsvenc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 5aa020d47b..029bb562d6 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1254,7 +1254,7 @@ static int encode_frame(AVCodecContext *avctx,
> > QSVEncContext *q,
> >      if (qsv_frame) {
> >          surf = &qsv_frame->surface;
> >          enc_ctrl = &qsv_frame->enc_ctrl;
> > -        memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl));
> > +        memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl) - sizeof(mfxPayload
> > + **));
> 
> Looks a little tricky, if someone didn't read carefully the sequence of
> mfxPayload usages, they can't know why this change happening.
> And potential risk for other elements such as "mfxExtBuffer** ExtParam" is
> still existed.
> 
> IMHO, one good habit is that is you want to memset a buffer, would better
> memset it when you allocate it.
> So I believe the correct fix should be: memset mfxEncodeCtrl at the start of
> qsv frame allocated: the place should be in the function get_free_frame()

frame->enc_ctrl  as a structure will be malloc and init to zero by calling 
frame = av_mallocz(sizeof(*frame)),
the memset may be redundant and can be removed in my opinion.
 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to