[vlc-devel] [PATCH 1/8] decoder: let the decoder tell how many pictures it needs in the pool

[Steve Lhomme]

On 2020-07-28 17:40, Rémi Denis-Courmont wrote:
> Le tiistaina 28. heinäkuuta 2020, 12.08.34 EEST Steve Lhomme a écrit :
>> This is necessary for pools that attach buffers/surfaces to the pictures
>> from the decoder so that the picture wait is done at the decoder_t level
>> and not inside each decoder module (which can't be easily canceled
>> otherwise).
>>
>> For now no module provides the proper size so the behavior is unchanged.
> 
> There are two ways to do it:
> 
> 1) (This patch) The decoder tells the owner, and the owner takes care of it.
> This only works for normal (i.e. software) picture buffers.
> 2) The decoder allocates its own pool of the correct size. This works for both
> normal and opaque picture buffers.
> 
> And based on that, we need to support the second mechanism anyway, so the first
> mechanism seems rather vain. The second mechanism is also more in line with
> the earlier design directions/decisions.

I don't think 1 and 2 are exclusive. It depends what you put before the 
word "pool". Is it picture_t pools or surface/buffer pools? In the 
solution I propose the decoders handle their own surface/buffer pool, 
and tell the decoder owner about it. The decoder owner just wraps them 
into picture_t.

The contract is that the decoder tells the owner how many surfaces it's 
going to use and it will query the owner for new wrappers via 
decoder_NewPicture().

For now it's only done for HW surfaces but it can also be done for all 
other buffers if we want to (IMO we don't).

The benefit is that we can get rid of the wait/missing surface handling 
of each decoder because when requested for a surface it's guaranteed 
that there's an available one. That's a lot of boilerplate code removed, 
tricky code to maintain as well (there are currently 2 or 3 
implementations).

In an old work branch I also had a set of callbacks in the picture pool 
so it can create/get the surfaces. In this scenario each decoder would 
provide the callbacks in addition to the size of the pool they need. The 
wrapping and waiting is done elsewhere, in a generic way.

I also have a branch where the picture_buffer_t is decorrelated from the 
picture_t allocation, for CPU pictures, if we want to go full "wrapper" 
for picture_t.

> In any case, this is putting the cart before the horses. The bigger problem is
> how the filters, splitter and display tell how many (extra) buffers they need.

This is a completely different problem. They don't tap in the same pool 
at all. And if they used a picture pool they would not be canceled the 
same way as they are in the vout thread, not the decoder thread(s)+input 
thread.

> We can't just decree that they'll have their own pool for extra buffers, as
> that would imply memory copying from decoder pool buffers to other pool buffers.
> 
> -- 
> レミ・デニ-クールモン
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>