[vlc-devel] [PATCH 02/15] filters: pass the owner sys to the buffer_new callback

[Steve Lhomme]

On 2019-07-22 7:12, Steve Lhomme wrote:
> On 2019-07-19 18:32, Rémi Denis-Courmont wrote:
>> Le perjantaina 19. heinäkuuta 2019, 16.32.06 EEST Steve Lhomme a écrit :
>>> This way we don't have to modify the the owner for each call in the
>>> filter chain.
>>
>> IMO, the fact that we even have to do this points at a larger problem.
>>
>> The owner of the filter chain should only care for the allocator of 
>> the last
>> element in the chain (if any), but should not know about the filter, 
>> only the
>> chain. The filter chain code should do the dispatch properly.
> 
> Yes, at first I thought we could just handle it through the video 
> context on the output of the filter chain (that's how I did it in a 
> previous version of my push branch). But since we're moving away from 
> the video context doing everything (no allocator in the context) we 
> still need a way to use the output pull for the last filter in the chain 
> (which may vary).
> 
> The filter must allocate output pictures in 2 different ways: its 
> original "owner" if it's an intermediate filter, the filter chain if 
> it's the last filter. Either we force a fake owner in some calls (what 
> the code does now) or we tell it what callbacks to use to allocate 
> pictures. When the filter is created we don't know if it's the last in 
> the chain (and it can be dynamic). So we set the allocator to something 
> more flexible.
> 
> Sometimes in the code the "owner" design pattern means it's the same 
> pointer but with extra fields/callbacks for its own use. It is not put 
> in the bare object (see decoder_owner for example). In other case the 
> "owner" is actually part of the object structure (vout_display_owner_t, 
> filter_owner_t, vlc_renderer_discovery_owner, 
> services_discovery_owner_t, vout_window_owner_t). In all cases it 
> doesn't seem logical to change the owner during the runtime. So I prefer 
> to pass the right callback set to use during allocation.
> 
> There might be a simple way to keep this special case in the filter 
> chain. I will have a look.

There are 3 ways a filter will be able to get its output pictures :

- use pictures from the display pool if it's the last filter in a chain 
(only possible because the video format and video context of the last 
filter will match the ones from the display module)
- provide its own allocator (especially with a GPU video context)
- use a default allocator

So to get the output pictures the filter has to ask its owner first (in 
case it's the last and will get pictures from somewhere else). But in 
general the owner is the filter chain rather than the filter chain 
owner. The filter chain redirects to the proper picture source depending 
where the filter is in the chain.

The current way to skip the filter chain "end allocator" is by 
pretending to the filter that its owner is the filter chain owner, 
rather than the filter chain itself. It doesn't seem right to change the 
owner of the filter during its lifecycle. If it was a decoder that would 
mean changing its own internal pointer. But here owner means it's just a 
set of callbacks for the filter environment. But these callbacks are 
actually independent of the filter. It doesn't seem to call them and 
force them to get data from the filter, that are unrelated to the 
filter, to work properly. In general callbacks that need to get their 
own environment are given an opaque pointer, not rely on the object they 
handle to recover their environment. (and so the opaque should actually 
be the first parameter of the call)

IMO this is not a clean way to use callbacks.