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

[Uoti Urpala]

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
objects.

> > 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.