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

[Steve Lhomme]

On 2019-07-25 8:35, Steve Lhomme wrote:
> 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.

There's also the case of the double display chain. There's the static 
filters chain and the interactive filters chain. Each chain has its 
owner. But the static chain calls the interactive filter allocator 
callback directly without changing its owner. It works because each 
owner share the same opaque.