[MPlayer-dev-eng] [PATCH]Fix possible memlead in vo_vdpau

Uoti Urpala uoti.urpala at pp1.inet.fi
Sat Oct 10 18:24:35 CEST 2009

On Sat, 2009-10-10 at 15:46 +0000, Carl Eugen Hoyos wrote:
> Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:
> > This looks wrong to me, bitstream_buffers gets neither overwritten nor
> > reallocated nor anything here.
> > I mean it may be correct in principle, but it violates the rule that
> > things should be closed/freed in the same or a symmetric place to where
> > they were allocated/opened.
> Am I correct that libavcodec currently doesn't "know" about those pointers and
> cannot free them?

struct vdpau_render_state is defined in libavcodec, but the objects are
allocated on the application side. Pointers to the objects are then
passed to libavcodec, which allocates new memory and places pointers in
the objects. I think libavcodec does not keep an overall list of the

> > I don't have a suggestion how to fix this right now though (IMO this
> > should be done on avcodec_close, but this opens up some ugly issues),
> > but at least I'd suggest to do this in uninit.
> Is the following (inlined) acceptable?

> +    for (i = 0; i < MAX_VIDEO_SURFACES; i++) {
> +        // Allocated in ff_vdpau_add_data_chunk()
> +        av_freep(&surface_render[i].bitstream_buffers);
> +        surface_render[i].bitstream_buffers_allocated = 0;

I think having a "free whatever libavcodec allocated" function in lavc
would be better. Though if it's guaranteed the allocated fields or their
allocation method will never change then just documenting this in lavc
could be OK. But either such a function or documentation of this
behavior really needs to be added to lavc.

More information about the MPlayer-dev-eng mailing list