[vlc-devel] [PATCH] picture_pool: do not unconditionally free the pictures
rem at videolan.org
Sun Jul 14 18:04:39 CEST 2013
On Thursday 11 July 2013 12:49:02 Luca Barbato wrote:
> > It won't work because you do not restore pf_destroy to its proper value.
> > You end up releasing the picture from the (already destroyed) pool instead
> > of actually destroying the picture.
> Valgrind claims otherwise, probably I'm lucky.
If you fail to restore the destroy callback and associated private data
pointer, there is no way that the picture is ever going to get freed.
I don't know if valgrind complains about leaked System V shared memory
segments. But it should complain about the picture_t and the picture_sys_t.
The bottom line is, your patch looks just plain wrong to me. Even if we
decided to allow pictures to outlive the video output display and pool, it
would still look wrong to me.
> > I agree that it is an ugly mess (not my design), but it used to be much
> > worse. But then again, it seems reasonable for video output plugins to
> > rely
> > on their pictures no longer being referenced upon invalid pictures reset
> > or
> > close. If someone wants to change this, I would be happy to adapt the XCB
> > video outputs, but what about the other ones?
> I'm not sure what's the best solution, ideally the whole
> vout-reset-when-the-decoder-is-active should be avoided.
What do you mean by that? Most if not all video outputs will require some
level of reinitialization and new picture buffers if the decoder format
> > Arguably the decoder should flush automatically before it changes format.
> > Is there any video codec where pictures reference can cross format change
> > boundaries? Admittedly, the codec interface does not make this easy since
> > the format change is combined with the picture allocation.
> mpeg4, h264, even mpeg2 might change midstream I guess.
That's not the question. The question is whether any codec can have pictures
after a format change reference older pictures from before the format change
(or vice versa). If that were the case, then flushing the decoder upon format
changes would be fundamentally impossible.
More information about the vlc-devel