[vlc-devel] [PATCH] picture_pool: do not unconditionally free the pictures

[fenrir at elivagar.org]

Hi,

On Thu, Jul 11, 2013 at 11:26:45AM +0200, Luca Barbato wrote:
> On 07/11/2013 11:15 AM, Rémi Denis-Courmont wrote:
> > Then where and how do you think the pictures gets freed?
> 
> By the code holding the additional references, that does exactly what it
> is supposed to do =)

The current code will not work if some of the pictures are not released before
the pool.

> The whole thing is a mess since the assumption (wrong) is that you call
> that function when there aren't any other active user holding a
> reference (assuming something went really wrong and whoever had that
> additional reference died an horrible and dirty death).
> 
> That's not the case and the whole assumption looks wrong from start to me.
I disagree. I mean it's obviously not the only possible solution, but is a
valid one if some requirements are respected.

The asumption the vout makes is that a decoder will NOT have any pictures
references left before making a resolution changes.
Up until threaded avcodec, this assumption was valid and thus it was not
necessary to worry about this issue.
(I know that this also means that on a resolution changes a few pictures
were lost).

Now, if the avcodec decoder ends up referencing pictures before and after
a resolution change, the current code will not work.

IMHO, the simplest and most risk free fix would be to ensure that the decoder
flush itself on resize i.e. obeying one of the the vout requirements.

Another solution could be to refcount the pool and destroy it only when it is
unreferenced and all the pictures associated are unreferenced too. But this opens
a new can of worm (you must ensure that the vout_display does not change or
refcount it as it may be responsible for releasing them, it also means that you
allocate twice video memory and this is a scarse resource when using HD, etc)

Regards,

-- 
fenrir