diff mbox

[FFmpeg-devel] libavfilter/vf_cover_rect.c: free the allocated frame

Message ID 20190606083056.10791-1-lance.lmwang@gmail.com
State Superseded
Headers show

Commit Message

Lance Wang June 6, 2019, 8:30 a.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_cover_rect.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer June 7, 2019, 3:13 p.m. UTC | #1
On Thu, Jun 06, 2019 at 04:30:56PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_cover_rect.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> index 41cd1a12b9..d63c03215d 100644
> --- a/libavfilter/vf_cover_rect.c
> +++ b/libavfilter/vf_cover_rect.c
> @@ -196,8 +196,10 @@ static av_cold void uninit(AVFilterContext *ctx)
>  {
>      CoverContext *cover = ctx->priv;
>  
> -    if (cover->cover_frame)
> +    if (cover->cover_frame) {
>          av_freep(&cover->cover_frame->data[0]);
> +        av_frame_free(&cover->cover_frame);

the AVFrame should be setup in such a way that a av_frame_free() alone is 
enough and explicit no av_freep of data is needed

thx

[...]
Lance Wang June 7, 2019, 3:30 p.m. UTC | #2
On Fri, Jun 7, 2019 at 11:13 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Thu, Jun 06, 2019 at 04:30:56PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavfilter/vf_cover_rect.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > index 41cd1a12b9..d63c03215d 100644
> > --- a/libavfilter/vf_cover_rect.c
> > +++ b/libavfilter/vf_cover_rect.c
> > @@ -196,8 +196,10 @@ static av_cold void uninit(AVFilterContext *ctx)
> >  {
> >      CoverContext *cover = ctx->priv;
> >
> > -    if (cover->cover_frame)
> > +    if (cover->cover_frame) {
> >          av_freep(&cover->cover_frame->data[0]);
> > +        av_frame_free(&cover->cover_frame);
>
> the AVFrame should be setup in such a way that a av_frame_free() alone is
> enough and explicit no av_freep of data is needed
>

Sorry, I haven't catch your point.  Don't need av_freep() or
av_frame_free() to free the memory?
For the av_freep is existing code already.



>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer June 7, 2019, 4:02 p.m. UTC | #3
On Fri, Jun 07, 2019 at 11:30:05PM +0800, Lance Wang wrote:
> On Fri, Jun 7, 2019 at 11:13 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Thu, Jun 06, 2019 at 04:30:56PM +0800, lance.lmwang@gmail.com wrote:
> > > From: Limin Wang <lance.lmwang@gmail.com>
> > >
> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > ---
> > >  libavfilter/vf_cover_rect.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
> > > index 41cd1a12b9..d63c03215d 100644
> > > --- a/libavfilter/vf_cover_rect.c
> > > +++ b/libavfilter/vf_cover_rect.c
> > > @@ -196,8 +196,10 @@ static av_cold void uninit(AVFilterContext *ctx)
> > >  {
> > >      CoverContext *cover = ctx->priv;
> > >
> > > -    if (cover->cover_frame)
> > > +    if (cover->cover_frame) {
> > >          av_freep(&cover->cover_frame->data[0]);
> > > +        av_frame_free(&cover->cover_frame);
> >
> > the AVFrame should be setup in such a way that a av_frame_free() alone is
> > enough and explicit no av_freep of data is needed
> >
> 
> Sorry, I haven't catch your point.  Don't need av_freep() or
> av_frame_free() to free the memory?

> For the av_freep is existing code already.

yes but its ugly.
A AVFrame should be free-able with just the standard av_frame_free()
the data[] memory should be deallocated via appropriately setup buf[]


[...]

thx
diff mbox

Patch

diff --git a/libavfilter/vf_cover_rect.c b/libavfilter/vf_cover_rect.c
index 41cd1a12b9..d63c03215d 100644
--- a/libavfilter/vf_cover_rect.c
+++ b/libavfilter/vf_cover_rect.c
@@ -196,8 +196,10 @@  static av_cold void uninit(AVFilterContext *ctx)
 {
     CoverContext *cover = ctx->priv;
 
-    if (cover->cover_frame)
+    if (cover->cover_frame) {
         av_freep(&cover->cover_frame->data[0]);
+        av_frame_free(&cover->cover_frame);
+    }
 }
 
 static av_cold int init(AVFilterContext *ctx)