[vlc-devel] [PATCH 12/14] filter_chain: add a default implementation for the video_allocator

[Steve Lhomme]

On 2019-07-29 19:50, Rémi Denis-Courmont wrote:
> Le perjantaina 26. heinäkuuta 2019, 9.18.37 EEST Steve Lhomme a écrit :
>> On 2019-07-25 19:08, Rémi Denis-Courmont wrote:
>>> Le torstaina 25. heinäkuuta 2019, 13.34.11 EEST Steve Lhomme a écrit :
>>>> At some point NULL should not be allowed there.
>>>
>>> I don't think we should have allocation callbacks in filters at all
>>> anymore. We got rid of them for audio years ago.
>>
>> It cannot work for filters outputting GPU surfaces.
> 
> I don't see the causality there?
> 
> Such a filter would need to get the video context, if it needs to allocate
> surfaces. In fact, a GPU filter might need the video context anyway to do
> processing, even if it operates "in-place" and does not allocate GPU surfaces.

Yes, this is what I'm working on. The input video context is "pushed" on 
each filter like the video format. The first filter using the video 
format/context from the decoder. The next filters then get their input 
video context from the previous filter output.

That's where it gets tricky because in some cases it cannot pick its own 
output video context, it has to use the one it's given (see 
b_allow_fmt_out_change in filters) and sometimes it may want to create 
its own and "push" it the next filter. In this case we should make sure 
to use the "decoder device" (aka decoder hint) to create new video 
contexts, especially the GPU ones, at least for the last filter in the 
chain. That means we should carry the original decoder device through 
all the filter chain.

> Why would the filter need to allocate through the callback? And if so, how will
> it work for filters that do not allocate pictures? (VDPAU already has such
> cases.)

For those not allocating anything, I don't think anything needs to be 
changed. If filter_NewPicture() is not used now, it won't in the future. 
So allocation changes have no impact here.

> Just because picture_NewFromFormat() does not work does not imply that
> filter_NewPicture() is required, or even that it works, IMU.

I'm all ears for proposals. IMO filter_NewPicture is still the best 
candidate to allocate output pictures. We don't need to modify existing 
filters much (in fact in many places I handle the "legacy" case so we 
don't have to). And it handles the specific constraints that filters 
have, specifically being able to get pictures from the display pool 
directly so we save a copy when it's time to display the filtered pictures.