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

Wed Jul 31 09:27:44 CEST 2019

On Wed, Jul 31, 2019, at 08:57, Rémi Denis-Courmont wrote:
> Hi,
> 
> In my understanding, the filter input pictures are allocated by upstream as before. This already followed the push model from the beginning.
> 
> And filter output pictures are the responsibility of the filter to allocate. For preexisting in-place/destructive filters, that means no changes from before. For non-destructive filters, that means allocating in whatever way they see fit, with the potential use of the video context.
> 
> filter_NewPicture() should be just a compatibility alias for picture_NewFromFormat(). Hardware-driving filters should use custom allocators like VDPAU already does, IIRC.

By the way, should we use a CPU picture pool in that case ? Even, if it can be slightly slower from my tests on Linux, using a pool won't cause performance/memory issues that are OS/runtime dependent.

If we want a CPU pool, should the filter decide the number of pictures from the beginning or should we use a dynamic pool (allocating a picture if there is no free pics in the pool) ?

> 
> tl;dr: push model for picture allocation
> 
> Le 30 juillet 2019 23:40:48 GMT+03:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>> Hi,
>> 
>> On Tue, Jul 30, 2019 at 08:12:47PM +0300, Rémi Denis-Courmont wrote:
>>> Le tiistaina 30. heinäkuuta 2019, 10.14.17 EEST Steve Lhomme a écrit :
>>>> On 2019-07-29 21:18, Rémi Denis-Courmont wrote:
>>>>> Le maanantaina 29. heinäkuuta 2019, 22.08.58 EEST Alexandre Janniaux a
>>> écrit :
>>>>>> While I agree that we probably don't need flow control and that image
>>>>>> number is probably dominated, the starvation issue seems a real issue to
>>>>>> tackle. What are the solution against starvation on Windows ? Should we
>>>>>> just fail if we got starved ?
>>>>>  In general, it is the halting problemm which is to say that the general
>>>>>  case cannot be solved and assumptions have to be made.
>>>>> 
>>>>>  If we need to make assumptions, we might just as well assume that filters
>>>>>  are well-behaved, as indeed they have so far been. Ideally, a filter that
>>>>>  needs to allocate surfaces (rather than modify input surface in-place)
>>>>>  should allocate a suitably large pool for itself. And that turns out
>>>>>  incompatible with filter_NewPicture().
>>>> I don't see how. This particular patch adds the possibility for filters
>>>> to provide their own (output) allocator. A filter could very well create
>>>> a pool and have its allocator pick pictures in that pool. That still
>>>> goes through filter_NewPicture, because the filter doesn't know if it
>>>> has to use pictures from the outside or it can use its own.
>> 
>> I don't really understand how can you use picture from the outside in a GPU
>> filter ? It doesn't seem to fit the push model too.
>> 
>> >
>>>  That makes no sense. The "outside" cannot know if the filter allocates or not.
>>>  Already now, both cases have to work - it has more or less always been that
>>>  way with filters.
>>> 
>>>  So far only converters could be assumed to allocate pictures - and in the push
>>>  model, that would be from the video context, not from the downstream/
>>>  filter_NewPicture().
>> 
>> Just to clarify, do we agree that it has to allocate pictures thanks to the
>> video context, but not from it ? Meaning the video context can only provide a
>> rendering-like context for rendering-like API in the case of filter (be it GPU
>> or CPU, the CPU case being "allocate your memory that way (shm) or that way
>> (dynamic memory chunk)") and the memory is allocated by the filter through this
>> context with the rendering API, which should match the context ?
>> 
>> For instance, even if I'm not really fond of this model, it would be an openGL
>> context as a video context, and the filter would use glGenTextures ? Or would
>> the video context provide a `picture_t pf_AllocatePicture(some args)` ?
>> 
>> > >
>>>>> Since the filter chain is (partially) dynamic, we cannot even rely on
>>>>> filters telling how many "extra" surfaces they need - the total value
>>>>> could change in the middle of the stream. Instead, the filter ought to
>>>>> allocate its surfaces during initialization, and potentially refuse to
>>>>> start if it cannot succeed - but not break the whole pipeline.
>>>> I'm not sure we can estimate this amount easily. For example what would
>>>> be the amount an adjust filter has to allocate ?
>>>  The adjust filter needs 0 surfaces since it operates in place.
>>> 
>>>  Point being anyway that only the filter can know that, if anything. Since the
>>>  filter chain is dynamic, it cannot be taken into account when the decoder and
>>>  display are set up. It may be that even the filter does not know how many
>>>  pictures it needs, but then there is nothing to fix; it will just maybe or
>>>  maybe not work by allocating on the fly.
>> 
>> I don't really agree, doing things in place seems to go against the usual
>> practice with filters. Or do we change this model to add copy filters ?
>> 
>> In the mean time, if filters are still processed synchronously, every filters
>> would need only a fixed number of pictures, or at least a number of pictures
>> known by the filter itself, except maybe the last one which have to take into
>> account the pictures allocated by the display. Copies can be made by the filters
>> itself if they absolutely need to keep a picture, and optimization could be made
>> so as to do late copies later only if the image is to be reused.
>> 
>> Remi, is the last-filter special-case what you mean with the fact that only
>> converters are allocating pictures or is it more complex than that ?
>> 
>> Regards,
>> --
>> Alexandre Janniaux
>> VideoLabs
>> 
>> 
>> >
>>>  --
>>>  レミ・デニ-クールモン
>>>  http://www.remlab.net/ vlc-devel mailing list
>>>  To unsubscribe or modify your subscription options:
>>>  https://mailman.videolan.org/listinfo/vlc-devel
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190731/c65d9dea/attachment.html>