[FFmpeg-devel] [PATCH] alloc_picture needs to free buffer when it's not going to use it

[Michael Niedermayer]

Hi

On Fri, Jul 20, 2007 at 01:27:17PM -0400, Daniel Kristjansson wrote:
> I'm resending this patch because my last message seems
> to have been moderated and rejected due to my e-mailing
> >from the wrong e-mail account.
> 
> I've also added an 2nd patch which takes care of the
> CHECKED_ALLOCZ case.
> 
> == original message below ==
> 
> If there is some problem with the buffer returned from
> s->avctx->get_buffer(...) mpegvideo.c:alloc_picture(...)
> returns an error code but never returns the picture with
> s->avctx->release_buffer(...).
> 
> The attached patch fixes this for the case when the
> linesizes are wrong, or the age type or data are empty.
> 
> We might want to also do this when CHECKED_ALLOCZ fails,
> but I'll leave that to the experts.
> 
> The reason I noticed this was because MythTV locks a frame
> when get_buffer is called and releases it when release_buffer
> is called. When a user played a video where the resolution
> changed but the decoding library didn't pick up on it the
> buffers were quickly exhausted. (This trigger is a separate
> problem, which may even be MythTV specific.)
> 
> I may have misunderstood who picks up the pieces when 
> alloc_picture() fails, if so I would appreciate a pointer
> in the right direction.
> 
> -- Daniel
> 
> 

> Index: libavcodec/mpegvideo.c
> ===================================================================
> --- libavcodec/mpegvideo.c	(revision 9766)
> +++ libavcodec/mpegvideo.c	(working copy)
> @@ -202,16 +202,22 @@
>  
>          if(r<0 || !pic->age || !pic->type || !pic->data[0]){
>              av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed (%d %d %d %p)\n", r, pic->age, pic->type, pic->data[0]);
> +            if(r>=0)
> +                s->avctx->release_buffer(s->avctx, (AVFrame*)pic);
>              return -1;
>          }

this is wrong, the general assumtation is that a failing function did the
needed cleanup already, we cannot assume that pic is even valid here


>  
>          if(s->linesize && (s->linesize != pic->linesize[0] || s->uvlinesize != pic->linesize[1])){
>              av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed (stride changed)\n");
> +            if(r>=0)
> +                s->avctx->release_buffer(s->avctx, (AVFrame*)pic);
>              return -1;
>          }

r cannot by <0 here


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070720/241abcf5/attachment.pgp>