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

[Luca Barbato]

On 07/11/2013 09:48 PM, fenrir at elivagar.org wrote:
> The current code will not work if some of the pictures are not released before
> the pool.

Thus why I said that we can have two functions, one to wipe and the
other just just drop the pool (assuming another pool is taking care of
the pictures, as it is happening now).

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

It assumes that after the call nothing with a reference to the picture
is active anymore. Sadly it is not happening.
It assumes that some code would leak pictures. That should be fixed
notwithstanding.

> The asumption the vout makes is that a decoder will NOT have any pictures
> references left before making a resolution changes.

Depends on the decoder, if you want a native QSV codec you'd have the
same issue. Same goes for other hwaccelerated ones.

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

I pestered people on give me a map so I can do that myself, I tend to
get lost jumping from a p_sys to a p_owner usually ^^;

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

Keeping a refcount on the pool seems sort of simple, if there are more
references to the pool you just do the normal release, if the count is
zero you do a wipe over the pool and warn about broken code not cleaning
up properly.

I can help but I need some direction =)

lu